Re: [PATCH] io_uring/io-wq: Fix a small time window for reading work->flags

[Pavel Begunkov <asml.silence@gmail.com>]

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.

> 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@huawei.com>
> ---
>   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