Re: [patch] generic-ipi: remove kmalloc, cleanup

From: Peter Zijlstra
Date: Thu Feb 12 2009 - 10:58:12 EST


On Thu, 2009-02-12 at 10:43 -0500, Steven Rostedt wrote:
> > + data = kmalloc(sizeof(*data), GFP_ATOMIC);
> > + if (data)
> > + data->csd.flags = CSD_FLAG_ALLOC;
> > + else {
> > + data = &per_cpu(cfd_data, me);
> > + while (data->csd.flags & CSD_FLAG_LOCK)
> > + cpu_relax();
> > + data->csd.flags = CSD_FLAG_LOCK;
>
> Wont the first CPU that runs the callback unlock this? And then we run the
> risk of two back to back callers on the same CPU, having the second
> caller possibly corrupt the first.

No, there's a ref count in there that ensures the last one unlocks it.

But that's still not enough, the global queue is RCU protected.

Suppose you have 4 cpus, and use smp_function_call_mask() to 2 others,
now its possible the 4th is also doing global ipis and is traversing the
global queue.

Therefore, if you remove the cfd when its done, it might be the 4th cpu
is in it trying to iterate to the next entry --> BANG.

The solution used is RCU freeing cfd's. This also means we have to RCU
free the LOCK flag, sadly an RCU grace period is waaaay too long to spin
wait on.

Hence this whole solution is not quite feasible.

There's various alternative solutions, but I'm not quite sure which
makes most sense.

The one I'm currently pondering is using the global queue only for
all-but-self cfd's, this matches the all-but-self ipi APIC case.

For smaller masks we could queue a csd per queue and send single ipis.



--
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/