Re: On possible data race in pollwake() / poll_schedule_timeout()

[Dmitry Antipov <dmantipov@xxxxxxxxx>]

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:

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);

        /*
         * Perform the default wake up operation using a dummy
@@ -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))
                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);

        return rc;
 }

Dmitry