Re: [PATCH 3/7] nvmet: allow async passthrough of commands that change logical block contents

Chaitanya Kulkarni
2022-12-14T04:53:00Z

On 12/13/22 08:24, Christoph Hellwig wrote:
> Mask out the logical block change effect to allow to keep supporting
> passthrough for Write commands once I/O Command effects are properly
> propagated.
>
> Fixes: c1fef73f793b ("nvmet: add passthru code to process commands")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/nvme/target/passthru.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
> index 79af5140af8bfe..ad80ba61c42236 100644
> --- a/drivers/nvme/target/passthru.c
> +++ b/drivers/nvme/target/passthru.c
> @@ -295,7 +295,6 @@ static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
> struct nvme_ns *ns = NULL;
> struct request *rq = NULL;
> unsigned int timeout;
> - u32 effects;
> u16 status;
> int ret;
>
> @@ -334,14 +333,16 @@ static void nvmet_passthru_execute_cmd(struct nvmet_req *req)
> }
>
> /*
> - * If there are effects for the command we are about to execute, or
> - * an end_req function we need to use nvme_execute_passthru_rq()
> - * synchronously in a work item seeing the end_req function and
> - * nvme_passthru_end() can't be called in the request done callback
> - * which is typically in interrupt context.
> + * If there are any non-trivial effects for the command we are about to
> + * execute, call nvme_execute_passthru_rq in a workqueue, so that the
> + * effects can be properly handled by the calls to nvme_passthru_start
> + * and nvme_passthru_end.
> */
> - effects = nvme_command_effects(ctrl, ns, req->cmd->common.opcode);
> - if (req->p.use_workqueue || effects) {
> + if (nvme_command_effects(ctrl, ns, req->cmd->common.opcode) &
> + ~NVME_CMD_EFFECTS_LBCC)
> + req->p.use_workqueue = true;
> +

How about this instead of calling function in if which is much cleaner
unless it has a bug ?

effect = nvme_command_effects(ctrl, ns, req->cmd->common.opcode);

req->p.use_workqueue = effetcs & ~NVME_CMD_EFFECTS_LBCC;

irrespective of that, looks good :-

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck