diff mbox series

[tools/io_uring] tools/io_uring: correctly set "ret" for sq_poll case

Message ID 20230221073736.628851-1-ZiyangZhang@linux.alibaba.com
State New
Headers
Series [tools/io_uring] tools/io_uring: correctly set "ret" for sq_poll case |

Commit Message

Ziyang Zhang Feb. 21, 2023, 7:37 a.m. UTC 
  For sq_poll case, "ret" is not initialized or cleared/set. In this way,
output of this test program is incorrect and we can not even stop this
program by pressing CTRL-C.

Reset "ret" to zero in each submission/completion round, and assign
"ret" to "this_reap".

Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com>
---
 tools/io_uring/io_uring-bench.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jens Axboe Feb. 23, 2023, 3:46 a.m. UTC | #1 
On 2/21/23 12:37?AM, Ziyang Zhang wrote:
> For sq_poll case, "ret" is not initialized or cleared/set. In this way,
> output of this test program is incorrect and we can not even stop this
> program by pressing CTRL-C.
> 
> Reset "ret" to zero in each submission/completion round, and assign
> "ret" to "this_reap".

Can you check if this issue also exists in the fio copy of this, which
is t/io_uring.c in:

git://git.kernel.dk/fio

The copy in the kernel is pretty outdated at this point, and should
probably get removed. But if the bug is in the above main version, then
we should fix it there and then ponder if we want to remove the one in
the kernel or just get it updated to match the upstream version.
Ziyang Zhang Feb. 24, 2023, 2:25 a.m. UTC | #2 
On 2023/2/23 11:46, Jens Axboe wrote:
> On 2/21/23 12:37?AM, Ziyang Zhang wrote:
>> For sq_poll case, "ret" is not initialized or cleared/set. In this way,
>> output of this test program is incorrect and we can not even stop this
>> program by pressing CTRL-C.
>>
>> Reset "ret" to zero in each submission/completion round, and assign
>> "ret" to "this_reap".
> 
> Can you check if this issue also exists in the fio copy of this, which
> is t/io_uring.c in:
> 
> git://git.kernel.dk/fio
> 
> The copy in the kernel is pretty outdated at this point, and should
> probably get removed. But if the bug is in the above main version, then
> we should fix it there and then ponder if we want to remove the one in
> the kernel or just get it updated to match the upstream version.
> 

Hi Jens,

I have checked t/io_uring.c and the code is correct with sq_poll.

Regards,
Zhang
Jens Axboe Feb. 24, 2023, 2:29 a.m. UTC | #3 
On 2/23/23 7:25 PM, Ziyang Zhang wrote:
> On 2023/2/23 11:46, Jens Axboe wrote:
>> On 2/21/23 12:37?AM, Ziyang Zhang wrote:
>>> For sq_poll case, "ret" is not initialized or cleared/set. In this way,
>>> output of this test program is incorrect and we can not even stop this
>>> program by pressing CTRL-C.
>>>
>>> Reset "ret" to zero in each submission/completion round, and assign
>>> "ret" to "this_reap".
>>
>> Can you check if this issue also exists in the fio copy of this, which
>> is t/io_uring.c in:
>>
>> git://git.kernel.dk/fio
>>
>> The copy in the kernel is pretty outdated at this point, and should
>> probably get removed. But if the bug is in the above main version, then
>> we should fix it there and then ponder if we want to remove the one in
>> the kernel or just get it updated to match the upstream version.
>>
> 
> Hi Jens,
> 
> I have checked t/io_uring.c and the code is correct with sq_poll.

OK good, I'll attempt a sync with the kernel copy...
diff mbox series

Patch

diff --git a/tools/io_uring/io_uring-bench.c b/tools/io_uring/io_uring-bench.c
index 7703f0118385..3c0feb48f6f6 100644
--- a/tools/io_uring/io_uring-bench.c
+++ b/tools/io_uring/io_uring-bench.c
@@ -289,6 +289,7 @@  static void *submitter_fn(void *data)
 	do {
 		int to_wait, to_submit, this_reap, to_prep;
 
+		ret = 0;
 		if (!prepped && s->inflight < DEPTH) {
 			to_prep = min(DEPTH - s->inflight, BATCH_SUBMIT);
 			prepped = prep_more_ios(s, to_prep);
@@ -334,6 +335,8 @@  static void *submitter_fn(void *data)
 				this_reap += r;
 		} while (sq_thread_poll && this_reap < to_wait);
 		s->reaps += this_reap;
+		if (sq_thread_poll)
+			ret = this_reap;
 
 		if (ret >= 0) {
 			if (!ret) {