Re: [PATCH 4.19] scsi: sd: Fix 'sdkp' in sd_first_printk

[Damien Le Moal]

On 2022/10/13 15:26, Jason Yan wrote:
> 
> On 2022/10/13 12:49, Li kunyu wrote:
>>
>> This is defined in the 4.19 kernel:
>>
>> #define sd_printk(prefix, sdsk, fmt, a...)                              \
>>          (sdsk)->disk ?                                                  \
>>                sdev_prefix_printk(prefix, (sdsk)->device,                \
>>                                   (sdsk)->disk->disk_name, fmt, ##a) :   \
>>                sdev_printk(prefix, (sdsk)->device, fmt, ##a)
>>
>> #define sd_first_printk(prefix, sdsk, fmt, a...)                        \
>>          do {                                                            \
>>                  if ((sdkp)->first_scan)                                 \
>>                          sd_printk(prefix, sdsk, fmt, ##a);              \
>>          } while (0)
>>
>>
>>
>> Most of the sdsk used in the macro definition has only one sdkp.
>>
>>
>> This is defined in the v6.0-rc7 kernel:
>>
>> #define sd_printk(prefix, sdsk, fmt, a...)                              \
>>          (sdsk)->disk ?                                                  \
>>                sdev_prefix_printk(prefix, (sdsk)->device,                \
>>                                   (sdsk)->disk->disk_name, fmt, ##a) :   \
>>                sdev_printk(prefix, (sdsk)->device, fmt, ##a)
>>
>> #define sd_first_printk(prefix, sdsk, fmt, a...)                        \
>>          do {                                                            \
>>                  if ((sdsk)->first_scan)                                 \
>>                          sd_printk(prefix, sdsk, fmt, ##a);              \
>>          } while (0)
>>
>> Use sdsk in macro definition.
>>
>>
>> I did report an error when compiling sd. o in the 4.19 kernel. It was modified to say that no more errors were reported in sdsk. Can I continue the 6.0-rc7 writing method here.
>>
> 
> You should backport the mainline patch to 4.19, not create a new one.

Yes, but since the mainline patch has a typo, better fix it and backport the fix
too with a "Fixes" tag.

My point about the proposed patch was to make the reverse change to fix the
macro: use sdkp instead of sdsk since the former is used everywhere and clear.
But sure, since this is not causing any issue, no strong need to fix the macro.
It is really ugly as-is though :)

> 
> commit df46cac3f71c57e0b23f6865651629aaa54f8ca9
> Author: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>
> Date:   Tue Feb 5 11:10:48 2019 +0100
> 
>      scsi: sd: Fix typo in sd_first_printk()
> 
>      Commit b2bff6ceb61a9 ("[SCSI] sd: Quiesce mode sense error messages")
>      added the macro sd_first_printk(). The macro takes "sdsk" as argument
>      but dereferences "sdkp". This hasn't caused any real issues since all
>      callers of sd_first_printk() have an sdkp. But fix the typo.
> 
>      [mkp: Turned this into a real patch and tweaked commit description]
> 
>      Signed-off-by: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>
>      Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
> 
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 1080c85d97f8..5796ace76225 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -132,7 +132,7 @@ static inline struct scsi_disk *scsi_disk(struct 
> gendisk *disk)
> 
>   #define sd_first_printk(prefix, sdsk, fmt, a...)                       \
>          do {                                                            \
> -               if ((sdkp)->first_scan)                                 \
> +               if ((sdsk)->first_scan)                                 \
>                          sd_printk(prefix, sdsk, fmt, ##a);              \
>          } while (0)
> 
> 
> 
>>
>> .
>>

-- 
Damien Le Moal
Western Digital Research