[PATCH liburing 0/5] overflow support

[PATCH liburing 0/5] overflow support

[Dylan Yudaken]

This series adds more support overflow conditions, including testing what happens when an overflow CQE is not able to be allocated.

Patches 1,2,4:
 - clean up existing documentation to account for updated kernel behaviour
Patch 3:
 - exposes some API to allow applications to inspect overflow state
Patch 5:
 - test new overflow API and new kernel error signalling

Dylan Yudaken (5):
  fix documentation shortenings
  update io_uring_enter.2 docs for IORING_FEAT_NODROP
  expose CQ ring overflow state
  add docs for overflow lost errors
  overflow: add tests

 man/io_uring_enter.2            |  24 +++-
 man/io_uring_setup.2            |  11 +-
 man/io_uring_wait_cqe.3         |   2 +-
 man/io_uring_wait_cqe_nr.3      |   2 +-
 man/io_uring_wait_cqe_timeout.3 |   2 +-
 man/io_uring_wait_cqes.3        |   2 +-
 src/include/liburing.h          |  10 ++
 src/queue.c                     |  31 +++--
 test/cq-overflow.c              | 240 +++++++++++++++++++++++++++++++-
 9 files changed, 298 insertions(+), 26 deletions(-)


base-commit: b7d8dd8bbf5b8550c8a0c1ed70431cd8050709f0
-- 
2.30.2

[PATCH liburing 1/5] fix documentation shortenings

[Dylan Yudaken]

'parm' probably was intended to be param, but parameter is more explicit,
so change it.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 man/io_uring_wait_cqe.3         | 2 +-
 man/io_uring_wait_cqe_nr.3      | 2 +-
 man/io_uring_wait_cqe_timeout.3 | 2 +-
 man/io_uring_wait_cqes.3        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/man/io_uring_wait_cqe.3 b/man/io_uring_wait_cqe.3
index 1190d9b..676c4bc 100644
--- a/man/io_uring_wait_cqe.3
+++ b/man/io_uring_wait_cqe.3
@@ -30,6 +30,6 @@ the application can retrieve the completion with
 .SH RETURN VALUE
 On success
 .BR io_uring_wait_cqe (3)
-returns 0 and the cqe_ptr parm is filled in. On failure it returns -errno.
+returns 0 and the cqe_ptr parameter is filled in. On failure it returns -errno.
 .SH SEE ALSO
 .BR io_uring_submit (3),  io_uring_wait_cqes(3)
diff --git a/man/io_uring_wait_cqe_nr.3 b/man/io_uring_wait_cqe_nr.3
index 3594b0b..7d4a56f 100644
--- a/man/io_uring_wait_cqe_nr.3
+++ b/man/io_uring_wait_cqe_nr.3
@@ -33,6 +33,6 @@ the application can retrieve the completion with
 .SH RETURN VALUE
 On success
 .BR io_uring_wait_cqe_nr (3)
-returns 0 and the cqe_ptr parm is filled in. On failure it returns -errno.
+returns 0 and the cqe_ptr parameter is filled in. On failure it returns -errno.
 .SH SEE ALSO
 .BR io_uring_submit (3),  io_uring_wait_cqes (3)
diff --git a/man/io_uring_wait_cqe_timeout.3 b/man/io_uring_wait_cqe_timeout.3
index 64e920b..f535433 100644
--- a/man/io_uring_wait_cqe_timeout.3
+++ b/man/io_uring_wait_cqe_timeout.3
@@ -34,6 +34,6 @@ calling io_uring_wait_cqes (3).
 .SH RETURN VALUE
 On success
 .BR io_uring_wait_cqes (3)
-returns 0 and the cqe_ptr parm is filled in. On failure it returns -errno.
+returns 0 and the cqe_ptr parameter is filled in. On failure it returns -errno.
 .SH SEE ALSO
 .BR io_uring_submit (3),  io_uring_wait_cqe_timeout (3), io_uring_wait_cqe(3).
diff --git a/man/io_uring_wait_cqes.3 b/man/io_uring_wait_cqes.3
index a9a4a0c..2010a7b 100644
--- a/man/io_uring_wait_cqes.3
+++ b/man/io_uring_wait_cqes.3
@@ -41,6 +41,6 @@ calling io_uring_wait_cqes (3).
 .SH RETURN VALUE
 On success
 .BR io_uring_wait_cqes (3)
-returns 0 and the cqe_ptr parm is filled in. On failure it returns -errno.
+returns 0 and the cqe_ptr parameter is filled in. On failure it returns -errno.
 .SH SEE ALSO
 .BR io_uring_submit (3),  io_uring_wait_cqe_timeout (3), io_uring_wait_cqe(3).
-- 
2.30.2

[PATCH liburing 2/5] update io_uring_enter.2 docs for IORING_FEAT_NODROP

[Dylan Yudaken]

The EBUSY docs are out of date, so update them for the IORING_FEAT_NODROP
feature flag

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 man/io_uring_enter.2 | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/man/io_uring_enter.2 b/man/io_uring_enter.2
index 22dbbd5..3112355 100644
--- a/man/io_uring_enter.2
+++ b/man/io_uring_enter.2
@@ -1327,7 +1327,18 @@ is a valid file descriptor, but the io_uring ring is not in the right state
 for details on how to enable the ring.
 .TP
 .B EBUSY
-The application is attempting to overcommit the number of requests it can have
+If the
+.B IORING_FEAT_NODROP
+feature flag is set, then
+.B EBUSY
+will be returned if there were overflow entries,
+.B IORING_ENTER_GETEVENTS
+flag is set and not all of the overflow entries were able to be flushed to
+the CQ ring.
+
+Without 
+.B IORING_FEAT_NODROP
+the application is attempting to overcommit the number of requests it can have
 pending. The application should wait for some completions and try again. May
 occur if the application tries to queue more requests than we have room for in
 the CQ ring, or if the application attempts to wait for more events without
-- 
2.30.2

[PATCH liburing 3/5] expose CQ ring overflow state

[Dylan Yudaken]

Allow the application to easily view if the CQ ring is in overflow state, and
also a simple method to flush overflow entries on to the CQ ring.
Explicit flushing can be useful for applications that prefer to have
reduced latency on CQs than to process as many as possible.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 src/include/liburing.h | 10 ++++++++++
 src/queue.c            | 31 +++++++++++++++++++------------
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/include/liburing.h b/src/include/liburing.h
index 5c03061..16c31a4 100644
--- a/src/include/liburing.h
+++ b/src/include/liburing.h
@@ -135,6 +135,7 @@ int io_uring_submit_and_wait_timeout(struct io_uring *ring,
 				     unsigned wait_nr,
 				     struct __kernel_timespec *ts,
 				     sigset_t *sigmask);
+int io_uring_flush_overflow(struct io_uring *ring);
 
 int io_uring_register_buffers(struct io_uring *ring, const struct iovec *iovecs,
 			      unsigned nr_iovecs);
@@ -781,6 +782,15 @@ static inline unsigned io_uring_cq_ready(const struct io_uring *ring)
 	return io_uring_smp_load_acquire(ring->cq.ktail) - *ring->cq.khead;
 }
 
+/*
+ * Returns true if there are overflow entries waiting to be flushed onto
+ * the CQ ring
+ */
+static inline bool io_uring_cq_has_overflow(const struct io_uring *ring)
+{
+	return IO_URING_READ_ONCE(*ring->sq.kflags) & IORING_SQ_CQ_OVERFLOW;
+}
+
 /*
  * Returns true if the eventfd notification is currently enabled
  */
diff --git a/src/queue.c b/src/queue.c
index 856d270..bfcf11f 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -33,14 +33,10 @@ static inline bool sq_ring_needs_enter(struct io_uring *ring, unsigned *flags)
 	return false;
 }
 
-static inline bool cq_ring_needs_flush(struct io_uring *ring)
-{
-	return IO_URING_READ_ONCE(*ring->sq.kflags) & IORING_SQ_CQ_OVERFLOW;
-}
-
 static inline bool cq_ring_needs_enter(struct io_uring *ring)
 {
-	return (ring->flags & IORING_SETUP_IOPOLL) || cq_ring_needs_flush(ring);
+	return (ring->flags & IORING_SETUP_IOPOLL) ||
+		io_uring_cq_has_overflow(ring);
 }
 
 struct get_data {
@@ -123,6 +119,21 @@ int __io_uring_get_cqe(struct io_uring *ring, struct io_uring_cqe **cqe_ptr,
 	return _io_uring_get_cqe(ring, cqe_ptr, &data);
 }
 
+static int io_uring_get_events(struct io_uring *ring)
+{
+	int flags = IORING_ENTER_GETEVENTS;
+
+	if (ring->int_flags & INT_FLAG_REG_RING)
+		flags |= IORING_ENTER_REGISTERED_RING;
+	return ____sys_io_uring_enter(ring->enter_ring_fd, 0, 0, flags, NULL);
+}
+
+int io_uring_flush_overflow(struct io_uring *ring)
+{
+	return io_uring_get_events(ring);
+}
+
+
 /*
  * Fill in an array of IO completions up to count, if any are available.
  * Returns the amount of IO completions filled.
@@ -152,12 +163,8 @@ again:
 	if (overflow_checked)
 		goto done;
 
-	if (cq_ring_needs_flush(ring)) {
-		int flags = IORING_ENTER_GETEVENTS;
-
-		if (ring->int_flags & INT_FLAG_REG_RING)
-			flags |= IORING_ENTER_REGISTERED_RING;
-		____sys_io_uring_enter(ring->enter_ring_fd, 0, 0, flags, NULL);
+	if (io_uring_cq_has_overflow(ring)) {
+		io_uring_get_events(ring);
 		overflow_checked = true;
 		goto again;
 	}
-- 
2.30.2

[PATCH liburing 4/5] add docs for overflow lost errors

[Dylan Yudaken]

Add man docs for return values indicating a CQE was lost.

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 man/io_uring_enter.2 | 11 +++++++++++
 man/io_uring_setup.2 | 11 +++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/man/io_uring_enter.2 b/man/io_uring_enter.2
index 3112355..15ef9d1 100644
--- a/man/io_uring_enter.2
+++ b/man/io_uring_enter.2
@@ -1326,6 +1326,17 @@ is a valid file descriptor, but the io_uring ring is not in the right state
 .BR io_uring_register (2)
 for details on how to enable the ring.
 .TP
+.B EBADR
+At least one CQE was dropped even with the
+.B IORING_FEAT_NODROP
+feature, and there are no otherwise available CQEs. This clears the error state
+and so with no other changes the next call to
+.BR io_uring_setup (2)
+will not have this error. This error should be extremely rare and indicates the
+machine is running critically low on memory and. It may be reasonable for the
+application to terminate running unless it is able to safely handle any CQE
+being lost.
+.TP
 .B EBUSY
 If the
 .B IORING_FEAT_NODROP
diff --git a/man/io_uring_setup.2 b/man/io_uring_setup.2
index f3911af..2f617c1 100644
--- a/man/io_uring_setup.2
+++ b/man/io_uring_setup.2
@@ -218,7 +218,7 @@ call. The SQEs must still be allocated separately. This brings the necessary
 calls down from three to two. Available since kernel 5.4.
 .TP
 .B IORING_FEAT_NODROP
-If this flag is set, io_uring supports never dropping completion events.
+If this flag is set, io_uring supports almost never dropping completion events.
 If a completion event occurs and the CQ ring is full, the kernel stores
 the event internally until such a time that the CQ ring has room for more
 entries. If this overflow condition is entered, attempting to submit more
@@ -226,7 +226,14 @@ IO will fail with the
 .B -EBUSY
 error value, if it can't flush the overflown events to the CQ ring. If this
 happens, the application must reap events from the CQ ring and attempt the
-submit again. Available since kernel 5.5.
+submit again. If the kernel has no free memory to store the event internally
+it will be visible by an increase in the overflow value on the cqring.
+Available since kernel 5.5. Additionally
+.BR io_uring_enter (2)
+will return
+.B -EBADR
+the next time it would otherwise sleep waiting for completions (since kernel 5.19).
+
 .TP
 .B IORING_FEAT_SUBMIT_STABLE
 If this flag is set, applications can be certain that any data for
-- 
2.30.2

[PATCH liburing 5/5] overflow: add tests

[Dylan Yudaken]

Add tests that verify that overflow conditions behave appropriately.
Specifically:
 * if overflow is continually flushed, then CQEs should arrive mostly in
 order to prevent starvation of some completions
 * if CQEs are dropped due to GFP_ATOMIC allocation failures it is
 possible to terminate cleanly. This is not tested by default as it
 requires debug kernel config, and also has system-wide effects

Signed-off-by: Dylan Yudaken <dylany@fb.com>
---
 test/cq-overflow.c | 240 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 233 insertions(+), 7 deletions(-)

diff --git a/test/cq-overflow.c b/test/cq-overflow.c
index 057570e..067308a 100644
--- a/test/cq-overflow.c
+++ b/test/cq-overflow.c
@@ -9,6 +9,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <fcntl.h>
+#include <assert.h>
 
 #include "helpers.h"
 #include "liburing.h"
@@ -21,6 +22,32 @@ static struct iovec *vecs;
 
 #define ENTRIES	8
 
+/*
+ * io_uring has rare cases where CQEs are lost.
+ * This happens when there is no space in the CQ ring, and also there is no
+ * GFP_ATOMIC memory available. In reality this probably means that the process
+ * is about to be killed as many other things might start failing, but we still
+ * want to test that liburing and the kernel deal with this properly. The fault
+ * injection framework allows us to test this scenario. Unfortunately this
+ * requires some system wide changes and so we do not enable this by default.
+ * The tests in this file should work in both cases (where overflows are queued
+ * and where they are dropped) on recent kernels.
+ *
+ * In order to test dropped CQEs you should enable fault injection in the kernel
+ * config:
+ *
+ * CONFIG_FAULT_INJECTION=y
+ * CONFIG_FAILSLAB=y
+ * CONFIG_FAULT_INJECTION_DEBUG_FS=y
+ *
+ * and then run the test as follows:
+ * echo Y > /sys/kernel/debug/failslab/task-filter
+ * echo 100 > /sys/kernel/debug/failslab/probability
+ * echo 0 > /sys/kernel/debug/failslab/verbose
+ * echo 100000 > /sys/kernel/debug/failslab/times
+ * bash -c "echo 1 > /proc/self/make-it-fail && exec ./cq-overflow.t"
+ */
+
 static int test_io(const char *file, unsigned long usecs, unsigned *drops, int fault)
 {
 	struct io_uring_sqe *sqe;
@@ -29,6 +56,7 @@ static int test_io(const char *file, unsigned long usecs, unsigned *drops, int f
 	unsigned reaped, total;
 	struct io_uring ring;
 	int nodrop, i, fd, ret;
+	bool cqe_dropped = false;
 
 	fd = open(file, O_RDONLY | O_DIRECT);
 	if (fd < 0) {
@@ -103,8 +131,8 @@ static int test_io(const char *file, unsigned long usecs, unsigned *drops, int f
 reap_it:
 	reaped = 0;
 	do {
-		if (nodrop) {
-			/* nodrop should never lose events */
+		if (nodrop && !cqe_dropped) {
+			/* nodrop should never lose events unless cqe_dropped */
 			if (reaped == total)
 				break;
 		} else {
@@ -112,7 +140,10 @@ reap_it:
 				break;
 		}
 		ret = io_uring_wait_cqe(&ring, &cqe);
-		if (ret) {
+		if (nodrop && ret == -EBADR) {
+			cqe_dropped = true;
+			continue;
+		} else if (ret) {
 			fprintf(stderr, "wait_cqe=%d\n", ret);
 			goto err;
 		}
@@ -132,7 +163,7 @@ reap_it:
 		goto err;
 	}
 
-	if (!nodrop) {
+	if (!nodrop || cqe_dropped) {
 		*drops = *ring.cq.koverflow;
 	} else if (*ring.cq.koverflow) {
 		fprintf(stderr, "Found %u overflows\n", *ring.cq.koverflow);
@@ -153,18 +184,31 @@ static int reap_events(struct io_uring *ring, unsigned nr_events, int do_wait)
 {
 	struct io_uring_cqe *cqe;
 	int i, ret = 0, seq = 0;
+	unsigned int start_overflow = *ring->cq.koverflow;
+	unsigned int drop_count = 0;
+	bool dropped = false;
 
 	for (i = 0; i < nr_events; i++) {
 		if (do_wait)
 			ret = io_uring_wait_cqe(ring, &cqe);
 		else
 			ret = io_uring_peek_cqe(ring, &cqe);
-		if (ret) {
+		if (do_wait && ret == -EBADR) {
+			unsigned int this_drop = *ring->cq.koverflow -
+				start_overflow;
+
+			dropped = true;
+			drop_count += this_drop;
+			start_overflow = *ring->cq.koverflow;
+			assert(this_drop > 0);
+			i += (this_drop - 1);
+			continue;
+		} else if (ret) {
 			if (ret != -EAGAIN)
 				fprintf(stderr, "cqe peek failed: %d\n", ret);
 			break;
 		}
-		if (cqe->user_data != seq) {
+		if (!dropped && cqe->user_data != seq) {
 			fprintf(stderr, "cqe sequence out-of-order\n");
 			fprintf(stderr, "got %d, wanted %d\n", (int) cqe->user_data,
 					seq);
@@ -241,19 +285,201 @@ err:
 	return 1;
 }
 
+
+static void submit_one_nop(struct io_uring *ring, int ud)
+{
+	struct io_uring_sqe *sqe;
+	int ret;
+
+	sqe = io_uring_get_sqe(ring);
+	assert(sqe);
+	io_uring_prep_nop(sqe);
+	sqe->user_data = ud;
+	ret = io_uring_submit(ring);
+	assert(ret == 1);
+}
+
+/*
+ * Create an overflow condition and ensure that SQEs are still processed
+ */
+static int test_overflow_handling(
+	bool batch,
+	int cqe_multiple,
+	bool poll)
+{
+	struct io_uring ring;
+	struct io_uring_params p;
+	int ret, i, j, ud, cqe_count;
+	unsigned int count;
+	int const N = 8;
+	int const LOOPS = 128;
+	int const QUEUE_LENGTH = 1024;
+	int completions[N];
+	int queue[QUEUE_LENGTH];
+	int queued = 0;
+	int outstanding = 0;
+	bool cqe_dropped = false;
+
+	memset(&completions, 0, sizeof(int) * N);
+	memset(&p, 0, sizeof(p));
+	p.cq_entries = 2 * cqe_multiple;
+	p.flags |= IORING_SETUP_CQSIZE;
+
+	if (poll)
+		p.flags |= IORING_SETUP_IOPOLL;
+
+	ret = io_uring_queue_init_params(2, &ring, &p);
+	if (ret) {
+		fprintf(stderr, "io_uring_queue_init failed %d\n", ret);
+		return 1;
+	}
+
+	assert(p.cq_entries < N);
+	/* submit N SQEs, some should overflow */
+	for (i = 0; i < N; i++) {
+		submit_one_nop(&ring, i);
+		outstanding++;
+	}
+
+	for (i = 0; i < LOOPS; i++) {
+		struct io_uring_cqe *cqes[N];
+
+		if (io_uring_cq_has_overflow(&ring)) {
+			/*
+			 * Flush any overflowed CQEs and process those. Actively
+			 * flush these to make sure CQEs arrive in vague order
+			 * of being sent.
+			 */
+			ret = io_uring_flush_overflow(&ring);
+			if (ret != 0) {
+				fprintf(stderr,
+					"io_uring_flush_overflow returned %d\n",
+					ret);
+				goto err;
+			}
+		} else if (!cqe_dropped) {
+			for (j = 0; j < queued; j++) {
+				submit_one_nop(&ring, queue[j]);
+				outstanding++;
+			}
+			queued = 0;
+		}
+
+		/* We have lost some random cqes, stop if no remaining. */
+		if (cqe_dropped && outstanding == *ring.cq.koverflow)
+			break;
+
+		ret = io_uring_wait_cqe(&ring, &cqes[0]);
+		if (ret == -EBADR) {
+			cqe_dropped = true;
+			fprintf(stderr, "CQE dropped\n");
+			continue;
+		} else if (ret != 0) {
+			fprintf(stderr, "io_uring_wait_cqes failed %d\n", ret);
+			goto err;
+		}
+		cqe_count = 1;
+		if (batch) {
+			ret = io_uring_peek_batch_cqe(&ring, &cqes[0], 2);
+			if (ret < 0) {
+				fprintf(stderr,
+					"io_uring_peek_batch_cqe failed %d\n",
+					ret);
+				goto err;
+			}
+			cqe_count = ret;
+		}
+		for (j = 0; j < cqe_count; j++) {
+			assert(cqes[j]->user_data < N);
+			ud = cqes[j]->user_data;
+			completions[ud]++;
+			assert(queued < QUEUE_LENGTH);
+			queue[queued++] = (int)ud;
+		}
+		io_uring_cq_advance(&ring, cqe_count);
+		outstanding -= cqe_count;
+	}
+
+	/* See if there were any drops by flushing the CQ ring *and* overflow */
+	do {
+		struct io_uring_cqe *cqe;
+
+		ret = io_uring_flush_overflow(&ring);
+		if (ret < 0) {
+			if (ret == -EBADR) {
+				fprintf(stderr, "CQE dropped\n");
+				cqe_dropped = true;
+				break;
+			}
+			goto err;
+		}
+		if (outstanding && !io_uring_cq_ready(&ring))
+			ret = io_uring_wait_cqe_timeout(&ring, &cqe, NULL);
+
+		if (ret && ret != -ETIME) {
+			if (ret == -EBADR) {
+				fprintf(stderr, "CQE dropped\n");
+				cqe_dropped = true;
+				break;
+			}
+			fprintf(stderr, "wait_cqe_timeout = %d\n", ret);
+			goto err;
+		}
+		count = io_uring_cq_ready(&ring);
+		io_uring_cq_advance(&ring, count);
+		outstanding -= count;
+	} while (count);
+
+	io_uring_queue_exit(&ring);
+
+	/* Make sure that completions come back in the same order they were
+	 * sent. If they come back unfairly then this will concentrate on a
+	 * couple of indices.
+	 */
+	for (i = 1; !cqe_dropped && i < N; i++) {
+		if (abs(completions[i] - completions[i - 1]) > 1) {
+			fprintf(
+				stderr,
+				"bad completion size %d %d\n",
+				completions[i],
+				completions[i - 1]);
+			goto err;
+		}
+	}
+	return 0;
+err:
+	io_uring_queue_exit(&ring);
+	return 1;
+}
+
 int main(int argc, char *argv[])
 {
 	const char *fname = ".cq-overflow";
 	unsigned iters, drops;
 	unsigned long usecs;
 	int ret;
+	int i;
 
 	if (argc > 1)
 		return 0;
 
+	for (i = 0; i < 8; i++) {
+		bool batch = i & 1;
+		int mult = (i & 2) ? 1 : 2;
+		bool poll = i & 4;
+
+		ret = test_overflow_handling(batch, mult, poll);
+		if (ret) {
+			fprintf(stderr, "test_overflow_handling("
+				"batch=%d, mult=%d, poll=%d) failed\n",
+				batch, mult, poll);
+			goto err;
+		}
+	}
+
 	ret = test_overflow();
 	if (ret) {
-		printf("test_overflow failed\n");
+		fprintf(stderr, "test_overflow failed\n");
 		return ret;
 	}
 
-- 
2.30.2

Re: [PATCH liburing 5/5] overflow: add tests

[Ammar Faizi]

On 4/21/22 4:14 PM, Dylan Yudaken wrote:
> Add tests that verify that overflow conditions behave appropriately.
> Specifically:
>   * if overflow is continually flushed, then CQEs should arrive mostly in
>   order to prevent starvation of some completions
>   * if CQEs are dropped due to GFP_ATOMIC allocation failures it is
>   possible to terminate cleanly. This is not tested by default as it
>   requires debug kernel config, and also has system-wide effects
> 
> Signed-off-by: Dylan Yudaken <dylany@fb.com>
> ---

Dylan, this breaks -Werror build with clang-15.

```
   cq-overflow.c:188:15: error: variable 'drop_count' set but not used [-Werror,-Wunused-but-set-variable]
           unsigned int drop_count = 0;
                        ^
   1 error generated.
   make[1]: *** [Makefile:210: cq-overflow.t] Error 1
   make[1]: *** Waiting for unfinished jobs....
```

Maybe you miss something that you forgot to use the value of @drop_count?

-- 
Ammar Faizi

Re: [PATCH liburing 5/5] overflow: add tests

[Dylan Yudaken]

On Thu, 2022-04-21 at 18:44 +0700, Ammar Faizi wrote:
> On 4/21/22 4:14 PM, Dylan Yudaken wrote:
> > Add tests that verify that overflow conditions behave
> > appropriately.
> > Specifically:
> >   * if overflow is continually flushed, then CQEs should arrive
> > mostly in
> >   order to prevent starvation of some completions
> >   * if CQEs are dropped due to GFP_ATOMIC allocation failures it is
> >   possible to terminate cleanly. This is not tested by default as
> > it
> >   requires debug kernel config, and also has system-wide effects
> > 
> > Signed-off-by: Dylan Yudaken <dylany@fb.com>
> > ---
> 
> Dylan, this breaks -Werror build with clang-15.
> 
> ```
>    cq-overflow.c:188:15: error: variable 'drop_count' set but not
> used [-Werror,-Wunused-but-set-variable]
>            unsigned int drop_count = 0;
>                         ^
>    1 error generated.
>    make[1]: *** [Makefile:210: cq-overflow.t] Error 1
>    make[1]: *** Waiting for unfinished jobs....
> ```
> 
> Maybe you miss something that you forgot to use the value of
> @drop_count?
> 

Ah - I had some debug statements that were using it but did not remove
it when removing those (the versions of gcc/clang I have do not
complain). I will clean it up in v2.
Thanks for spotting it!

Re: [PATCH liburing 5/5] overflow: add tests

[Jens Axboe]

On 4/21/22 3:14 AM, Dylan Yudaken wrote:
> Add tests that verify that overflow conditions behave appropriately.
> Specifically:
>  * if overflow is continually flushed, then CQEs should arrive mostly in
>  order to prevent starvation of some completions
>  * if CQEs are dropped due to GFP_ATOMIC allocation failures it is
>  possible to terminate cleanly. This is not tested by default as it
>  requires debug kernel config, and also has system-wide effects

Since you're doing a v2 of this anyway, can we change:

> +/*
> + * Create an overflow condition and ensure that SQEs are still processed
> + */
> +static int test_overflow_handling(
> +	bool batch,
> +	int cqe_multiple,
> +	bool poll)
> +{

to follow the normal stye of:

static int test_overflow_handling(bool batch, int cqe_multiple,	bool poll)
{

instead?

-- 
Jens Axboe