[PATCH 2/2] nvme: use blk-mq polling for uring commands

[Keith Busch]

On Mon, Mar 27, 2023 at 10:50:47PM +0530, Kanchan Joshi wrote:
> On Mon, Mar 27, 2023 at 8:59 PM Keith Busch <kbusch at kernel.org> wrote:
> > > >     rcu_read_lock();
> > > > -   bio = READ_ONCE(ioucmd->cookie);
> > > > -   ns = container_of(file_inode(ioucmd->file)->i_cdev,
> > > > -                   struct nvme_ns, cdev);
> > > > -   q = ns->queue;
> > > > -   if (test_bit(QUEUE_FLAG_POLL, &q->queue_flags) && bio && bio->bi_bdev)
> > > > -           ret = bio_poll(bio, iob, poll_flags);
> > > > +   req = READ_ONCE(ioucmd->cookie);
> > > > +   if (req) {
> > >
> > > This is risky. We are not sure if the cookie is actually "req" at this
> > > moment.
> >
> > What else could it be? It's either a real request from a polled hctx tag, or
> > NULL at this point.
> 
> It can also be a function pointer that gets assigned on irq-driven completion.
> See the "struct io_uring_cmd" - we are tight on cacheline, so cookie
> and task_work_cb share the storage.
> 
> > It's safe to check the cookie like this and rely on its contents.
> Hence not safe. Please try running this without poll-queues (at nvme
> level), you'll see failures.

Okay, you have a iouring polling instance used with a file that has poll
capabilities, but doesn't have any polling hctx's. It would be nice to exclude
these from io_uring's polling since they're wasting CPU time, but that doesn't
look easily done. This simple patch atop should work though.

---
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 369e8519b87a2..e3ff019404816 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -612,6 +612,8 @@ static int nvme_uring_cmd_io(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 
 	if (blk_rq_is_poll(req))
 		WRITE_ONCE(ioucmd->cookie, req);
+	else if (issue_flags & IO_URING_F_IOPOLL)
+		ioucmd->flags |= IORING_URING_CMD_NOPOLL;
 
 	/* to free bio on completion, as req->bio will be null at that time */
 	pdu->bio = req->bio;
@@ -774,6 +776,9 @@ int nvme_ns_chr_uring_cmd_iopoll(struct io_uring_cmd *ioucmd,
 	struct request *req;
 	int ret = 0;
 
+	if (ioucmd->flags & IORING_URING_CMD_NOPOLL)
+		return 0;
+
 	/*
 	 * The rcu lock ensures the 'req' in the command cookie will not be
 	 * freed until after the unlock. The queue must be frozen to free the
diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
index 934e5dd4ccc08..2abf45491b3df 100644
--- a/include/linux/io_uring.h
+++ b/include/linux/io_uring.h
@@ -22,6 +22,8 @@ enum io_uring_cmd_flags {
 	IO_URING_F_IOPOLL		= (1 << 10),
 };
 
+#define IORING_URING_CMD_NOPOLL	(1U << 31)
+
 struct io_uring_cmd {
 	struct file	*file;
 	const void	*cmd;
--