Re: percpu_foreach() does not execute remotely

[Jason Thorpe <thorpej%me.com@localhost>]

> On Jan 28, 2020, at 9:46 PM, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> wrote:
> 
>> Date: Tue, 28 Jan 2020 20:49:50 -0800
>> From: Jason Thorpe <thorpej%me.com@localhost>
>> 
>> What happens if:
>> 
>> 	oink = percpu_getref(...);
>> 	...
>> 	mutex_enter(...);	// blocks for a long time for whatever reason.
>> 	// while we're blocked, someone else does a percpu_alloc() that results
>> 	// in percpu_cpu_enlarge()?
>> 	// Isn't "oink" invalid now?
>> 	...
>> 	percpu_putref(...);
> 
> Normally sleeping under percpu_getref -- even on an adaptive lock --
> is a no-no for precisely this reason.  (percpu_getref, while very nice
> in some ways, is an extremely delicate abstraction!  It would be nice
> if violating this rule triggered an immediate panic.)

My point was about the percpu_swap_lock ... percpu_foreach() is protected from this scenario by its use of that lock, percpu_getref() is not.  Maybe we want to consider adding panics in the cases where a thread goes to sleep while preemption is disabled?  IF we did that, then we should also disable preemption after acquiring percpu_swap_lock to catch bad behavior by the percpu_foreach() callback.

> Do you mean to make it part of the contract of percpu_foreach_xcall
> that sleeping is allowed in the callback, even though it would
> generally be forbidden under percpu_getref?

No, I agree with you that it should be forbidden, but was pointing out that there is a gap in the consistency protection provided by percpu_getref(), and the fact that only one low-pri xcall can run at a time closes it in the percpu_foreach_xcall() case.  If we had strict enforcement of "no voluntary switching for any reason while preemption disabled", then it would not matter.

-- thorpej