Re: [PATCH 02/84] scsi: core: Rename scsi_mq_done() into scsi_done() and export it

From: John Garry <john.garry@huawei.com>
To: Bart Van Assche <bvanassche@acm.org>,
	"Martin K . Petersen" <martin.petersen@oracle.com>
Cc: <linux-scsi@vger.kernel.org>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>
Subject: Re: [PATCH 02/84] scsi: core: Rename scsi_mq_done() into scsi_done() and export it
Date: Mon, 20 Sep 2021 10:42:28 +0100	[thread overview]
Message-ID: <ee1a1197-1fd1-e9d9-6b45-79108d56830b@huawei.com> (raw)
In-Reply-To: <20210918000607.450448-3-bvanassche@acm.org>

On 18/09/2021 01:04, Bart Van Assche wrote:
> Since the removal of the legacy block layer there is only one completion
> function left in the SCSI core, namely scsi_mq_done(). Rename it into
> scsi_done().

It would be nice to remove "mq" from other symbol naming in scsi_lib.c 
as well then. Could be a separate job, I suppose.

Export that function to allow SCSI LLDs to call it directly.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/scsi/scsi_lib.c  | 5 +++--
>   include/scsi/scsi_cmnd.h | 2 ++
>   2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index ba6d748a0246..c3a0283dbff0 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1575,7 +1575,7 @@ static blk_status_t scsi_prepare_cmd(struct request *req)
>   	return scsi_cmd_to_driver(cmd)->init_command(cmd);
>   }
>   
> -static void scsi_mq_done(struct scsi_cmnd *cmd)
> +void scsi_done(struct scsi_cmnd *cmd)
>   {
>   	switch (cmd->submitter) {
>   	case BLOCK_LAYER:
> @@ -1593,6 +1593,7 @@ static void scsi_mq_done(struct scsi_cmnd *cmd)
>   	trace_scsi_dispatch_cmd_done(cmd);
>   	blk_mq_complete_request(scsi_cmd_to_rq(cmd));
>   }
> +EXPORT_SYMBOL(scsi_done);
>   
>   static void scsi_mq_put_budget(struct request_queue *q, int budget_token)
>   {
> @@ -1692,7 +1693,7 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
>   
>   	scsi_set_resid(cmd, 0);
>   	memset(cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
> -	cmd->scsi_done = scsi_mq_done;
> +	cmd->scsi_done = scsi_done;

I have gone to the end of the series, and we still set 
scsi_cmnd.scsi_done. So some drivers still rely on it. I thought that 
the idea was that we don't need this callback pointer any longer.

As an aside, this is the current declaration of scsi_cmnd.scsi_done:

/* Low-level done function - can be used by low-level driver to point
*        to completion function.  Not used by mid/upper level code. */
	void (*scsi_done) (struct scsi_cmnd *);

That does not sound right, as scsi_done is set by the mid-layer.

>   
>   	blk_mq_start_request(req);
>   	reason = scsi_dispatch_cmd(cmd);
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 365d47a66c18..5b230d06527f 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -172,6 +172,8 @@ static inline struct scsi_driver *scsi_cmd_to_driver(struct scsi_cmnd *cmd)
>   	return *(struct scsi_driver **)rq->rq_disk->private_data;
>   }
>   
> +void scsi_done(struct scsi_cmnd *cmd);
> +
>   extern void scsi_finish_command(struct scsi_cmnd *cmd);
>   
>   extern void *scsi_kmap_atomic_sg(struct scatterlist *sg, int sg_count,
> .
> 