On Tue, Feb 25, 2025 at 05:04:37PM -0500, Joel Fernandes wrote:
>
>
> On 2/25/2025 4:53 PM, Paul E. McKenney wrote:
> > On Tue, Feb 25, 2025 at 04:20:07PM -0500, Joel Fernandes wrote:
> >> On Tue, Feb 25, 2025 at 09:54:29AM -0800, Paul E. McKenney wrote:
> >>> On Tue, Feb 25, 2025 at 08:11:11AM -0800, Boqun Feng wrote:
> >> [...]
> >>>>>> These passed other than a KCSAN complaint involving
> >>>>>> rcu_preempt_deferred_qs_handler() and rcu_read_unlock_special().
> >>>>>> This looks like the plain C-language writes to ->defer_qs_iw_pending.
> >>>>>>
> >>>>>> My guess is that this is low probability, despite having happened
> >>>>>> twice,
> >>>>>> and that it happens when rcu_read_unlock_special() is interrupted,
> >>>>>> resulting in rcu_preempt_deferred_qs_handler() being invoked as an
> >>>>>> IRQ-work handler. Keeping in mind that RCU runs KCSAN so as to locate
> >>>>>> data races between task and handler on the same CPU.
> >>>>>>
> >>>>>> Thoughts?
> >>>>>>
> >>>>>
> >>>>> Do you have a KCSAN of this? Also this is not a regression, right?
> >>>>> Meaning you probably have seen this before? Anyway, it should be an easy
> >>>>> fix (just using READ_ONCE() and WRITE_ONCE()). I can send the fix out
> >>>>> and put it in.
> >>>
> >>> Here you go! And you are right, if it is a regression, it is from a
> >>> long time ago, though something more recent might have made it more
> >>> probable.
> >>
> >> In my opinion I probably wouldn't even call it a regression because the
> >> data-race is happening on a boolean element. If I am not mistaken, this is
> >> thus a false-positive and KCSAN has no way of silencing it?
> >
> > You can still get in trouble with booleans. The usual example
> > is as follows:
> >
> > bool x;
> >
> > ...
> >
> >
> > while (!x)
> > do_something();
> >
> > In many cases, the compiler is free to transform that "while" loop
> > into this:
> >
> > if (!x)
> > for (;;)
> > do_something();
> >
> > Putting a READ_ONCE() in the original "while" condition prevents this
> > transformation.
>
> True, thanks for clarifying. I will be a bit more annoying and say that in
> rcu_read_unlock_special(), there is no such looping transformation possible
> though AFAICS. The test is an if() block. But this is beyond KCSAN's ability
> to
> analyze I guess.
Besides, better safe than sorry. Especially given the decades-long
trend of increasingly clever compilers. ;-)
Thanx, Paul
