Clearly the code doesn't handle failure here or in the other XXX places. I think two of them (this and the logpage command) are both due to the AER handler? That is, when submitting the initial back of AER requests during attach, it's ok to use M_WAITOK here, but presumably when an AER command is completed, the handler needs to fetch log pages and then submit a new AER request and that is where the M_NOWAIT requirement comes from?

Yes, I believe request completion handlers are not sleepable, so calls via nvme_ctrlr_async_event_log_page_cb() and nvme_ctrlr_async_event_cb() must not sleep.

FWIW, for nvmf(4) I use a deferred task on a taskqueue to handle AER completions (specifically fetching the log page and re-submitting a new AER command)

(I had this tagged, but didn't submit it)

I was thinking we need to do the same thing here.

And we need to have comments about the XXX not just XXX

Why is this non sleepable?

Why is this unsleepable?

Why is this unsleepable (though maybe we use it for recovery from an interrupt context... which maybe we should defer to a thread/workqueue)

like jhb said: this is to get the log page from aer completions
It will be needed in the future too for some enhanced error reporting that we're contemplating.

This one should be more specific: "Currently called from AER completion processing, which is a non sleepable context"

The reason for this one is different: It's because we do reset from non-sleepable context and abort commands as part of that.