Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
IronPort-SDR: mE8guRoek435iQ2RZfnSi+386z6z6X+c+NJzJQTFGfIDfSEZhH7K04Iqbs8rL1xvVlTti+8B6C
 Bh0JiYUy3g73dZO/UUMFvOyTpL7wJPVefDP1ewxGAumkREQEF8abhiT5e9Cj301Eb26jX55vvY
 cM9ywL4o+a9Quaor2VkppEeLGNt15K599BYRVBdN/jf08KfXDn4z03PXabrNGOYNhi/1b5U8Yk
 IYd3yJ5KVWs2Ww8G9T31fDM6WvmvFRWck9YvvmIDTFUjyG7nLhk0ATxEev9TfquM3C5UYkbMJ3
 QveSU9EGTnPeMB+SsES2pmMk
IronPort-SDR: DsPGD5IPYik3zERSPK9gyjc+VlUjjG8UuxEwjyphibhWXUkPn0TFFeYlDKAb1/cv1a2DIv6h11
 feY8U7NXSN4m30thVOhO4+/cxfNmhY8jymBaixttYx6IqQmNqREqFfFvd7r870CZ+EmVIgEy2V
 62rUtUE3HdLymzfZpZTBdHl7SaokNHUcXo0ARsIMFc8G1H6bknMnigVHkfmktqTktjIPdAwNdR
 fCdjX4yG02trRCBckKIxB9ZUDwBulcn3HlqP8lciplIVfcmjGzApTniGdFJHycxqz/vg9Teqk8
 Rqc=
WDCIronportException: Internal
Message-ID: <4fc8207d-0851-3b38-339d-de1452f6ee4d@opensource.wdc.com>
Date:   Thu, 13 Oct 2022 16:14:12 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.3.2
Subject: Re: [PATCH 4.19] scsi: sd: Fix 'sdkp' in sd_first_printk
Content-Language: en-US
To:     Jason Yan <yanaijie@huawei.com>, Li kunyu <kunyu@nfschina.com>
Cc:     jejb@linux.vnet.ibm.com, linux-kernel@vger.kernel.org,
        linux-scsi@vger.kernel.org, martin.petersen@oracle.com
References: <0e67aa4d-f66e-f392-5950-31b1c90c287b@opensource.wdc.com>
 <20221013044927.278854-1-kunyu@nfschina.com>
 <badf255f-df60-fbc7-0f61-c69b99ebbaa6@huawei.com>
 <45f33e3e-c1a7-7183-bf04-83649af8ac04@opensource.wdc.com>
 <7baa5eb6-95f7-a0cb-6aef-157cf43866e7@huawei.com>
From:   Damien Le Moal <damien.lemoal@opensource.wdc.com>
Organization: Western Digital Research
In-Reply-To: <7baa5eb6-95f7-a0cb-6aef-157cf43866e7@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022/10/13 16:02, Jason Yan wrote:
> 
> On 2022/10/13 14:39, Damien Le Moal wrote:
>> 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.
>>
> 
> What typo in the patch? I did not see it.

I meant the weird variable name sdsk in the original patch instead of the more
natural sdkp.

> 
>> 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 :)
>>
> 
> I agree that there is no need to backport it.

Yes. Beside using that strange variable name, no problem.

> 
> Thanks,
> Jason
> 
>>>
>>> commit df46cac3f71c57e0b23f6865651629aaa54f8ca9
>>> Author: Dietmar Hahn <dietmar.hahn@ts.fujitsu.com>
>>> 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@ts.fujitsu.com>
>>>       Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
>>>
>>> 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