Re: [PATCH -v4] generic-ipi: remove kmalloc()

Oleg Nesterov
2009-02-17T20:06:35Z

On 02/17, Peter Zijlstra wrote:
>
> + list_for_each_entry_rcu(data, &call_function.queue, csd.list) {
> + void (*func)(void *);
> + void *info;
> + int refs, wait;
>
> spin_lock(&data->lock);
> - cpumask_clear_cpu(cpu, to_cpumask(data->cpumask_bits));
> + if (!cpumask_test_cpu(cpu, data->cpumask)) {
> + spin_unlock(&data->lock);
> + continue;
> + }
> + cpumask_clear_cpu(cpu, data->cpumask);
> WARN_ON(data->refs == 0);
> - data->refs--;
> - refs = data->refs;
> + refs = --data->refs;
> + func = data->csd.func;
> + info = data->csd.info;
> + wait = (data->csd.flags & CSD_FLAG_WAIT);
> spin_unlock(&data->lock);
>
> - if (refs)
> - continue;
> + if (!refs) {
> + spin_lock(&call_function.lock);
> + list_del_rcu(&data->csd.list);
> + spin_unlock(&call_function.lock);
> + csd_unlock(&data->csd);
> + }
>
> - spin_lock(&call_function_lock);
> - list_del_rcu(&data->csd.list);
> - spin_unlock(&call_function_lock);
> + func(info);
>
> - if (data->csd.flags & CSD_FLAG_WAIT) {
> - /*
> - * serialize stores to data with the flag clear
> - * and wakeup
> - */
> - smp_wmb();
> - data->csd.flags &= ~CSD_FLAG_WAIT;
> - }
> - if (data->csd.flags & CSD_FLAG_ALLOC)
> - call_rcu(&data->rcu_head, rcu_free_call_data);
> + if (!refs && wait)
> + csd_complete(&data->csd);

Argh! Sorry-sorry-sorry. I managed to confused myself and you.

Please return to the previous version. With this patch we do:

func(info);

if (!refs && wait)
csd_complete();

but we can't do csd_complete(), we can not be sure other CPUs
has already passed this function call.

Of course we can add another counter, but I don't think this
worth the trouble.

As for the previous version. I am almost sure I missed something
again, but do we really need both CSD_FLAG_WAIT and CSD_FLAG_LOCK
flags? smp_call_function_many(wait => 1) can just wait for
!CSD_FLAG_LOCK.

Oleg.