Re: [PATCH v2 2/4] sched: Introduce is_pcpu_safe()

From: Boqun Feng
Date: Tue Aug 10 2021 - 08:49:31 EST


On Sun, Aug 08, 2021 at 05:15:20PM +0100, Valentin Schneider wrote:
> On 07/08/21 03:42, Mike Galbraith wrote:
> > On Sat, 2021-08-07 at 01:58 +0100, Valentin Schneider wrote:
> >>
> >> +static inline bool is_pcpu_safe(void)
> >
> > Nit: seems odd to avoid spelling it out to save two characters, percpu
> > is word like, rolls off the ole tongue better than p-c-p-u.
> >
> > -Mike
>
> True. A quick grep says both versions are used, though "percpu" wins by
> about a factor of 2. I'll tweak that for a v3.

I wonder why is_percpu_safe() is the correct name. The safety of
accesses to percpu variables means two things to me:

a) The thread cannot migrate to other CPU in the middle of
accessing a percpu variable, in other words, the following
cannot happen:

{ percpu variable X is 0 on CPU 0 and 2 on CPU 1
CPU 0 CPU 1
======== =========
<in thread A>
__this_cpu_inc(X);
tmp = X; // tmp is 0
<preempted>
<migrate to CPU 1>
// continue __this_cpu_inc(X);
X = tmp + 1; // CPU 0 miss this
// increment (this
// may be OK), and
// CPU 1's X got
// corrupted.

b) The accesses to a percpu variable are exclusive, i.e. no
interrupt or preemption can happen in the middle of accessing,
in other words, the following cannot happen:

{ percpu variable X is 0 on CPU 0 }
CPU 0
========
<in thread A>
__this_cpu_inc(X);
tmp = X; // tmp is 0
<preempted>
<in other thread>
this_cpu_inc(X); // X is 1 afterwards.
<back to thread A>
X = tmp + 1; // X is 1, and we have a race condition.

And the is_p{er}cpu_safe() only detects the first, and it doesn't mean
totally safe for percpu accesses.

Maybe we can implement a migratable()? Although not sure it's a English
word.

Regards,
Boqun