On Mon, Mar 27, 2023 at 8:59 PM Keith Busch <kbusch@xxxxxxxxxx> 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@xxxxxxxxxx/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.
