Hi,
> -----Original Message-----
> From: Pavel Begunkov <asml.silence@xxxxxxxxx>
> Sent: Wednesday, January 15, 2025 12:22 AM
> To: lizetao <lizetao1@xxxxxxxxxx>; Jens Axboe <axboe@xxxxxxxxx>
> Cc: io-uring@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] io_uring/io-wq: Fix a small time window for reading work-
> >flags
>
> On 1/14/25 02:06, lizetao wrote:
> > There is a small time window that is modified by other tasks after
> > reading work->flags. It is changed to read before use, which is more
>
> Can you elaborate on what races with what? I don't immediately see any race
> here.
There is such a race context:
worker process
io_worker_handle_work: IORING_OP_ASYNC_CANCEL
io_wq_enqueue __io_wq_worker_cancel
work_flags = atomic_read(&work->flags); // no IO_WQ_WORK_CANCEL
atomic_or(IO_WQ_WORK_CANCEL, &work->flags);
if (work_flags & IO_WQ_WORK_CANCEL) // false
There seems to be a small time window here, resulting in the latest flags not being used.
>
> > in line with the semantics of atoms.
> > Fixes: 3474d1b93f89 ("io_uring/io-wq: make io_wq_work flags atomic")
> > Signed-off-by: Li Zetao <lizetao1@xxxxxxxxxx>
> > ---
> > io_uring/io-wq.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c index
> > a38f36b68060..75096e77b1fe 100644
> > --- a/io_uring/io-wq.c
> > +++ b/io_uring/io-wq.c
> > @@ -932,7 +932,6 @@ static bool io_wq_work_match_item(struct
> io_wq_work *work, void *data)
> > void io_wq_enqueue(struct io_wq *wq, struct io_wq_work *work)
> > {
> > struct io_wq_acct *acct = io_work_get_acct(wq, work);
> > - unsigned int work_flags = atomic_read(&work->flags);
> > struct io_cb_cancel_data match = {
> > .fn = io_wq_work_match_item,
> > .data = work,
> > @@ -945,7 +944,7 @@ void io_wq_enqueue(struct io_wq *wq, struct
> io_wq_work *work)
> > * been marked as one that should not get executed, cancel it here.
> > */
> > if (test_bit(IO_WQ_BIT_EXIT, &wq->state) ||
> > - (work_flags & IO_WQ_WORK_CANCEL)) {
> > + (atomic_read(&work->flags) & IO_WQ_WORK_CANCEL)) {
> > io_run_cancel(work, wq);
> > return;
> > }
> > @@ -959,7 +958,7 @@ void io_wq_enqueue(struct io_wq *wq, struct
> io_wq_work *work)
> > do_create = !io_wq_activate_free_worker(wq, acct);
> > rcu_read_unlock();
> >
> > - if (do_create && ((work_flags & IO_WQ_WORK_CONCURRENT) ||
> > + if (do_create && ((atomic_read(&work->flags) &
> > +IO_WQ_WORK_CONCURRENT) ||
> > !atomic_read(&acct->nr_running))) {
> > bool did_create;
> >
>
> --
> Pavel Begunkov
---
Li Zetao
