io_uring: fix progress and error handling

[xhernandez]

The io_uring system calls don't return the error in errno, but directly in the returned value from the system call as a negative value. This patch correctly handles potential error codes.

Additionally, the way how previous io_uring implementation was checking for progress in the completion queue was racy and could generate false positives, making the program believe that there were no progress when in fact there was.

Fixes: #4104

[xhernandez]

/run regression

[gluster-ant]

CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format

index 7c68236c8..ac332b92d 100644
--- a/libglusterfs/src/gf-io-uring.c
+++ b/libglusterfs/src/gf-io-uring.c
@@ -35,17 +35,20 @@
 #define GF_IO_URING_FLAG_TIMER GF_IO_ID_FLAG_1
 
 /* Helper macro to define names of bits. */
-#define GF_IO_BITNAME(_prefix, _name) { _prefix##_##_name, #_name }
+#define GF_IO_BITNAME(_prefix, _name)                                          \
+    {                                                                          \
+        _prefix##_##_name, #_name                                              \
+    }
 
 /* We use the last padding word to store the number of committed SQEs. Since
  * the size of the padding may change between version, we compute the index
  * of the last one. */
-#define GF_IO_URING_SQE_PAD_IDX \
+#define GF_IO_URING_SQE_PAD_IDX                                                \
     (CAA_ARRAY_SIZE(((struct io_uring_sqe *)0)->__pad2) - 1)
 
 /* This accesses the padding word of a given SQE (referenced by index) that
  * contains the number of committed SQEs. */
-#define GF_IO_URING_SQE_COUNT(_idx) \
+#define GF_IO_URING_SQE_COUNT(_idx)                                            \
     (gf_io_uring.sq.sqes[(_idx)].__pad2[GF_IO_URING_SQE_PAD_IDX])
 
 /* Structure to keep names of bits. */
@@ -157,8 +160,7 @@ gf_io_uring_dump_params(struct io_uring_params *params)
         GF_IO_BITNAME(IORING_SETUP, CLAMP),
         GF_IO_BITNAME(IORING_SETUP, ATTACH_WQ),
         GF_IO_BITNAME(IORING_SETUP, R_DISABLED),
-        {}
-    };
+        {}};
     static gf_io_uring_bitname_t feature_names[] = {
         GF_IO_BITNAME(IORING_FEAT, SINGLE_MMAP),
         GF_IO_BITNAME(IORING_FEAT, NODROP),
@@ -170,13 +172,11 @@ gf_io_uring_dump_params(struct io_uring_params *params)
         GF_IO_BITNAME(IORING_FEAT, SQPOLL_NONFIXED),
         GF_IO_BITNAME(IORING_FEAT, EXT_ARG),
         GF_IO_BITNAME(IORING_FEAT, NATIVE_WORKERS),
-        {}
-    };
+        {}};
 
     char names[256];
 
-    GF_LOG_D("io", "io_uring settings", 4,
-             GLFS_U32(sqes, params->sq_entries),
+    GF_LOG_D("io", "io_uring settings", 4, GLFS_U32(sqes, params->sq_entries),
              GLFS_U32(cqes, params->cq_entries),
              GLFS_U32(cpu, params->sq_thread_cpu),
              GLFS_U32(idle, params->sq_thread_idle));
@@ -229,8 +229,7 @@ gf_io_uring_dump_ops(struct io_uring_probe *probe)
         [IORING_OP_TEE] = "TEE",
         [IORING_OP_SHUTDOWN] = "SHUTDOWN",
         [IORING_OP_RENAMEAT] = "RENAMEAT",
-        [IORING_OP_UNLINKAT] = "UNLINKAT"
-    };
+        [IORING_OP_UNLINKAT] = "UNLINKAT"};
 
     char names[4096];
     char *ptr;
@@ -266,8 +265,9 @@ gf_io_uring_mmap(void **ring, uint32_t fd, size_t size, off_t offset)
     void *ptr;
     int32_t res;
 
-    res = gf_res_ptr(&ptr, mmap(NULL, size, PROT_READ | PROT_WRITE,
-                                MAP_SHARED | MAP_POPULATE, fd, offset),
+    res = gf_res_ptr(&ptr,
+                     mmap(NULL, size, PROT_READ | PROT_WRITE,
+                          MAP_SHARED | MAP_POPULATE, fd, offset),
                      MAP_FAILED);
     if (caa_unlikely(res < 0)) {
         return gf_check("io", GF_LOG_ERROR, "mmap", res);
@@ -328,9 +328,9 @@ gf_io_uring_cq_init(uint32_t fd, struct io_uring_params *params)
 static void
 gf_io_uring_sq_fini(void)
 {
-    gf_check("io", GF_LOG_WARNING, "munmap",
-             gf_res_errno0(munmap(gf_io_uring.sq.sqes,
-                                  gf_io_uring.sq.sqes_size)));
+    gf_check(
+        "io", GF_LOG_WARNING, "munmap",
+        gf_res_errno0(munmap(gf_io_uring.sq.sqes, gf_io_uring.sq.sqes_size)));
     gf_check("io", GF_LOG_WARNING, "munmap",
              gf_res_errno0(munmap(gf_io_uring.sq.ring, gf_io_uring.sq.size)));
 }
@@ -874,5 +874,4 @@ const gf_io_engine_t gf_io_engine_io_uring = {
     .flush = gf_io_uring_flush,
 
     .cancel = gf_io_uring_cancel,
-    .callback = gf_io_uring_callback
-};
+    .callback = gf_io_uring_callback};

[stale[bot]]

Thank you for your contributions. Noticed that this issue is not having any activity in last ~6 months! We are marking this issue as stale because it has not had recent activity. It will be closed in 2 weeks if no one responds with a comment here.

[stale[bot]]

Closing this issue as there was no update since my last update on issue. If this is an issue which is still valid, feel free to open it.

[geiseri]

@xhernandez did this patch actually fix the issue? Could it be applied?