On 2/25/2025 6:45 PM, Paul E. McKenney wrote: > 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. ;-) Yeah true, thanks! - Joel
