[PATCH V7 1/1] nvme: allow passthru cmd error logging

[Chaitanya Kulkarni]

Sagi/Christoph,

On 9/19/23 09:02, alan.adamson at oracle.com wrote:
>
> On 9/12/23 4:39 AM, Sagi Grimberg wrote:
>>
>>>>>       if (req->q->queuedata)
>>>>>           req->timeout = NVME_IO_TIMEOUT;
>>>>> -    else /* no queuedata implies admin queue */
>>>>> +    else { /* no queuedata implies admin queue */
>>>>>           req->timeout = NVME_ADMIN_TIMEOUT;
>>>>> +        if (!nr->ctrl->passthru_err_log_enabled)
>>>>> +            req->rq_flags |= RQF_QUIET;
>>>>> +    }
>>>>>       /* passthru commands should let the driver set the SGL flags */
>>>>>       cmd->common.flags &= ~NVME_CMD_SGL_ALL;
>>>>> @@ -687,8 +720,7 @@ void nvme_init_request(struct request *req, 
>>>>> struct nvme_command *cmd)
>>>>>       if (req->mq_hctx->type == HCTX_TYPE_POLL)
>>>>>           req->cmd_flags |= REQ_POLLED;
>>>>>       nvme_clear_nvme_request(req);
>>>>> -    req->rq_flags |= RQF_QUIET;
>>>> Isn't thisw now dropping the RQF_QUIET flag I/O commands that we
>>>> previously set?  While we're at it, why do we only allow passthrough
>>>> error logging for admin commands anyway?
>>>
>>> While working through this, we decided that for IO commands 
>>> (passthrough or not) error logging would always be enabled. For 
>>> passthrough admin commands, error logging could be enabled or disabled.
>>
>> Why shouldn't it apply to both?
>
> Currently, we have /sys/class/nvme/nvme0/passthru_err_log_enabled that 
> enables/disables logging for admin command only.  Are you saying it 
> should also enable/disable for IO commands across all namespaces or 
> should there also be:
>
> /sys/class/nvme/nvme0/nvme0n1/passthru_err_log_enabled
>
> /sys/class/nvme/nvme0/nvme0n2/passthru_err_log_enabled
>
> .
>
> .
>
> Alan
>
>
>

Can we please have resolution on this patch so we can merge it ?

-ck