Re: [PATCH 1/10] Add generic helpers for arch IPI function calls

From: Paul E. McKenney
Date: Sat May 03 2008 - 00:49:16 EST


On Fri, May 02, 2008 at 07:21:39AM -0700, Paul E. McKenney wrote:
> On Fri, May 02, 2008 at 02:59:29PM +0200, Peter Zijlstra wrote:
> > On Fri, 2008-05-02 at 05:42 -0700, Paul E. McKenney wrote:
> >
> > > And here is one scenario that makes me doubt that my imagination is
> > > faulty:
> > >
> > > 1. CPU 0 disables irqs.
> > >
> > > 2. CPU 1 disables irqs.
> > >
> > > 3. CPU 0 invokes smp_call_function(). But CPU 1 will never respond
> > > because its irqs are disabled.
> > >
> > > 4. CPU 1 invokes smp_call_function(). But CPU 0 will never respond
> > > because its irqs are disabled.
> > >
> > > Looks like inherent deadlock to me, requiring that smp_call_function()
> > > be invoked with irqs enabled.
> > >
> > > So, what am I missing here?
> >
> > The wish to do it anyway ;-)
> >
> > I can imagine some situations where I'd like to try anyway and fall back
> > to a slower path when failing.
> >
> > With the initial design we would simply allocate data, stick it on the
> > queue and call the ipi (when needed).
> >
> > This is perfectly deadlock free when wait=0 and it just returns -ENOMEM
> > on allocation failure.
> >
> > It it doesn't return -ENOMEM I know its been queued and will be
> > processed at some point, if it does fail, I can deal with it in another
> > way.
> >
> > I know I'd like to do that and I suspect Nick has a few use cases up his
> > sleeve as well.
>
> OK, so one approach would be to check for irqs being disabled,
> perhaps as follows, on top of my previous patch:
>
> struct call_single_data *data = NULL;
>
> if (!wait) {
> data = kmalloc(sizeof(*data), GFP_ATOMIC);
> if (data)
> data->flags = CSD_FLAG_ALLOC;
> }
> if (!data) {
> if (unlikely(irqs_disabled())) {
> put_cpu();
> return -ENOMEM;
> }
> data = &d;
> data->flags = CSD_FLAG_WAIT;
> }
>
> data->func = func;
> data->info = info;
> generic_exec_single(cpu, data);
>
> That would prevent -ENOMEM unless you invoked the function with irqs
> disabled. So normal callers would still see the current failure-free
> semantics -- you really don't want to be inflicting failure when not
> necessary, right?
>
> There could only be one irq-disabled caller at a time, which could be
> handled using a trylock, returning -EBUSY if the lock is already held.
> Otherwise, you end up with the scenario called out above (which Keith
> Ownens pointed out some years ago).
>
> Does this approach make sense?

Actually, no... The irq-disabled callers would need to acquire the
spinlock -before- disabling irqs, otherwise we end up right back in
the deadlock scenario.

Thanx, Paul
--
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/