[PATCH v2] io_uring/io-wq: fix incorrect io_wq_for_each_worker() termination logic

[PATCH v2] io_uring/io-wq: fix incorrect io_wq_for_each_worker() termination logic

[Jens Axboe]

A previous commit added this helper, and had it terminate if false is
returned from the handler. However, that is completely opposite, it
should abort the loop if true is returned.

Fix this up by having io_wq_for_each_worker() keep iterating as long
as false is returned, and only abort if true is returned.

Cc: stable@vger.kernel.org
Fixes: 751eedc4b4b7 ("io_uring/io-wq: move worker lists to struct io_wq_acct")
Signed-off-by: Jens Axboe <axboe@kernel.dk>

---

v2: fix the actual bug, rather than work-around it for the exit
    condition only.

diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c
index cd13d8aac3d2..6c5ef629e59a 100644
--- a/io_uring/io-wq.c
+++ b/io_uring/io-wq.c
@@ -952,11 +952,11 @@ static bool io_wq_for_each_worker(struct io_wq *wq,
 				  void *data)
 {
 	for (int i = 0; i < IO_WQ_ACCT_NR; i++) {
-		if (!io_acct_for_each_worker(&wq->acct[i], func, data))
-			return false;
+		if (io_acct_for_each_worker(&wq->acct[i], func, data))
+			return true;
 	}
 
-	return true;
+	return false;
 }
 
 static bool io_wq_worker_wake(struct io_worker *worker, void *data)

-- 
Jens Axboe

Re: [PATCH v2] io_uring/io-wq: fix incorrect io_wq_for_each_worker() termination logic

[Gabriel Krisman Bertazi]

Jens Axboe <axboe@kernel.dk> writes:

> A previous commit added this helper, and had it terminate if false is
> returned from the handler. However, that is completely opposite, it
> should abort the loop if true is returned.
>
> Fix this up by having io_wq_for_each_worker() keep iterating as long
> as false is returned, and only abort if true is returned.

The fix is good, but the API is just weird.

io_acct_for_each_worker returning true indicates an error that will
abort the wq walk.  It is a non-issue, since all the two callers cannot
fail and always return false for success :-)

Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>


> Cc: stable@vger.kernel.org
> Fixes: 751eedc4b4b7 ("io_uring/io-wq: move worker lists to struct io_wq_acct")
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>
> ---
>
> v2: fix the actual bug, rather than work-around it for the exit
>     condition only.
>
> diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c
> index cd13d8aac3d2..6c5ef629e59a 100644
> --- a/io_uring/io-wq.c
> +++ b/io_uring/io-wq.c
> @@ -952,11 +952,11 @@ static bool io_wq_for_each_worker(struct io_wq *wq,
>  				  void *data)
>  {
>  	for (int i = 0; i < IO_WQ_ACCT_NR; i++) {
> -		if (!io_acct_for_each_worker(&wq->acct[i], func, data))
> -			return false;
> +		if (io_acct_for_each_worker(&wq->acct[i], func, data))
> +			return true;
>  	}
>  
> -	return true;
> +	return false;
>  }
>  
>  static bool io_wq_worker_wake(struct io_worker *worker, void *data)

-- 
Gabriel Krisman Bertazi

Re: [PATCH v2] io_uring/io-wq: fix incorrect io_wq_for_each_worker() termination logic

[Jens Axboe]

On 1/5/26 11:07 AM, Gabriel Krisman Bertazi wrote:
> Jens Axboe <axboe@kernel.dk> writes:
> 
>> A previous commit added this helper, and had it terminate if false is
>> returned from the handler. However, that is completely opposite, it
>> should abort the loop if true is returned.
>>
>> Fix this up by having io_wq_for_each_worker() keep iterating as long
>> as false is returned, and only abort if true is returned.
> 
> The fix is good, but the API is just weird.
> 
> io_acct_for_each_worker returning true indicates an error that will
> abort the wq walk.  It is a non-issue, since all the two callers cannot
> fail and always return false for success :-)
> 
> Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>

Thanks for taking a look! Was going to post a followup just killing
the return value of io_wq_for_each_worker() to void as nobody uses
it. Only the cancelation side with its matching will use a true
return, most of iterations want to iterate all of them.

-- 
Jens Axboe

Re: [PATCH v2] io_uring/io-wq: fix incorrect io_wq_for_each_worker() termination logic

[David Kahurani]

FOSS is generally in perfect condition for rip off :-)

On Mon, Jan 5, 2026 at 9:41 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 1/5/26 11:07 AM, Gabriel Krisman Bertazi wrote:
> > Jens Axboe <axboe@kernel.dk> writes:
> >
> >> A previous commit added this helper, and had it terminate if false is
> >> returned from the handler. However, that is completely opposite, it
> >> should abort the loop if true is returned.
> >>
> >> Fix this up by having io_wq_for_each_worker() keep iterating as long
> >> as false is returned, and only abort if true is returned.
> >
> > The fix is good, but the API is just weird.
> >
> > io_acct_for_each_worker returning true indicates an error that will
> > abort the wq walk.  It is a non-issue, since all the two callers cannot
> > fail and always return false for success :-)
> >
> > Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de>
>
> Thanks for taking a look! Was going to post a followup just killing
> the return value of io_wq_for_each_worker() to void as nobody uses
> it. Only the cancelation side with its matching will use a true
> return, most of iterations want to iterate all of them.
>
> --
> Jens Axboe
>