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

[Kanchan Joshi]

On Mon, Mar 27, 2023 at 8:59 PM Keith Busch <kbusch at kernel.org> wrote:
>
> On Mon, Mar 27, 2023 at 07:28:10PM +0530, Kanchan Joshi wrote:
> > > -   }
> > > +   if (blk_rq_is_poll(req))
> > > +           WRITE_ONCE(ioucmd->cookie, req);
> >
> > blk_rq_is_poll(req) warns for null "req->bio" and returns false if that
> > is the case. That defeats one of the purpose of the series i.e. poll on
> > no-payload commands such as flush/write-zeroes.
>
> Sorry, I'm sending out various patches piecemeal. This patch here depends on
> this one sent out earlier:
>
>   https://lore.kernel.org/linux-block/3f670ca7-908d-db55-3da1-4090f116005d@nvidia.com/T/#mbc6174ce3f9dbae38ae2ca646518be4bf105f6e4

That clears it up, thanks.

> > >     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.