On 10/14/21 1:43 AM, Christoph Hellwig wrote:
> On Wed, Oct 13, 2021 at 10:54:13AM -0600, Jens Axboe wrote:
>> +void nvme_complete_batch_req(struct request *req)
>> +{
>> + nvme_cleanup_cmd(req);
>> + nvme_end_req_zoned(req);
>> + req->status = BLK_STS_OK;
>> +}
>> +EXPORT_SYMBOL_GPL(nvme_complete_batch_req);
>> +
>
> I'd be tempted to just merge this helper into the only caller.
> nvme_cleanup_cmd is exported anyway, so this would just add an export
> for nvme_end_req_zoned.
Sure, I can do that.
>> +static __always_inline void nvme_complete_batch(struct io_batch *iob,
>> + void (*fn)(struct request *rq))
>> +{
>> + struct request *req;
>> +
>> + req = rq_list_peek(&iob->req_list);
>> + while (req) {
>> + fn(req);
>> + nvme_complete_batch_req(req);
>> + req = rq_list_next(req);
>> + }
>> +
>> + blk_mq_end_request_batch(iob);
>
> Can we turn this into a normal for loop?
>
> for (req = rq_list_peek(&iob->req_list); req; req = rq_list_next(req)) {
> ..
> }
If you prefer it that way for nvme, for me the while () setup is much
easier to read than a really long for line.
>> + if (!nvme_try_complete_req(req, cqe->status, cqe->result)) {
>> + /*
>> + * Do normal inline completion if we don't have a batch
>> + * list, if we have an end_io handler, or if the status of
>> + * the request isn't just normal success.
>> + */
>> + if (!iob || req->end_io || nvme_req(req)->status)
>> + nvme_pci_complete_rq(req);
>> + else
>> + rq_list_add_tail(&iob->req_list, req);
>> + }
>
> The check for the conditions where we can or cannot batch complete
> really should go into a block layer helper. Something like the
> incremental patch below:
That's a good idea, I'll add that.
--
Jens Axboe
