Re: [PATCH] io_uring: reset -EBUSY error when io sq thread is waken up

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



hi,

On 5/19/20 10:40 PM, Xiaoguang Wang wrote:
In io_sq_thread(), currently if we get an -EBUSY error, we will
won't clear it again, which will result in io_sq_thread() will
never have a chance to submit sqes again. Below test program test.c
can reveal this bug:

int main(int argc, char *argv[])
{
         struct io_uring ring;
         int i, fd, ret;
         struct io_uring_sqe *sqe;
         struct io_uring_cqe *cqe;
         struct iovec *iovecs;
         void *buf;
         struct io_uring_params p;

         if (argc < 2) {
                 printf("%s: file\n", argv[0]);
                 return 1;
         }

         memset(&p, 0, sizeof(p));
         p.flags = IORING_SETUP_SQPOLL;
         ret = io_uring_queue_init_params(4, &ring, &p);
         if (ret < 0) {
                 fprintf(stderr, "queue_init: %s\n", strerror(-ret));
                 return 1;
         }

         fd = open(argv[1], O_RDONLY | O_DIRECT);
         if (fd < 0) {
                 perror("open");
                 return 1;
         }

         iovecs = calloc(10, sizeof(struct iovec));
         for (i = 0; i < 10; i++) {
                 if (posix_memalign(&buf, 4096, 4096))
                         return 1;
                 iovecs[i].iov_base = buf;
                 iovecs[i].iov_len = 4096;
         }

         ret = io_uring_register_files(&ring, &fd, 1);
         if (ret < 0) {
                 fprintf(stderr, "%s: register %d\n", __FUNCTION__, ret);
                 return ret;
         }

         for (i = 0; i < 10; i++) {
                 sqe = io_uring_get_sqe(&ring);
                 if (!sqe)
                         break;

                 io_uring_prep_readv(sqe, 0, &iovecs[i], 1, 0);
                 sqe->flags |= IOSQE_FIXED_FILE;

                 ret = io_uring_submit(&ring);
                 sleep(1);
                 printf("submit %d\n", i);
         }

         for (i = 0; i < 10; i++) {
                 io_uring_wait_cqe(&ring, &cqe);
                 printf("receive: %d\n", i);
                 if (cqe->res != 4096) {
                         fprintf(stderr, "ret=%d, wanted 4096\n", cqe->res);
                         ret = 1;
                 }
                 io_uring_cqe_seen(&ring, cqe);
         }

         close(fd);
         io_uring_queue_exit(&ring);
         return 0;
}
sudo ./test testfile
above command will hang on the tenth request, to fix this bug, when io
sq_thread is waken up, we reset the variable 'ret' to be zero.

Signed-off-by: Xiaoguang Wang <xiaoguang.wang@xxxxxxxxxxxxxxxxx>
---
  fs/io_uring.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 6e51140a5722..747de5cdf38a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -6045,6 +6045,8 @@ static int io_sq_thread(void *data)
  				finish_wait(&ctx->sqo_wait, &wait);
ctx->rings->sq_flags &= ~IORING_SQ_NEED_WAKEUP;
+				if (ret == -EBUSY)
+					ret = 0;
  				continue;
  			}
  			finish_wait(&ctx->sqo_wait, &wait);

How about just unconditionally clearing it? There's nothing we
need to preserve at this point, it should just get set to zero
regardless of the value.
OK, agree, will send a new version soon, thanks.

Regards,
Xiaoguang Wang




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux