On Thu, Dec 16, 2021 at 08:45:46AM -0700, Jens Axboe wrote: > On 12/16/21 2:08 AM, Christoph Hellwig wrote: > > On Wed, Dec 15, 2021 at 09:24:21AM -0700, Jens Axboe wrote: > >> + spin_lock(&nvmeq->sq_lock); > >> + while (!rq_list_empty(*rqlist)) { > >> + struct request *req = rq_list_pop(rqlist); > >> + struct nvme_iod *iod = blk_mq_rq_to_pdu(req); > >> + > >> + memcpy(nvmeq->sq_cmds + (nvmeq->sq_tail << nvmeq->sqes), > >> + absolute_pointer(&iod->cmd), sizeof(iod->cmd)); > >> + if (++nvmeq->sq_tail == nvmeq->q_depth) > >> + nvmeq->sq_tail = 0; > > > > So this doesn't even use the new helper added in patch 2? I think this > > should call nvme_sq_copy_cmd(). > > But you NAK'ed that one? It definitely should use that helper, so I take it > you are fine with it then if we do it here too? That would make 3 call sites, > and I still do think the helper makes sense... I explained two times that the new helpers is fine as long as you open code nvme_submit_cmd in its two callers as it now is a trivial wrapper.