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

[alan.adamson at oracle.com]

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