Re: [PATCH 1/2] smp_call_function: don't use lock in call_function_data

From: Nick Piggin
Date: Thu Aug 21 2008 - 21:47:56 EST


On Friday 22 August 2008 10:29, Jeremy Fitzhardinge wrote:
> There's no need for a lock in call_function_data, since it's only used
> to decrement-and-test a counter. Use an atomic instead.

Actually I wanted to convert the cpu_clear operation to be non-atomic,
and keep it under the lock. Thus the spinlock would save one atomic
operation. I simply forgot about this after Jens took over the patchset.

We could get rid of that WARN_ON branch in 2.6.28 I expect, unless we
see it trigger.

>
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>
> ---
> kernel/smp.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> ===================================================================
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -10,6 +10,7 @@
> #include <linux/rcupdate.h>
> #include <linux/rculist.h>
> #include <linux/smp.h>
> +#include <asm/atomic.h>
>
> bool __read_mostly smp_single_ipi_queue = false;
>
> @@ -37,8 +38,7 @@
>
> struct call_function_data {
> struct call_single_data csd;
> - spinlock_t lock;
> - unsigned int refs;
> + atomic_t refs;
> cpumask_t cpumask;
> struct rcu_head rcu_head;
> };
> @@ -125,21 +125,13 @@
> */
> rcu_read_lock();
> list_for_each_entry_rcu(data, &queue->list, csd.list) {
> - int refs;
> -
> if (!cpu_isset(cpu, data->cpumask))
> continue;
>
> data->csd.func(data->csd.info);
>
> - spin_lock(&data->lock);
> cpu_clear(cpu, data->cpumask);
> - WARN_ON(data->refs == 0);
> - data->refs--;
> - refs = data->refs;
> - spin_unlock(&data->lock);
> -
> - if (refs)
> + if (!atomic_dec_and_test(&data->refs))
> continue;
>
> spin_lock(&queue->lock);
> @@ -379,10 +371,9 @@
> slowpath = 1;
> }
>
> - spin_lock_init(&data->lock);
> data->csd.func = func;
> data->csd.info = info;
> - data->refs = num_cpus;
> + atomic_set(&data->refs, num_cpus);
> data->cpumask = mask;
>
> spin_lock_irqsave(&queue->lock, flags);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/