On Wed 18-06-25 20:08:22, Dmitry Antipov wrote: > On 6/18/25 6:20 PM, Jan Kara wrote: > > > So KCSAN is really trigger-happy about issues like this. There's no > > practical issue here because it is hard to imagine how the compiler could > > compile the above code using some intermediate values stored into > > 'triggered' or multiple fetches from 'triggered'. But for the cleanliness > > of code and silencing of KCSAN your changes make sense. > > Thanks. Surely I've read Documentation/memory-barriers.txt more than > once, but, just for this particual case: is _ONCE() pair from the above > expected to work in the same way as: Firstly, I would not mess with this subtle code in unobvious ways unless necessary ;) > diff --git a/fs/select.c b/fs/select.c > index 9fb650d03d52..1a4096fd3a95 100644 > --- a/fs/select.c > +++ b/fs/select.c > @@ -191,8 +191,7 @@ static int __pollwake(wait_queue_entry_t *wait, unsigned mode, int sync, void *k > * smp_wmb() is equivalent to smp_wmb() in try_to_wake_up() > * and is paired with smp_store_mb() in poll_schedule_timeout. > */ > - smp_wmb(); > - pwq->triggered = 1; > + smp_store_release(&pwq->triggered, 1); Yes, this should be equivalent AFAICS. > @@ -237,7 +236,7 @@ static int poll_schedule_timeout(struct poll_wqueues *pwq, int state, > int rc = -EINTR; > > set_current_state(state); > - if (!pwq->triggered) > + if (!smp_load_acquire(&pwq->triggered)) set_current_state() already contains a full barrier. Thus smp_load_acquire() would add unnecessary overhead here AFAICT. > rc = schedule_hrtimeout_range(expires, slack, HRTIMER_MODE_ABS); > __set_current_state(TASK_RUNNING); > > @@ -252,7 +251,7 @@ static int poll_schedule_timeout(struct poll_wqueues *pwq, int state, > * this problem doesn't exist for the first iteration as > * add_wait_queue() has full barrier semantics. > */ > - smp_store_mb(pwq->triggered, 0); > + smp_store_release(&pwq->triggered, 0); This is IMO wrong and would need very good explanation why you think this is safe to do. Full barrier is stronger than just a RELEASE operation. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
