linux-kernel - Re: [PATCH 2/6] sched: add function to execute a function synchronously on a physical cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56E2BD89.4020400@suse.com>
Date:	Fri, 11 Mar 2016 13:43:53 +0100
From:	Juergen Gross <jgross@...e.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel@...r.kernel.org, xen-devel@...ts.xenproject.org,
	konrad.wilk@...cle.com, boris.ostrovsky@...cle.com,
	david.vrabel@...rix.com, mingo@...hat.com,
	Douglas_Warzecha@...l.com, pali.rohar@...il.com, jdelvare@...e.com,
	linux@...ck-us.net, tglx@...utronix.de, hpa@...or.com,
	x86@...nel.org
Subject: Re: [PATCH 2/6] sched: add function to execute a function
 synchronously on a physical cpu

On 11/03/16 13:19, Peter Zijlstra wrote:
> On Fri, Mar 11, 2016 at 12:59:30PM +0100, Juergen Gross wrote:
>> +int call_sync_on_phys_cpu(unsigned cpu, int (*func)(void *), void *par)
>> +{
>> +	cpumask_var_t old_mask;
>> +	int ret;
>> +
>> +	if (cpu >= nr_cpu_ids)
>> +		return -EINVAL;
>> +
>> +	if (!alloc_cpumask_var(&old_mask, GFP_KERNEL))
>> +		return -ENOMEM;
>> +
>> +	cpumask_copy(old_mask, &current->cpus_allowed);
>> +	ret = set_cpus_allowed_ptr(current, cpumask_of(cpu));
>> +	if (ret)
>> +		goto out;
> 
> So what happens if someone does sched_setaffinity() right about here?

Aah, okay. Seems I should add preempt_disable() in this patch already
and retry the set_cpus_allowed_ptr() in case cpus_allowed has changed.

> 
>> +
>> +	ret = func(par);
>> +
>> +	set_cpus_allowed_ptr(current, old_mask);
>> +
>> +out:
>> +	free_cpumask_var(old_mask);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(call_sync_on_phys_cpu);
> 
> This is disgusting, and you're adding this to !Xen kernels too.

Sure. It is called on !Xen kernels too. Without Xen it is just omitting
the vcpu pinning. I've copied above logic from dcdbas/i8k (it was open
coded twice).

BTW: I tried to get rid of the complete logic to call a function on cpu
0 only. It was rejected by the Dell maintainers.


Juergen