[PATCH 3/5] nvme: move result handling into nvme_execute_rq()

[Hannes Reinecke]

On 2/13/23 10:59, Sagi Grimberg wrote:
> 
> 
> On 2/9/23 16:38, Hannes Reinecke wrote:
>> and simplify the calling convention.
>>
>> Signed-off-by: Hannes Reinecke <hare at suse.de>
>> ---
>>   drivers/nvme/host/core.c       | 16 ++++++++++------
>>   drivers/nvme/host/ioctl.c      |  5 +++--
>>   drivers/nvme/host/nvme.h       |  3 ++-
>>   drivers/nvme/target/passthru.c |  3 +--
>>   4 files changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 78da9c6cbba8..11faebe87764 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -1004,15 +1004,21 @@ EXPORT_SYMBOL_GPL(nvme_setup_cmd);
>>    * >0: nvme controller's cqe status response
>>    * <0: kernel error in lieu of controller response
>>    */
>> -int nvme_execute_rq(struct request *rq, bool at_head)
>> +int nvme_execute_rq(struct request *rq, union nvme_result *result,
>> +            bool at_head)
>>   {
>>       blk_status_t status;
>> +    int ret;
>>       status = blk_execute_rq(rq, at_head);
>>       if (nvme_req(rq)->flags & NVME_REQ_CANCELLED)
>>           return -EINTR;
>> -    if (nvme_req(rq)->status)
>> -        return nvme_req(rq)->status;
>> +    ret = nvme_req(rq)->status;
>> +    if (ret) {
>> +        if (result && ret >= 0)
> 
> How can ret be < 0 here?
> 
> Why not always take the result, unconditionally?
> 
Yeah, right. Will be fixing it up.

> Is this really necessary though?
> 
??
Yes, it is necessary to return the status (if there is one).
Quite some callers expect that.
And no, it's probably not necessary to check if there is a status;
we might always return a status here. But for the result I guess we
should be checking the status first; I'm slightly hesitant to blindly
return the result even if we have a zero status.

Cheers,

Hannes