On Friday 24 October 2008 15:47:20 Hiroshi Shimamoto wrote:> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>>> The following assignment in smp_call_function_many() may cause unexpected> behavior, when !CPUMASK_OFFSTACK.> data->cpumask = allbutself;>> Because it copys pointer of stack and the value will be modified after> exit from smp_call_function_many().

Good catch!

> The type of cpumask field of call_function_data structure should be> cpumask_var_t and an operation to assign is needed.

This makes the lifetime rules dependent on the config option, which iscomplicated.

Your insight into this issue is appreciated: this code is not simple!

How's this version instead? It puts the cpumask at the end of the kmalloc,and falls back to smp_call_function_single instead of doing obscurequiescing stuff.

-/* Dummy function */-static void quiesce_dummy(void *unused)-{-}--/*- * Ensure stack based data used in call function mask is safe to free.- *- * This is needed by smp_call_function_many when using on-stack data, because- * a single call function queue is shared by all CPUs, and any CPU may pick up- * the data item on the queue at any time before it is deleted. So we need to- * ensure that all CPUs have transitioned through a quiescent state after- * this call.- *- * This is a very slow function, implemented by sending synchronous IPIs to- * all possible CPUs. For this reason, we have to alloc data rather than use- * stack based data even in the case of synchronous calls. The stack based- * data is then just used for deadlock/oom fallback which will be very rare.- *- * If a faster scheme can be made, we could go back to preferring stack based- * data -- the data allocation/free is non-zero cost.- */-static void smp_call_function_mask_quiesce_stack(const struct cpumask *mask)-{- struct call_single_data data;- int cpu;-- data.func = quiesce_dummy;- data.info = NULL;-- for_each_cpu(cpu, mask) {- data.flags = CSD_FLAG_WAIT;- generic_exec_single(cpu, &data);- }-}- /** * smp_call_function_many(): Run a function on a set of other CPUs. * @mask: The set of cpus to run on (only runs on online subset).@@ -320,73 +284,59 @@ void smp_call_function_many(const struct void (*func)(void *), void *info, bool wait) {- struct call_function_data d;- struct call_function_data *data = NULL;- cpumask_var_t allbutself;+ struct call_function_data *data; unsigned long flags;- int cpu, num_cpus;- int slowpath = 0;+ int cpu, next_cpu;

/* Can deadlock when called with interrupts disabled */ WARN_ON(irqs_disabled());