search

joshkan / nvme-uring-pt

4 stars 0 forks source link

Async ioctl does not respect IO_URING_F_NONBLOCK flag #2

Open clay-mayers opened 2 years ago

clay-mayers commented 2 years ago

While reviewing the patch for Kioxia, I noticed what I believe is an issue in https://github.com/joshkan/nvme-uring-pt/blob/main/v6/v6-0010-nvme-wire-up-support-for-async-passthru-on-char-d.patch.

nvme_ns_chr_async_cmd() has the io uring command flags as a parameter but doesn't pass them to nvme_ns_async_ioctl(). If the flag IO_URING_F_NONBLOCK is set (which is is often with the 5.10 kernel), I think the call to nvme_alloc_request() in nvme_submit_user_cmd() needs to pass in BLK_MQ_REQ_NOWAIT as the flags parameter.

The call to blk_mq_alloc_request() made by nvme_alloc_request() will block when out of tags or the queue is being frozen (e.g. when an admin command causes an enumeration of the namespaces).

If that's all you did, this pushes the responsibility on to the uring client to handle -EBUSY (when the queues are being frozen) and -EAGAIN (when out of tags) to retry the request. A more complex solution would move to a blocking context and retry - but then is that really an async ioctl if you're using a thread?

joshkan commented 2 years ago

Yes, this part is still unhandled. Will take care of it in the next version.