[PATCH 0/3] Improve const and reserved command handling

[PATCH 0/3] Improve const and reserved command handling

[Bart Van Assche]

Hi Martin,

This patch series includes three cleanup patches for the SCSI core that do not
modify any functionality. Please consider these patches for the next merge
window.

Thanks,

Bart.

Bart Van Assche (3):
  scsi: core: Make scsi_cmd_to_rq() accept const arguments
  scsi: core: Make scsi_cmd_priv() accept const arguments
  scsi: core: Use scsi_cmd_priv() instead of open-coding it

 drivers/scsi/scsi_lib.c     |  2 +-
 drivers/scsi/scsi_logging.c | 10 +++++-----
 include/scsi/scsi_cmnd.h    | 17 +++++++++--------
 3 files changed, 15 insertions(+), 14 deletions(-)

[PATCH 1/3] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[Bart Van Assche]

Instead of requiring the caller to cast away constness, make
scsi_cmd_to_rq() accept const arguments.

Cc: Hannes Reinecke <hare@suse.de>
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_logging.c | 10 +++++-----
 include/scsi/scsi_cmnd.h    |  9 +++++----
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c
index b02af340c2d3..5aaff629b999 100644
--- a/drivers/scsi/scsi_logging.c
+++ b/drivers/scsi/scsi_logging.c
@@ -28,7 +28,7 @@ static void scsi_log_release_buffer(char *bufptr)
 
 static inline const char *scmd_name(const struct scsi_cmnd *scmd)
 {
-	struct request *rq = scsi_cmd_to_rq((struct scsi_cmnd *)scmd);
+	const struct request *rq = scsi_cmd_to_rq(scmd);
 
 	if (!rq->q || !rq->q->disk)
 		return NULL;
@@ -94,7 +94,7 @@ void scmd_printk(const char *level, const struct scsi_cmnd *scmd,
 	if (!logbuf)
 		return;
 	off = sdev_format_header(logbuf, logbuf_len, scmd_name(scmd),
-				 scsi_cmd_to_rq((struct scsi_cmnd *)scmd)->tag);
+				 scsi_cmd_to_rq(scmd)->tag);
 	if (off < logbuf_len) {
 		va_start(args, fmt);
 		off += vscnprintf(logbuf + off, logbuf_len - off, fmt, args);
@@ -374,8 +374,8 @@ EXPORT_SYMBOL(__scsi_print_sense);
 void scsi_print_sense(const struct scsi_cmnd *cmd)
 {
 	scsi_log_print_sense(cmd->device, scmd_name(cmd),
-			     scsi_cmd_to_rq((struct scsi_cmnd *)cmd)->tag,
-			     cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
+			     scsi_cmd_to_rq(cmd)->tag, cmd->sense_buffer,
+			     SCSI_SENSE_BUFFERSIZE);
 }
 EXPORT_SYMBOL(scsi_print_sense);
 
@@ -393,7 +393,7 @@ void scsi_print_result(const struct scsi_cmnd *cmd, const char *msg,
 		return;
 
 	off = sdev_format_header(logbuf, logbuf_len, scmd_name(cmd),
-				 scsi_cmd_to_rq((struct scsi_cmnd *)cmd)->tag);
+				 scsi_cmd_to_rq(cmd)->tag);
 
 	if (off >= logbuf_len)
 		goto out_printk;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 8ecfb94049db..154fbb39ca0c 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -144,10 +144,11 @@ struct scsi_cmnd {
 };
 
 /* Variant of blk_mq_rq_from_pdu() that verifies the type of its argument. */
-static inline struct request *scsi_cmd_to_rq(struct scsi_cmnd *scmd)
-{
-	return blk_mq_rq_from_pdu(scmd);
-}
+#define scsi_cmd_to_rq(scmd)                                       \
+	_Generic(scmd,                                             \
+		const struct scsi_cmnd *: (const struct request *) \
+			blk_mq_rq_from_pdu((void *)scmd),          \
+		struct scsi_cmnd *: blk_mq_rq_from_pdu((void *)scmd))
 
 /*
  * Return the driver private allocation behind the command.

Re: [PATCH 1/3] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[John Garry]

On 24/06/2025 22:05, Bart Van Assche wrote:
> Instead of requiring the caller to cast away constness, make
> scsi_cmd_to_rq() accept const arguments.
> 
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: John Garry <john.g.garry@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   drivers/scsi/scsi_logging.c | 10 +++++-----
>   include/scsi/scsi_cmnd.h    |  9 +++++----
>   2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c

Is there something special about the logging code that it even requires 
that const scsi_cmnd * be used?

Or will it be encouraged to use const scsi_cmnd * elsewhere in future 
(after this change)? Or, further than that, convert all scsi core code 
to use const scsi_cmnd * (when possible)?

> index b02af340c2d3..5aaff629b999 100644
> --- a/drivers/scsi/scsi_logging.c
> +++ b/drivers/scsi/scsi_logging.c
> @@ -28,7 +28,7 @@ static void scsi_log_release_buffer(char *bufptr)
>   
>   static inline const char *scmd_name(const struct scsi_cmnd *scmd)
>   {
> -	struct request *rq = scsi_cmd_to_rq((struct scsi_cmnd *)scmd);
> +	const struct request *rq = scsi_cmd_to_rq(scmd);
>   
>   	if (!rq->q || !rq->q->disk)
>   		return NULL;
> @@ -94,7 +94,7 @@ void scmd_printk(const char *level, const struct scsi_cmnd *scmd,
>   	if (!logbuf)
>   		return;
>   	off = sdev_format_header(logbuf, logbuf_len, scmd_name(scmd),
> -				 scsi_cmd_to_rq((struct scsi_cmnd *)scmd)->tag);
> +				 scsi_cmd_to_rq(scmd)->tag);
>   	if (off < logbuf_len) {
>   		va_start(args, fmt);
>   		off += vscnprintf(logbuf + off, logbuf_len - off, fmt, args);
> @@ -374,8 +374,8 @@ EXPORT_SYMBOL(__scsi_print_sense);
>   void scsi_print_sense(const struct scsi_cmnd *cmd)
>   {
>   	scsi_log_print_sense(cmd->device, scmd_name(cmd),
> -			     scsi_cmd_to_rq((struct scsi_cmnd *)cmd)->tag,
> -			     cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
> +			     scsi_cmd_to_rq(cmd)->tag, cmd->sense_buffer,
> +			     SCSI_SENSE_BUFFERSIZE);
>   }
>   EXPORT_SYMBOL(scsi_print_sense);
>   
> @@ -393,7 +393,7 @@ void scsi_print_result(const struct scsi_cmnd *cmd, const char *msg,
>   		return;
>   
>   	off = sdev_format_header(logbuf, logbuf_len, scmd_name(cmd),
> -				 scsi_cmd_to_rq((struct scsi_cmnd *)cmd)->tag);
> +				 scsi_cmd_to_rq(cmd)->tag);
>   
>   	if (off >= logbuf_len)
>   		goto out_printk;
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 8ecfb94049db..154fbb39ca0c 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -144,10 +144,11 @@ struct scsi_cmnd {
>   };
>   
>   /* Variant of blk_mq_rq_from_pdu() that verifies the type of its argument. */
> -static inline struct request *scsi_cmd_to_rq(struct scsi_cmnd *scmd)
> -{
> -	return blk_mq_rq_from_pdu(scmd);
> -}
> +#define scsi_cmd_to_rq(scmd)                                       \
> +	_Generic(scmd,                                             \
> +		const struct scsi_cmnd *: (const struct request *) \
> +			blk_mq_rq_from_pdu((void *)scmd),          \
> +		struct scsi_cmnd *: blk_mq_rq_from_pdu((void *)scmd))
>   
>   /*
>    * Return the driver private allocation behind the command.

Re: [PATCH 1/3] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[Bart Van Assche]

On 6/26/25 1:36 AM, John Garry wrote:
> Is there something special about the logging code that it even requires 
> that const scsi_cmnd * be used?

No. Declaring pointers 'const' helps to make the intention of code more
clear to human readers.

> Or will it be encouraged to use const scsi_cmnd * elsewhere in future 
> (after this change)? Or, further than that, convert all scsi core code 
> to use const scsi_cmnd * (when possible)?
Many kernel developers don't care about declaring pointers 'const' even
if these can be declared 'const'. Hence, a large scale change that
changes struct scsi_cmnd pointer to const pointers would be considered
controversial.

Thanks,

Bart.

Re: [PATCH 1/3] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[John Garry]

On 26/06/2025 16:40, Bart Van Assche wrote:
> n 6/26/25 1:36 AM, John Garry wrote:
>> Is there something special about the logging code that it even 
>> requires that const scsi_cmnd * be used?
> 
> No. Declaring pointers 'const' helps to make the intention of code more
> clear to human readers.
> 
>> Or will it be encouraged to use const scsi_cmnd * elsewhere in future 
>> (after this change)? Or, further than that, convert all scsi core code 
>> to use const scsi_cmnd * (when possible)?
> Many kernel developers don't care about declaring pointers 'const' even
> if these can be declared 'const'. Hence, a large scale change that
> changes struct scsi_cmnd pointer to const pointers would be considered
> controversial.

I don't have a strong feeling on this change either way, but I'd be more 
inclined to get rid of the const usage in the logging code.  But, again, 
I don't have a strong feeling either way.

Re: [PATCH 1/3] scsi: core: Make scsi_cmd_to_rq() accept const arguments

[Martin K. Petersen]

John,

> I don't have a strong feeling on this change either way, but I'd be
> more inclined to get rid of the const usage in the logging code.

I agree. Don't really see the benefit.

-- 
Martin K. Petersen

[PATCH 2/3] scsi: core: Make scsi_cmd_priv() accept const arguments

[Bart Van Assche]

Instead of requiring the caller to cast away constness, make
scsi_cmd_priv() accept const arguments.

Cc: Hannes Reinecke <hare@suse.de>
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/scsi/scsi_cmnd.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 154fbb39ca0c..09176b07e891 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -154,10 +154,10 @@ struct scsi_cmnd {
  * Return the driver private allocation behind the command.
  * Only works if cmd_size is set in the host template.
  */
-static inline void *scsi_cmd_priv(struct scsi_cmnd *cmd)
-{
-	return cmd + 1;
-}
+#define scsi_cmd_priv(cmd)                                         \
+	_Generic(cmd,                                              \
+		const struct scsi_cmnd *: (const void *)(cmd + 1), \
+		struct scsi_cmnd *: (void *)(cmd + 1))
 
 void scsi_done(struct scsi_cmnd *cmd);
 void scsi_done_direct(struct scsi_cmnd *cmd);

Re: [PATCH 2/3] scsi: core: Make scsi_cmd_priv() accept const arguments

[John Garry]

On 24/06/2025 22:05, Bart Van Assche wrote:
> Instead of requiring the caller to cast away constness, make
> scsi_cmd_priv() accept const arguments.

Are there even instances where we are casting away const (for calling 
this function)? I assume not, since there are no changes to get rid of 
the casting away const.

> 
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: John Garry <john.g.garry@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>   include/scsi/scsi_cmnd.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 154fbb39ca0c..09176b07e891 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -154,10 +154,10 @@ struct scsi_cmnd {
>    * Return the driver private allocation behind the command.
>    * Only works if cmd_size is set in the host template.
>    */
> -static inline void *scsi_cmd_priv(struct scsi_cmnd *cmd)
> -{
> -	return cmd + 1;
> -}
> +#define scsi_cmd_priv(cmd)                                         \
> +	_Generic(cmd,                                              \
> +		const struct scsi_cmnd *: (const void *)(cmd + 1), \
> +		struct scsi_cmnd *: (void *)(cmd + 1))
>   
>   void scsi_done(struct scsi_cmnd *cmd);
>   void scsi_done_direct(struct scsi_cmnd *cmd);

Re: [PATCH 2/3] scsi: core: Make scsi_cmd_priv() accept const arguments

[Bart Van Assche]

On 6/27/25 5:08 AM, John Garry wrote:
> On 24/06/2025 22:05, Bart Van Assche wrote:
>> Instead of requiring the caller to cast away constness, make
>> scsi_cmd_priv() accept const arguments.
> 
> Are there even instances where we are casting away const (for calling 
> this function)? I assume not, since there are no changes to get rid of 
> the casting away const.

I have a patch pending that would need to cast away constness without 
this patch. See also
https://lore.kernel.org/linux-scsi/20250403211937.2225615-1-bvanassche@acm.org/

Bart.

[PATCH 3/3] scsi: core: Use scsi_cmd_priv() instead of open-coding it

[Bart Van Assche]

Improve code readability without modifying the behavior of the code.

Cc: Hannes Reinecke <hare@suse.de>
Cc: John Garry <john.g.garry@oracle.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 144c72f0737a..0c65ecfedfbd 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1843,7 +1843,7 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
 	 * a function to initialize that data.
 	 */
 	if (shost->hostt->cmd_size && !shost->hostt->init_cmd_priv)
-		memset(cmd + 1, 0, shost->hostt->cmd_size);
+		memset(scsi_cmd_priv(cmd), 0, shost->hostt->cmd_size);
 
 	if (!(req->rq_flags & RQF_DONTPREP)) {
 		ret = scsi_prepare_cmd(req);

Re: [PATCH 3/3] scsi: core: Use scsi_cmd_priv() instead of open-coding it

[John Garry]

On 24/06/2025 22:05, Bart Van Assche wrote:
> Improve code readability without modifying the behavior of the code.
> 
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: John Garry <john.g.garry@oracle.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: John Garry <john.g.garry@oracle.com>

Do you even need to preceding patch for this change? cmd is not a 
pointer to const in this function AFAICS


> ---
>   drivers/scsi/scsi_lib.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 144c72f0737a..0c65ecfedfbd 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1843,7 +1843,7 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
>   	 * a function to initialize that data.
>   	 */
>   	if (shost->hostt->cmd_size && !shost->hostt->init_cmd_priv)
> -		memset(cmd + 1, 0, shost->hostt->cmd_size);
> +		memset(scsi_cmd_priv(cmd), 0, shost->hostt->cmd_size);
>   
>   	if (!(req->rq_flags & RQF_DONTPREP)) {
>   		ret = scsi_prepare_cmd(req);

Re: [PATCH 3/3] scsi: core: Use scsi_cmd_priv() instead of open-coding it

[Bart Van Assche]

On 6/26/25 1:34 AM, John Garry wrote:
> Do you even need to preceding patch for this change? cmd is not a 
> pointer to const in this function AFAICS

Hi John,

This patch is unrelated to the previous patches. This patch series
includes the cleanup patches from a larger series. I'm splitting up
the larger series to make it easier to review the patches.

Thanks,

Bart.

Re: [PATCH 3/3] scsi: core: Use scsi_cmd_priv() instead of open-coding it

[Martin K. Petersen]

Bart,

> Improve code readability without modifying the behavior of the code.

Applied to 6.17/scsi-staging, thanks!

-- 
Martin K. Petersen

Re: [PATCH 0/3] Improve const and reserved command handling

[John Garry]

On 24/06/2025 22:05, Bart Van Assche wrote:
> Hi Martin,
> 
> This patch series includes three cleanup patches for the SCSI core that do not
> modify any functionality. Please consider these patches for the next merge
> window.
> 

Hi Bart,

I seem to remember seeing some of these patches before. Am I right?

If so, what's the history? Has anything changed (in this series)? Were 
RB tags dropped or not picked up?

Thanks,
John

> Thanks,
> 
> Bart.
> 
> Bart Van Assche (3):
>    scsi: core: Make scsi_cmd_to_rq() accept const arguments
>    scsi: core: Make scsi_cmd_priv() accept const arguments
>    scsi: core: Use scsi_cmd_priv() instead of open-coding it
> 
>   drivers/scsi/scsi_lib.c     |  2 +-
>   drivers/scsi/scsi_logging.c | 10 +++++-----
>   include/scsi/scsi_cmnd.h    | 17 +++++++++--------
>   3 files changed, 15 insertions(+), 14 deletions(-)
> 
> 

Re: [PATCH 0/3] Improve const and reserved command handling

[Bart Van Assche]

On 6/25/25 9:06 AM, John Garry wrote:
> I seem to remember seeing some of these patches before. Am I right?
> 
> If so, what's the history? Has anything changed (in this series)? Were 
> RB tags dropped or not picked up?

Hi John,

Yes, these patches have been posted before. Last time nobody replied 
with "Reviewed-by:" on any of the patches in this series, hence no
Reviewed-by tags in this patch series either. I'm splitting this patch
series because 24 patches is a bit much for a single patch series:
https://lore.kernel.org/linux-scsi/20250403211937.2225615-1-bvanassche@acm.org/

Other than rebasing on top of Martin's latest for-next branch, no
changes have been made in any of the three patches in this patch series.

Thanks,

Bart.

Re: (subset) [PATCH 0/3] Improve const and reserved command handling

[Martin K. Petersen]

On Tue, 24 Jun 2025 14:05:37 -0700, Bart Van Assche wrote:

> This patch series includes three cleanup patches for the SCSI core that do not
> modify any functionality. Please consider these patches for the next merge
> window.
> 
> Thanks,
> 
> Bart.
> 
> [...]

Applied to 6.17/scsi-queue, thanks!

[3/3] scsi: core: Use scsi_cmd_priv() instead of open-coding it
      https://git.kernel.org/mkp/scsi/c/8314312c5286

-- 
Martin K. Petersen	Oracle Linux Engineering