Re: [PATCH] nvme: ignore starting sector while error logging for passthrough requests

[Pankaj Raghav <p.raghav@samsung.com>]

The final consensus in Daniel's patch[1] seems to be adding an opt in to
log error only for admin commands, and logging error for user IO commands
will remain as is.

In that case, should we reconsider this patch to avoid printing a garbage
value ((sector_t)-1) while logging error for IO passthrough commands?

[1]
https://lore.kernel.org/all/79E937D0-58DA-4AE2-A0B8-A30185CE7FCE@oracle.com/

On 2022-10-06 11:10, Pankaj Raghav wrote:
> Error log will print a garbage value as the starting LBA for passthrough
> requests and requests with uninitialized starting sector that return an
> error. Print LBA as 0 instead for these requests. This behaviour is
> similar what tracing does for these type of requests.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>  drivers/nvme/host/core.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 059737c1a2c1..41e0a257f1c2 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -305,6 +305,13 @@ static void nvme_retry_req(struct request *req)
>  	blk_mq_delay_kick_requeue_list(req->q, delay);
>  }
>  
> +static inline u64 nvme_log_error_rq_lba(struct nvme_ns *ns, struct request *rq)
> +{
> +	if (blk_rq_is_passthrough(rq) || blk_rq_pos(rq) == (sector_t)-1)
> +		return 0;
> +	return nvme_sect_to_lba(ns, blk_rq_pos(rq));
> +}
> +
>  static void nvme_log_error(struct request *req)
>  {
>  	struct nvme_ns *ns = req->q->queuedata;
> @@ -315,7 +322,7 @@ static void nvme_log_error(struct request *req)
>  		       ns->disk ? ns->disk->disk_name : "?",
>  		       nvme_get_opcode_str(nr->cmd->common.opcode),
>  		       nr->cmd->common.opcode,
> -		       (unsigned long long)nvme_sect_to_lba(ns, blk_rq_pos(req)),
> +		       (unsigned long long)nvme_log_error_rq_lba(ns, req),
>  		       (unsigned long long)blk_rq_bytes(req) >> ns->lba_shift,
>  		       nvme_get_error_status_str(nr->status),
>  		       nr->status >> 8 & 7,	/* Status Code Type */