Re: [PATCH V2 2/2] scsi: implement .cleanup_rq callback

Bart Van Assche
2019-07-22T15:40:28Z

On 7/19/19 8:06 PM, Ming Lei wrote:
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index e1da8c70a266..52537c145762 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -154,12 +154,9 @@ scsi_set_blocked(struct scsi_cmnd *cmd, int reason)
>
> static void scsi_mq_requeue_cmd(struct scsi_cmnd *cmd)
> {
> - if (cmd->request->rq_flags & RQF_DONTPREP) {
> - cmd->request->rq_flags &= ~RQF_DONTPREP;
> - scsi_mq_uninit_cmd(cmd);
> - } else {
> - WARN_ON_ONCE(true);
> - }
> + WARN_ON_ONCE(!(cmd->request->rq_flags & RQF_DONTPREP));
> +
> + scsi_mq_uninit_cmd(cmd);
> blk_mq_requeue_request(cmd->request, true);
> }

The above changes are independent of this patch series. Have you
considered to move these into a separate patch?

> +/*
> + * Only called when the request isn't completed by SCSI, and not freed by
> + * SCSI
> + */
> +static void scsi_cleanup_rq(struct request *rq)
> +{
> + struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
> +
> + scsi_mq_uninit_cmd(cmd);
> +}

Is the comment above this function correct? The previous patch adds an
unconditional call to mq_ops->cleanup_rq() in blk_mq_free_request().

Thanks,

Bart.