2022-10-13 02:56:56

by Li kunyu

[permalink] [raw]
Subject: [PATCH 4.19] scsi: sd: Fix 'sdkp' in sd_first_printk

In the sd_first_printk macro, replace the sdkp parameter with the
defined sdsk parameter to resolve the compilation error.

Signed-off-by: Li kunyu <[email protected]>
---
drivers/scsi/sd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index a7d4f50b67d4..e5bdf0a10071 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -133,7 +133,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)

--
2.18.2


2022-10-13 04:06:29

by Damien Le Moal

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

On 2022/10/13 11:34, Li kunyu wrote:
> In the sd_first_printk macro, replace the sdkp parameter with the
> defined sdsk parameter to resolve the compilation error.

Which compilation errors ? None that I can see.
Do you mean "to avoid potential compilation errors" ?

>
> Signed-off-by: Li kunyu <[email protected]>
> ---
> drivers/scsi/sd.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index a7d4f50b67d4..e5bdf0a10071 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -133,7 +133,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) \

Instead of changing this one, I would prefer changing sdsk to sdkp in the macro
parameter list. "sdkp" is used everywhere in sd.c. "sdsk" is not used anywhere
so it would be unclear.

> sd_printk(prefix, sdsk, fmt, ##a); \
> } while (0)
>

--
Damien Le Moal
Western Digital Research

2022-10-13 04:58:16

by Li kunyu

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


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.

2022-10-13 06:49:23

by Jason Yan

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


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.

commit df46cac3f71c57e0b23f6865651629aaa54f8ca9
Author: Dietmar Hahn <[email protected]>
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 <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>

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)



>
> .
>

2022-10-13 07:35:27

by Jason Yan

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


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.

> 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.

Thanks,
Jason

>>
>> commit df46cac3f71c57e0b23f6865651629aaa54f8ca9
>> Author: Dietmar Hahn <[email protected]>
>> 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 <[email protected]>
>> Signed-off-by: Martin K. Petersen <[email protected]>
>>
>> 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)
>>
>>
>>
>>>
>>> .
>>>
>

2022-10-13 07:35:35

by Damien Le Moal

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

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 <[email protected]>
>>> 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 <[email protected]>
>>> Signed-off-by: Martin K. Petersen <[email protected]>
>>>
>>> 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

2022-10-13 07:39:34

by Damien Le Moal

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

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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Martin K. Petersen <[email protected]>
>
> 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

2022-10-13 07:44:35

by Li kunyu

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


Hello, I'm not sure how to operate it. Do you want to execute git push to submit this patch in branch 4.19.

thanks,
kunyu

2022-10-13 08:36:06

by Damien Le Moal

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

On 2022/10/13 16:39, Li kunyu wrote:
>
> Hello, I'm not sure how to operate it. Do you want to execute git push to submit this patch in branch 4.19.
>
> thanks,
> kunyu
>

Please see Documentation/process/stable-kernel-rules.rst for how to proceed. In
this case, I think you should follow "option 2", but read the entire document to
understand all cases.

--
Damien Le Moal
Western Digital Research

2022-10-13 08:45:02

by Li kunyu

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


I see. I will send it after I understand the steps.