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

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

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?

							Thanx, Paul