A push makes the node the new tail, and then links it to its predecessor.
In between the queue isn't empty, but mpscq_pop() can't get at the node.
The consumers deal with that by trying again, with a cond_resched() in
the loop to give a preempted producer the chance to finish.

cond_resched() doesn't do that if the consumer has a higher priority than
the producer and both have to use the same CPU:

  producer (SCHED_OTHER)              consumer (SCHED_FIFO), same CPU
  io_req_local_work_add()
    mpscq_push()
      prev = xchg(&q->tail, node)
  <preempted>                         io_cqring_wait()
                                        __io_run_local_work()
                                      again:
                                          cond_resched()  // nothing to do
                                          mpscq_pop()     // NULL
                                          // work pending, no events yet
                                          goto again

The producer can be any task that is able to queue task_work for the
ring: an io-wq worker, or somebody who sends a message with
IORING_OP_MSG_RING. The consumer spins, holding ->uring_lock, until the
scheduler takes pity on the fair class, which takes about a second with
the default settings of the fair server, and forever without it. The
loops in tctx_task_work_run() and io_cancel_local_task_work() have the
same problem.

Don't allow preemption between the two steps. That bounds the time the
window is open on the CPU of the producer by what interrupts take, and a
consumer that waits for that is on another CPU. The cost is a
preempt_disable()/preempt_enable() pair per push.

Fixes: 50cb44bd0d5f ("io_uring/mpscq: add lockless multi-producer, single-consumer FIFO queue")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 io_uring/mpscq.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/io_uring/mpscq.h b/io_uring/mpscq.h
index f910526766fd..da9474497cc8 100644
--- a/io_uring/mpscq.h
+++ b/io_uring/mpscq.h
@@ -25,9 +25,10 @@
  * its predecessor. In between, the tail end of the queue is not yet reachable
  * from the head. mpscq_pop() detects this and returns NULL, while mpscq_empty()
  * reports false. The consumer must not treat such a NULL as "queue empty" - it
- * should retry later. The window is two instructions wide, but a producer can
- * be preempted inside it, so the consumer must not spin on it while holding
- * resources the producer might need to make progress.
+ * should retry later. The window is two instructions wide and the producer
+ * can't be preempted inside it, but it can be interrupted, so the consumer must
+ * not spin on it while holding resources the producer might need to make
+ * progress.
  *
  * The consumer side only supports a single consumer at a time, callers must
  * provide their own serialization for it. The stub node is what allows the
@@ -69,6 +70,12 @@ static inline bool mpscq_push(struct mpscq *q, struct llist_node *node)
 	struct llist_node *prev;
 
 	node->next = NULL;
+	/*
+	 * All the consumer can do about a node that isn't linked yet is to try
+	 * again. If it is what preempted us, and we don't get to run as long
+	 * as it does, that never ends.
+	 */
+	preempt_disable();
 	/*
 	 * xchg() implies a full barrier, so the initialization of the
 	 * entry (including ->next above) is visible before the node can
@@ -77,6 +84,7 @@ static inline bool mpscq_push(struct mpscq *q, struct llist_node *node)
 	 */
 	prev = xchg(&q->tail, node);
 	WRITE_ONCE(prev->next, node);
+	preempt_enable();
 	return prev == &q->stub;
 }
 

-- 
2.55.0