public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 4/9] io-uring/rw: Use accessor for hrtimer_sleeper ->task field
       [not found] <e76a426b-f65c-4357-b263-96506d877136@paulmck-laptop>
@ 2026-07-31  0:40 ` Paul E. McKenney
  2026-07-31 15:03   ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Paul E. McKenney @ 2026-07-31  0:40 UTC (permalink / raw)
  To: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner
  Cc: Peter Zijlstra (Intel), linux-kernel, kernel-team,
	Paul E. McKenney, Jens Axboe, io-uring

The hrtimer_sleeper structure's ->task field is used as a flag to indicate
that the associated hrtimer has expired.  This means that the hrtimer
handler can be storing to this field while other code is loading from it
to check for expiry.  Note that additional races appear for hrtimers that
can be restarted, which could be argued to be a user error.  However, that
is no reason to let the compiler introduce additional confusion, and to
this end, the hrtimer_sleeper_task_get() was introduced, use of which also
has the benefit of avoiding open-code access to hrtimer_sleeper innards.

Therefore, apply this accessor to the io_hybrid_iopoll_delay() function.

KCSAN located this issue.

Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Anna-Maria Behnsen <anna-maria@linutronix.de>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: <io-uring@vger.kernel.org>
---
 io_uring/rw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/rw.c b/io_uring/rw.c
index 63b6519e498cd7..4afbc3ceea0bb4 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -1291,7 +1291,7 @@ static u64 io_hybrid_iopoll_delay(struct io_ring_ctx *ctx, struct io_kiocb *req)
 	set_current_state(TASK_INTERRUPTIBLE);
 	hrtimer_sleeper_start_expires(&timer, mode);
 
-	if (timer.task)
+	if (hrtimer_sleeper_task_get(&timer))
 		io_schedule();
 
 	hrtimer_cancel(&timer.timer);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH RFC 4/9] io-uring/rw: Use accessor for hrtimer_sleeper ->task field
  2026-07-31  0:40 ` [PATCH RFC 4/9] io-uring/rw: Use accessor for hrtimer_sleeper ->task field Paul E. McKenney
@ 2026-07-31 15:03   ` Jens Axboe
  2026-07-31 17:42     ` Paul E. McKenney
  0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2026-07-31 15:03 UTC (permalink / raw)
  To: Paul E. McKenney, Anna-Maria Behnsen, Frederic Weisbecker,
	Thomas Gleixner
  Cc: Peter Zijlstra (Intel), linux-kernel, kernel-team, io-uring

On 7/30/26 6:40 PM, Paul E. McKenney wrote:
> The hrtimer_sleeper structure's ->task field is used as a flag to indicate
> that the associated hrtimer has expired.  This means that the hrtimer
> handler can be storing to this field while other code is loading from it
> to check for expiry.  Note that additional races appear for hrtimers that
> can be restarted, which could be argued to be a user error.  However, that
> is no reason to let the compiler introduce additional confusion, and to
> this end, the hrtimer_sleeper_task_get() was introduced, use of which also
> has the benefit of avoiding open-code access to hrtimer_sleeper innards.
> 
> Therefore, apply this accessor to the io_hybrid_iopoll_delay() function.

Seems fine to me, but for some reason you didn't CC all the people on
the series. Hence I don't know if each can stand on its own. A quick
grep tells me that, no, that is not the case, as
hrtimer_sleeper_task_get() isn't a thing in the tree right now.

That's a slight nudge to please don't do partial CCs, it's impossible to
tell without needing to dig further.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH RFC 4/9] io-uring/rw: Use accessor for hrtimer_sleeper ->task field
  2026-07-31 15:03   ` Jens Axboe
@ 2026-07-31 17:42     ` Paul E. McKenney
  0 siblings, 0 replies; 3+ messages in thread
From: Paul E. McKenney @ 2026-07-31 17:42 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Anna-Maria Behnsen, Frederic Weisbecker, Thomas Gleixner,
	Peter Zijlstra (Intel), linux-kernel, kernel-team, io-uring

On Fri, Jul 31, 2026 at 09:03:29AM -0600, Jens Axboe wrote:
> On 7/30/26 6:40 PM, Paul E. McKenney wrote:
> > The hrtimer_sleeper structure's ->task field is used as a flag to indicate
> > that the associated hrtimer has expired.  This means that the hrtimer
> > handler can be storing to this field while other code is loading from it
> > to check for expiry.  Note that additional races appear for hrtimers that
> > can be restarted, which could be argued to be a user error.  However, that
> > is no reason to let the compiler introduce additional confusion, and to
> > this end, the hrtimer_sleeper_task_get() was introduced, use of which also
> > has the benefit of avoiding open-code access to hrtimer_sleeper innards.
> > 
> > Therefore, apply this accessor to the io_hybrid_iopoll_delay() function.
> 
> Seems fine to me, but for some reason you didn't CC all the people on
> the series. Hence I don't know if each can stand on its own. A quick
> grep tells me that, no, that is not the case, as
> hrtimer_sleeper_task_get() isn't a thing in the tree right now.
> 
> That's a slight nudge to please don't do partial CCs, it's impossible to
> tell without needing to dig further.

Good point, and apologies!

However, in this case I would have overrun the CC limit.  Would it suffice
to just add all the lists (in your case, "Cc: <io-uring@vger.kernel.org>")
on all the commits?

							Thanx, Paul

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-31 17:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <e76a426b-f65c-4357-b263-96506d877136@paulmck-laptop>
2026-07-31  0:40 ` [PATCH RFC 4/9] io-uring/rw: Use accessor for hrtimer_sleeper ->task field Paul E. McKenney
2026-07-31 15:03   ` Jens Axboe
2026-07-31 17:42     ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox