[PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Mingzheng Xing]

This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
merged into the mainline commit 39365395046f ("riscv: kdump: use generic
interface to simplify crashkernel reservation"), but the latter's series of
patches are not included in the 6.6 branch.

This will result in the loss of Crash kernel data in /proc/iomem, and kdump
loading the kernel will also cause an error:

```
Memory for crashkernel is not reserved
Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
Then try to loading kdump kernel
```

After revert this patch, verify that it works properly on QEMU riscv.

Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
---
 arch/riscv/kernel/setup.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index aac853ae4eb74..e600aab116a40 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -173,6 +173,19 @@ static void __init init_resources(void)
 	if (ret < 0)
 		goto error;
 
+#ifdef CONFIG_KEXEC_CORE
+	if (crashk_res.start != crashk_res.end) {
+		ret = add_resource(&iomem_resource, &crashk_res);
+		if (ret < 0)
+			goto error;
+	}
+	if (crashk_low_res.start != crashk_low_res.end) {
+		ret = add_resource(&iomem_resource, &crashk_low_res);
+		if (ret < 0)
+			goto error;
+	}
+#endif
+
 #ifdef CONFIG_CRASH_DUMP
 	if (elfcorehdr_size > 0) {
 		elfcorehdr_res.start = elfcorehdr_addr;
-- 
2.34.1

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Greg Kroah-Hartman]

On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
> interface to simplify crashkernel reservation"), but the latter's series of
> patches are not included in the 6.6 branch.
> 
> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
> loading the kernel will also cause an error:
> 
> ```
> Memory for crashkernel is not reserved
> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
> Then try to loading kdump kernel
> ```
> 
> After revert this patch, verify that it works properly on QEMU riscv.
> 
> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
> ---

I do not understand, what branch is this for?  Why have you not cc:ed
any of the original developers here?  Why does Linus's tree not have the
same problem?  And the first sentence above does not make much sense as
a 6.6 change is merged into 6.7?

confused,

greg k-h

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Mingzheng Xing]

On 4/19/24 18:44, Greg Kroah-Hartman wrote:
> On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
>> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
>> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
>> interface to simplify crashkernel reservation"), but the latter's series of
>> patches are not included in the 6.6 branch.
>>
>> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
>> loading the kernel will also cause an error:
>>
>> ```
>> Memory for crashkernel is not reserved
>> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
>> Then try to loading kdump kernel
>> ```
>>
>> After revert this patch, verify that it works properly on QEMU riscv.
>>
>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
>> ---
> 
> I do not understand, what branch is this for?  Why have you not cc:ed
> any of the original developers here?  Why does Linus's tree not have the
> same problem?  And the first sentence above does not make much sense as
> a 6.6 change is merged into 6.7?

Sorry, I'll try to explain it more clearly.

This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
on RISC-V") should not have existed because this patch has been merged into
another larger patch [1]. Here is that complete series:

c37e56cac3d62 crash_core.c: remove unneeded functions
39365395046fe riscv: kdump: use generic interface to simplify crashkernel reservation [1]
fdc268232dbba arm64: kdump: use generic interface to simplify crashkernel reservation
9c08a2a139fe8 x86: kdump: use generic interface to simplify crashkernel reservation code
b631b95dded5e crash_core: move crashk_*res definition into crash_core.c
0ab97169aa051 crash_core: add generic function to do reservation
70916e9c8d9f1 crash_core: change parse_crashkernel() to support crashkernel=,high|low parsing
a9e1a3d84e4a0 crash_core: change the prototype of function parse_crashkernel()
a6304272b03ec crash_core.c: remove unnecessary parameter of function

I checked and that series above is not present in 6.6.y. It is only present
in 6.7+. So this commit is causing an error. Crash kernel information
cannot be read from /proc/iomem when using the 6.6.y kernel.

I tested two ways to fix this error, the first one is to revert this
commit. the second one is to backport the complete series above to 6.6.y,
but according to stable-kernel-rules, it seems that the most appropriate
method is the first one.

This revert is only for 6.6.y branch.


Thanks,
Mingzheng

> 
> confused,
> 
> greg k-h

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Greg Kroah-Hartman]

On Fri, Apr 19, 2024 at 08:26:07PM +0800, Mingzheng Xing wrote:
> On 4/19/24 18:44, Greg Kroah-Hartman wrote:
> > On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
> >> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
> >> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
> >> interface to simplify crashkernel reservation"), but the latter's series of
> >> patches are not included in the 6.6 branch.
> >>
> >> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
> >> loading the kernel will also cause an error:
> >>
> >> ```
> >> Memory for crashkernel is not reserved
> >> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
> >> Then try to loading kdump kernel
> >> ```
> >>
> >> After revert this patch, verify that it works properly on QEMU riscv.
> >>
> >> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
> >> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
> >> ---
> > 
> > I do not understand, what branch is this for?  Why have you not cc:ed
> > any of the original developers here?  Why does Linus's tree not have the
> > same problem?  And the first sentence above does not make much sense as
> > a 6.6 change is merged into 6.7?
> 
> Sorry, I'll try to explain it more clearly.
> 
> This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
> on RISC-V") should not have existed because this patch has been merged into
> another larger patch [1]. Here is that complete series:

What "larger patch"?  It is in Linus's tree, so it's not part of
something different, right?  I'm confused.

> c37e56cac3d62 crash_core.c: remove unneeded functions
> 39365395046fe riscv: kdump: use generic interface to simplify crashkernel reservation [1]
> fdc268232dbba arm64: kdump: use generic interface to simplify crashkernel reservation
> 9c08a2a139fe8 x86: kdump: use generic interface to simplify crashkernel reservation code
> b631b95dded5e crash_core: move crashk_*res definition into crash_core.c
> 0ab97169aa051 crash_core: add generic function to do reservation
> 70916e9c8d9f1 crash_core: change parse_crashkernel() to support crashkernel=,high|low parsing
> a9e1a3d84e4a0 crash_core: change the prototype of function parse_crashkernel()
> a6304272b03ec crash_core.c: remove unnecessary parameter of function
> 
> I checked and that series above is not present in 6.6.y. It is only present
> in 6.7+. So this commit is causing an error. Crash kernel information
> cannot be read from /proc/iomem when using the 6.6.y kernel.

Did that ever work in older kernels?  Is this a regression?  Or are the
commits in 6.7 just to fix this feature up and get it to work?

> I tested two ways to fix this error, the first one is to revert this
> commit. the second one is to backport the complete series above to 6.6.y,
> but according to stable-kernel-rules, it seems that the most appropriate
> method is the first one.

It depends if this is a regression from older kernels or not.

Please work with the maintainers of the above code to figure out what is
best to do here and get them to agree what needs to happen.

thanks,

greg k-h

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Mingzheng Xing]

On 4/19/24 21:58, Greg Kroah-Hartman wrote:
> On Fri, Apr 19, 2024 at 08:26:07PM +0800, Mingzheng Xing wrote:
>> On 4/19/24 18:44, Greg Kroah-Hartman wrote:
>>> On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
>>>> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
>>>> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
>>>> interface to simplify crashkernel reservation"), but the latter's series of
>>>> patches are not included in the 6.6 branch.
>>>>
>>>> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
>>>> loading the kernel will also cause an error:
>>>>
>>>> ```
>>>> Memory for crashkernel is not reserved
>>>> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
>>>> Then try to loading kdump kernel
>>>> ```
>>>>
>>>> After revert this patch, verify that it works properly on QEMU riscv.
>>>>
>>>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
>>>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
>>>> ---
>>>
>>> I do not understand, what branch is this for?  Why have you not cc:ed
>>> any of the original developers here?  Why does Linus's tree not have the
>>> same problem?  And the first sentence above does not make much sense as
>>> a 6.6 change is merged into 6.7?
>>
>> Sorry, I'll try to explain it more clearly.
>>
>> This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
>> on RISC-V") should not have existed because this patch has been merged into
>> another larger patch [1]. Here is that complete series:
> 
> What "larger patch"?  It is in Linus's tree, so it's not part of
> something different, right?  I'm confused.
> 

Hi, Greg

The email Cc:ed to author Chen Jiahao was bounced by the system, so maybe
we can wait for Baoquan He to confirm.

This is indeed a bit confusing. The Fixes: tag in 1d6cd2146c2b58 is a false
reference. If I understand correctly, this is similar to the following
scenario:

A Fixes B, B doesn't go into linus mainline. C contains A, C goes into linus
mainline 6.7, and C has more reconstruction code. but A goes into 6.6, so
it doesn't make sense for A to be in the mainline, and there's no C in 6.6
but there's an A, thus resulting in an incomplete code that creates an error.

The link I quoted [1] shows that Baoquan had expressed an opinion on this
at the time.

Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv [1]


Thanks,
Mingzheng

>> c37e56cac3d62 crash_core.c: remove unneeded functions
>> 39365395046fe riscv: kdump: use generic interface to simplify crashkernel reservation [1]
>> fdc268232dbba arm64: kdump: use generic interface to simplify crashkernel reservation
>> 9c08a2a139fe8 x86: kdump: use generic interface to simplify crashkernel reservation code
>> b631b95dded5e crash_core: move crashk_*res definition into crash_core.c
>> 0ab97169aa051 crash_core: add generic function to do reservation
>> 70916e9c8d9f1 crash_core: change parse_crashkernel() to support crashkernel=,high|low parsing
>> a9e1a3d84e4a0 crash_core: change the prototype of function parse_crashkernel()
>> a6304272b03ec crash_core.c: remove unnecessary parameter of function
>>
>> I checked and that series above is not present in 6.6.y. It is only present
>> in 6.7+. So this commit is causing an error. Crash kernel information
>> cannot be read from /proc/iomem when using the 6.6.y kernel.
> 
> Did that ever work in older kernels?  Is this a regression?  Or are the
> commits in 6.7 just to fix this feature up and get it to work?
> 
>> I tested two ways to fix this error, the first one is to revert this
>> commit. the second one is to backport the complete series above to 6.6.y,
>> but according to stable-kernel-rules, it seems that the most appropriate
>> method is the first one.
> 
> It depends if this is a regression from older kernels or not.
> 
> Please work with the maintainers of the above code to figure out what is
> best to do here and get them to agree what needs to happen.
> 
> thanks,
> 
> greg k-h

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Greg Kroah-Hartman]

On Fri, Apr 19, 2024 at 10:55:44PM +0800, Mingzheng Xing wrote:
> On 4/19/24 21:58, Greg Kroah-Hartman wrote:
> > On Fri, Apr 19, 2024 at 08:26:07PM +0800, Mingzheng Xing wrote:
> >> On 4/19/24 18:44, Greg Kroah-Hartman wrote:
> >>> On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
> >>>> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
> >>>> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
> >>>> interface to simplify crashkernel reservation"), but the latter's series of
> >>>> patches are not included in the 6.6 branch.
> >>>>
> >>>> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
> >>>> loading the kernel will also cause an error:
> >>>>
> >>>> ```
> >>>> Memory for crashkernel is not reserved
> >>>> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
> >>>> Then try to loading kdump kernel
> >>>> ```
> >>>>
> >>>> After revert this patch, verify that it works properly on QEMU riscv.
> >>>>
> >>>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
> >>>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
> >>>> ---
> >>>
> >>> I do not understand, what branch is this for?  Why have you not cc:ed
> >>> any of the original developers here?  Why does Linus's tree not have the
> >>> same problem?  And the first sentence above does not make much sense as
> >>> a 6.6 change is merged into 6.7?
> >>
> >> Sorry, I'll try to explain it more clearly.
> >>
> >> This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
> >> on RISC-V") should not have existed because this patch has been merged into
> >> another larger patch [1]. Here is that complete series:
> > 
> > What "larger patch"?  It is in Linus's tree, so it's not part of
> > something different, right?  I'm confused.
> > 
> 
> Hi, Greg
> 
> The email Cc:ed to author Chen Jiahao was bounced by the system, so maybe
> we can wait for Baoquan He to confirm.
> 
> This is indeed a bit confusing. The Fixes: tag in 1d6cd2146c2b58 is a false
> reference. If I understand correctly, this is similar to the following
> scenario:
> 
> A Fixes B, B doesn't go into linus mainline. C contains A, C goes into linus
> mainline 6.7, and C has more reconstruction code. but A goes into 6.6, so
> it doesn't make sense for A to be in the mainline, and there's no C in 6.6
> but there's an A, thus resulting in an incomplete code that creates an error.
> 
> The link I quoted [1] shows that Baoquan had expressed an opinion on this
> at the time.
> 
> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv [1]

I'm sorry, but I still do not understand what I need to do here for a
stable branch.  Do I need to apply something?  Revert something?
Something else?

confused,

greg k-h

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Baoquan He]

On 04/19/24 at 10:55pm, Mingzheng Xing wrote:
> On 4/19/24 21:58, Greg Kroah-Hartman wrote:
> > On Fri, Apr 19, 2024 at 08:26:07PM +0800, Mingzheng Xing wrote:
> >> On 4/19/24 18:44, Greg Kroah-Hartman wrote:
> >>> On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
> >>>> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
> >>>> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
> >>>> interface to simplify crashkernel reservation"), but the latter's series of
> >>>> patches are not included in the 6.6 branch.
> >>>>
> >>>> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
> >>>> loading the kernel will also cause an error:
> >>>>
> >>>> ```
> >>>> Memory for crashkernel is not reserved
> >>>> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
> >>>> Then try to loading kdump kernel
> >>>> ```
> >>>>
> >>>> After revert this patch, verify that it works properly on QEMU riscv.
> >>>>
> >>>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
> >>>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
> >>>> ---
> >>>
> >>> I do not understand, what branch is this for?  Why have you not cc:ed
> >>> any of the original developers here?  Why does Linus's tree not have the
> >>> same problem?  And the first sentence above does not make much sense as
> >>> a 6.6 change is merged into 6.7?
> >>
> >> Sorry, I'll try to explain it more clearly.
> >>
> >> This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
> >> on RISC-V") should not have existed because this patch has been merged into
> >> another larger patch [1]. Here is that complete series:
> > 
> > What "larger patch"?  It is in Linus's tree, so it's not part of
> > something different, right?  I'm confused.
> > 
> 
> Hi, Greg
> 
> The email Cc:ed to author Chen Jiahao was bounced by the system, so maybe
> we can wait for Baoquan He to confirm.
> 
> This is indeed a bit confusing. The Fixes: tag in 1d6cd2146c2b58 is a false
> reference. If I understand correctly, this is similar to the following
> scenario:
> 
> A Fixes B, B doesn't go into linus mainline. C contains A, C goes into linus
> mainline 6.7, and C has more reconstruction code. but A goes into 6.6, so
> it doesn't make sense for A to be in the mainline, and there's no C in 6.6
> but there's an A, thus resulting in an incomplete code that creates an error.

Do you mean commit 1d6cd2146c2b58 ("iscv: kdump: fix crashkernel reserving
problem on RISC-V") was mistakenly added into v6.6, but the commit which
it fixed is not existing in v6.6?

I checked code, it does look like what you said, and it's truly
confusing. If so, we should revert commit 1d6cd2146c2b58 ("iscv: kdump: fix
crashkernel reserving problem on RISC-V") in v6.6.y to make kdump work
on risc-v.

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Mingzheng Xing]

On 4/23/24 21:12, Greg Kroah-Hartman wrote:
> On Fri, Apr 19, 2024 at 10:55:44PM +0800, Mingzheng Xing wrote:
>> On 4/19/24 21:58, Greg Kroah-Hartman wrote:
>>> On Fri, Apr 19, 2024 at 08:26:07PM +0800, Mingzheng Xing wrote:
>>>> On 4/19/24 18:44, Greg Kroah-Hartman wrote:
>>>>> On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
>>>>>> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
>>>>>> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
>>>>>> interface to simplify crashkernel reservation"), but the latter's series of
>>>>>> patches are not included in the 6.6 branch.
>>>>>>
>>>>>> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
>>>>>> loading the kernel will also cause an error:
>>>>>>
>>>>>> ```
>>>>>> Memory for crashkernel is not reserved
>>>>>> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
>>>>>> Then try to loading kdump kernel
>>>>>> ```
>>>>>>
>>>>>> After revert this patch, verify that it works properly on QEMU riscv.
>>>>>>
>>>>>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
>>>>>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
>>>>>> ---
>>>>>
>>>>> I do not understand, what branch is this for?  Why have you not cc:ed
>>>>> any of the original developers here?  Why does Linus's tree not have the
>>>>> same problem?  And the first sentence above does not make much sense as
>>>>> a 6.6 change is merged into 6.7?
>>>>
>>>> Sorry, I'll try to explain it more clearly.
>>>>
>>>> This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
>>>> on RISC-V") should not have existed because this patch has been merged into
>>>> another larger patch [1]. Here is that complete series:
>>>
>>> What "larger patch"?  It is in Linus's tree, so it's not part of
>>> something different, right?  I'm confused.
>>>
>>
>> Hi, Greg
>>
>> The email Cc:ed to author Chen Jiahao was bounced by the system, so maybe
>> we can wait for Baoquan He to confirm.
>>
>> This is indeed a bit confusing. The Fixes: tag in 1d6cd2146c2b58 is a false
>> reference. If I understand correctly, this is similar to the following
>> scenario:
>>
>> A Fixes B, B doesn't go into linus mainline. C contains A, C goes into linus
>> mainline 6.7, and C has more reconstruction code. but A goes into 6.6, so
>> it doesn't make sense for A to be in the mainline, and there's no C in 6.6
>> but there's an A, thus resulting in an incomplete code that creates an error.
>>
>> The link I quoted [1] shows that Baoquan had expressed an opinion on this
>> at the time.
>>
>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv [1]
> 
> I'm sorry, but I still do not understand what I need to do here for a
> stable branch.  Do I need to apply something?  Revert something?
> Something else?

Hi, Greg

I saw Baoquan's reply in thread[1], thanks Baoquan for confirming.

So I think the right thing to do would be just to REVERT the commit
1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem on RISC-V")
in the 6.6.y branch, which is exactly the patch I submitted. If I need to
make changes to my commit message, feel free to let me know and I'll post
the second version.

Link: https://lore.kernel.org/stable/ZihbAYMOI4ylazpt@MiWiFi-R3L-srv [1]

Thanks,
Mingzheng

> 
> confused,
> 
> greg k-h

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Mingzheng Xing]

On 4/24/24 09:06, Baoquan He wrote:
> On 04/19/24 at 10:55pm, Mingzheng Xing wrote:
>> On 4/19/24 21:58, Greg Kroah-Hartman wrote:
>>> On Fri, Apr 19, 2024 at 08:26:07PM +0800, Mingzheng Xing wrote:
>>>> On 4/19/24 18:44, Greg Kroah-Hartman wrote:
>>>>> On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
>>>>>> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
>>>>>> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
>>>>>> interface to simplify crashkernel reservation"), but the latter's series of
>>>>>> patches are not included in the 6.6 branch.
>>>>>>
>>>>>> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
>>>>>> loading the kernel will also cause an error:
>>>>>>
>>>>>> ```
>>>>>> Memory for crashkernel is not reserved
>>>>>> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
>>>>>> Then try to loading kdump kernel
>>>>>> ```
>>>>>>
>>>>>> After revert this patch, verify that it works properly on QEMU riscv.
>>>>>>
>>>>>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
>>>>>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
>>>>>> ---
>>>>>
>>>>> I do not understand, what branch is this for?  Why have you not cc:ed
>>>>> any of the original developers here?  Why does Linus's tree not have the
>>>>> same problem?  And the first sentence above does not make much sense as
>>>>> a 6.6 change is merged into 6.7?
>>>>
>>>> Sorry, I'll try to explain it more clearly.
>>>>
>>>> This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
>>>> on RISC-V") should not have existed because this patch has been merged into
>>>> another larger patch [1]. Here is that complete series:
>>>
>>> What "larger patch"?  It is in Linus's tree, so it's not part of
>>> something different, right?  I'm confused.
>>>
>>
>> Hi, Greg
>>
>> The email Cc:ed to author Chen Jiahao was bounced by the system, so maybe
>> we can wait for Baoquan He to confirm.
>>
>> This is indeed a bit confusing. The Fixes: tag in 1d6cd2146c2b58 is a false
>> reference. If I understand correctly, this is similar to the following
>> scenario:
>>
>> A Fixes B, B doesn't go into linus mainline. C contains A, C goes into linus
>> mainline 6.7, and C has more reconstruction code. but A goes into 6.6, so
>> it doesn't make sense for A to be in the mainline, and there's no C in 6.6
>> but there's an A, thus resulting in an incomplete code that creates an error.
> 
> Do you mean commit 1d6cd2146c2b58 ("iscv: kdump: fix crashkernel reserving
> problem on RISC-V") was mistakenly added into v6.6, but the commit which
> it fixed is not existing in v6.6?
> 

Yeah, you're right, and it was merged into 6.7 by another commit
39365395046f ("riscv: kdump: use generic interface to simplify crashkernel reservation")

> I checked code, it does look like what you said, and it's truly
> confusing. If so, we should revert commit 1d6cd2146c2b58 ("iscv: kdump: fix
> crashkernel reserving problem on RISC-V") in v6.6.y to make kdump work
> on risc-v.

I agree. I built the 6.6.y branch kernel, tested it on qemu, and there is
no 'Crash kernel' entry in /proc/iomem. After revert, it's back to normal.


Thanks,
Mingzheng

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Greg Kroah-Hartman]

On Wed, Apr 24, 2024 at 11:40:16AM +0800, Mingzheng Xing wrote:
> On 4/23/24 21:12, Greg Kroah-Hartman wrote:
> > On Fri, Apr 19, 2024 at 10:55:44PM +0800, Mingzheng Xing wrote:
> >> On 4/19/24 21:58, Greg Kroah-Hartman wrote:
> >>> On Fri, Apr 19, 2024 at 08:26:07PM +0800, Mingzheng Xing wrote:
> >>>> On 4/19/24 18:44, Greg Kroah-Hartman wrote:
> >>>>> On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
> >>>>>> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
> >>>>>> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
> >>>>>> interface to simplify crashkernel reservation"), but the latter's series of
> >>>>>> patches are not included in the 6.6 branch.
> >>>>>>
> >>>>>> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
> >>>>>> loading the kernel will also cause an error:
> >>>>>>
> >>>>>> ```
> >>>>>> Memory for crashkernel is not reserved
> >>>>>> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
> >>>>>> Then try to loading kdump kernel
> >>>>>> ```
> >>>>>>
> >>>>>> After revert this patch, verify that it works properly on QEMU riscv.
> >>>>>>
> >>>>>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
> >>>>>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
> >>>>>> ---
> >>>>>
> >>>>> I do not understand, what branch is this for?  Why have you not cc:ed
> >>>>> any of the original developers here?  Why does Linus's tree not have the
> >>>>> same problem?  And the first sentence above does not make much sense as
> >>>>> a 6.6 change is merged into 6.7?
> >>>>
> >>>> Sorry, I'll try to explain it more clearly.
> >>>>
> >>>> This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
> >>>> on RISC-V") should not have existed because this patch has been merged into
> >>>> another larger patch [1]. Here is that complete series:
> >>>
> >>> What "larger patch"?  It is in Linus's tree, so it's not part of
> >>> something different, right?  I'm confused.
> >>>
> >>
> >> Hi, Greg
> >>
> >> The email Cc:ed to author Chen Jiahao was bounced by the system, so maybe
> >> we can wait for Baoquan He to confirm.
> >>
> >> This is indeed a bit confusing. The Fixes: tag in 1d6cd2146c2b58 is a false
> >> reference. If I understand correctly, this is similar to the following
> >> scenario:
> >>
> >> A Fixes B, B doesn't go into linus mainline. C contains A, C goes into linus
> >> mainline 6.7, and C has more reconstruction code. but A goes into 6.6, so
> >> it doesn't make sense for A to be in the mainline, and there's no C in 6.6
> >> but there's an A, thus resulting in an incomplete code that creates an error.
> >>
> >> The link I quoted [1] shows that Baoquan had expressed an opinion on this
> >> at the time.
> >>
> >> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv [1]
> > 
> > I'm sorry, but I still do not understand what I need to do here for a
> > stable branch.  Do I need to apply something?  Revert something?
> > Something else?
> 
> Hi, Greg
> 
> I saw Baoquan's reply in thread[1], thanks Baoquan for confirming.
> 
> So I think the right thing to do would be just to REVERT the commit
> 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem on RISC-V")
> in the 6.6.y branch, which is exactly the patch I submitted. If I need to
> make changes to my commit message, feel free to let me know and I'll post
> the second version.
> 
> Link: https://lore.kernel.org/stable/ZihbAYMOI4ylazpt@MiWiFi-R3L-srv [1]

Can someone just send me a patch series showing EXACTLY what needs to be
done here, as I am _still_ confused.

thanks,

greg k-h

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Baoquan He]

On 04/29/24 at 12:52pm, Greg Kroah-Hartman wrote:
> On Wed, Apr 24, 2024 at 11:40:16AM +0800, Mingzheng Xing wrote:
> > On 4/23/24 21:12, Greg Kroah-Hartman wrote:
> > > On Fri, Apr 19, 2024 at 10:55:44PM +0800, Mingzheng Xing wrote:
> > >> On 4/19/24 21:58, Greg Kroah-Hartman wrote:
> > >>> On Fri, Apr 19, 2024 at 08:26:07PM +0800, Mingzheng Xing wrote:
> > >>>> On 4/19/24 18:44, Greg Kroah-Hartman wrote:
> > >>>>> On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
> > >>>>>> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
> > >>>>>> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
> > >>>>>> interface to simplify crashkernel reservation"), but the latter's series of
> > >>>>>> patches are not included in the 6.6 branch.
> > >>>>>>
> > >>>>>> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
> > >>>>>> loading the kernel will also cause an error:
> > >>>>>>
> > >>>>>> ```
> > >>>>>> Memory for crashkernel is not reserved
> > >>>>>> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
> > >>>>>> Then try to loading kdump kernel
> > >>>>>> ```
> > >>>>>>
> > >>>>>> After revert this patch, verify that it works properly on QEMU riscv.
> > >>>>>>
> > >>>>>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
> > >>>>>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
> > >>>>>> ---
> > >>>>>
> > >>>>> I do not understand, what branch is this for?  Why have you not cc:ed
> > >>>>> any of the original developers here?  Why does Linus's tree not have the
> > >>>>> same problem?  And the first sentence above does not make much sense as
> > >>>>> a 6.6 change is merged into 6.7?
> > >>>>
> > >>>> Sorry, I'll try to explain it more clearly.
> > >>>>
> > >>>> This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
> > >>>> on RISC-V") should not have existed because this patch has been merged into
> > >>>> another larger patch [1]. Here is that complete series:
> > >>>
> > >>> What "larger patch"?  It is in Linus's tree, so it's not part of
> > >>> something different, right?  I'm confused.
> > >>>
> > >>
> > >> Hi, Greg
> > >>
> > >> The email Cc:ed to author Chen Jiahao was bounced by the system, so maybe
> > >> we can wait for Baoquan He to confirm.
> > >>
> > >> This is indeed a bit confusing. The Fixes: tag in 1d6cd2146c2b58 is a false
> > >> reference. If I understand correctly, this is similar to the following
> > >> scenario:
> > >>
> > >> A Fixes B, B doesn't go into linus mainline. C contains A, C goes into linus
> > >> mainline 6.7, and C has more reconstruction code. but A goes into 6.6, so
> > >> it doesn't make sense for A to be in the mainline, and there's no C in 6.6
> > >> but there's an A, thus resulting in an incomplete code that creates an error.
> > >>
> > >> The link I quoted [1] shows that Baoquan had expressed an opinion on this
> > >> at the time.
> > >>
> > >> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv [1]
> > > 
> > > I'm sorry, but I still do not understand what I need to do here for a
> > > stable branch.  Do I need to apply something?  Revert something?
> > > Something else?
> > 
> > Hi, Greg
> > 
> > I saw Baoquan's reply in thread[1], thanks Baoquan for confirming.
> > 
> > So I think the right thing to do would be just to REVERT the commit
> > 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem on RISC-V")
> > in the 6.6.y branch, which is exactly the patch I submitted. If I need to
> > make changes to my commit message, feel free to let me know and I'll post
> > the second version.
> > 
> > Link: https://lore.kernel.org/stable/ZihbAYMOI4ylazpt@MiWiFi-R3L-srv [1]
> 
> Can someone just send me a patch series showing EXACTLY what needs to be
> done here, as I am _still_ confused.

I think Mingzheng's patch is good to apply in the 6.6.y stable branch.

Hi Mingzheng,

Can you resend this patch to Greg and stable@vger.kernel.org and CC me?
I would like to Ack your patch, but can't find the original patch since
you didn't cc me.

Thanks
Baoquan

Re: [PATCH] Revert "riscv: kdump: fix crashkernel reserving problem on RISC-V"

[Mingzheng Xing]

On 4/30/24 08:02, Baoquan He wrote:
> On 04/29/24 at 12:52pm, Greg Kroah-Hartman wrote:
>> On Wed, Apr 24, 2024 at 11:40:16AM +0800, Mingzheng Xing wrote:
>>> On 4/23/24 21:12, Greg Kroah-Hartman wrote:
>>>> On Fri, Apr 19, 2024 at 10:55:44PM +0800, Mingzheng Xing wrote:
>>>>> On 4/19/24 21:58, Greg Kroah-Hartman wrote:
>>>>>> On Fri, Apr 19, 2024 at 08:26:07PM +0800, Mingzheng Xing wrote:
>>>>>>> On 4/19/24 18:44, Greg Kroah-Hartman wrote:
>>>>>>>> On Tue, Apr 16, 2024 at 04:56:47PM +0800, Mingzheng Xing wrote:
>>>>>>>>> This reverts commit 1d6cd2146c2b58bc91266db1d5d6a5f9632e14c0 which has been
>>>>>>>>> merged into the mainline commit 39365395046f ("riscv: kdump: use generic
>>>>>>>>> interface to simplify crashkernel reservation"), but the latter's series of
>>>>>>>>> patches are not included in the 6.6 branch.
>>>>>>>>>
>>>>>>>>> This will result in the loss of Crash kernel data in /proc/iomem, and kdump
>>>>>>>>> loading the kernel will also cause an error:
>>>>>>>>>
>>>>>>>>> ```
>>>>>>>>> Memory for crashkernel is not reserved
>>>>>>>>> Please reserve memory by passing"crashkernel=Y@X" parameter to kernel
>>>>>>>>> Then try to loading kdump kernel
>>>>>>>>> ```
>>>>>>>>>
>>>>>>>>> After revert this patch, verify that it works properly on QEMU riscv.
>>>>>>>>>
>>>>>>>>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv
>>>>>>>>> Signed-off-by: Mingzheng Xing <xingmingzheng@iscas.ac.cn>
>>>>>>>>> ---
>>>>>>>>
>>>>>>>> I do not understand, what branch is this for?  Why have you not cc:ed
>>>>>>>> any of the original developers here?  Why does Linus's tree not have the
>>>>>>>> same problem?  And the first sentence above does not make much sense as
>>>>>>>> a 6.6 change is merged into 6.7?
>>>>>>>
>>>>>>> Sorry, I'll try to explain it more clearly.
>>>>>>>
>>>>>>> This commit 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem
>>>>>>> on RISC-V") should not have existed because this patch has been merged into
>>>>>>> another larger patch [1]. Here is that complete series:
>>>>>>
>>>>>> What "larger patch"?  It is in Linus's tree, so it's not part of
>>>>>> something different, right?  I'm confused.
>>>>>>
>>>>>
>>>>> Hi, Greg
>>>>>
>>>>> The email Cc:ed to author Chen Jiahao was bounced by the system, so maybe
>>>>> we can wait for Baoquan He to confirm.
>>>>>
>>>>> This is indeed a bit confusing. The Fixes: tag in 1d6cd2146c2b58 is a false
>>>>> reference. If I understand correctly, this is similar to the following
>>>>> scenario:
>>>>>
>>>>> A Fixes B, B doesn't go into linus mainline. C contains A, C goes into linus
>>>>> mainline 6.7, and C has more reconstruction code. but A goes into 6.6, so
>>>>> it doesn't make sense for A to be in the mainline, and there's no C in 6.6
>>>>> but there's an A, thus resulting in an incomplete code that creates an error.
>>>>>
>>>>> The link I quoted [1] shows that Baoquan had expressed an opinion on this
>>>>> at the time.
>>>>>
>>>>> Link: https://lore.kernel.org/linux-riscv/ZSiQRDGLZk7lpakE@MiWiFi-R3L-srv [1]
>>>>
>>>> I'm sorry, but I still do not understand what I need to do here for a
>>>> stable branch.  Do I need to apply something?  Revert something?
>>>> Something else?
>>>
>>> Hi, Greg
>>>
>>> I saw Baoquan's reply in thread[1], thanks Baoquan for confirming.
>>>
>>> So I think the right thing to do would be just to REVERT the commit
>>> 1d6cd2146c2b ("riscv: kdump: fix crashkernel reserving problem on RISC-V")
>>> in the 6.6.y branch, which is exactly the patch I submitted. If I need to
>>> make changes to my commit message, feel free to let me know and I'll post
>>> the second version.
>>>
>>> Link: https://lore.kernel.org/stable/ZihbAYMOI4ylazpt@MiWiFi-R3L-srv [1]
>>
>> Can someone just send me a patch series showing EXACTLY what needs to be
>> done here, as I am _still_ confused.
> 
> I think Mingzheng's patch is good to apply in the 6.6.y stable branch.
> 
> Hi Mingzheng,
> 
> Can you resend this patch to Greg and stable@vger.kernel.org and CC me?
> I would like to Ack your patch, but can't find the original patch since
> you didn't cc me.

Hi, Greg, Baoquan,

I sent the second version [1]. Thank you for taking the time.

Link: https://lore.kernel.org/stable/20240430032403.19562-1-xingmingzheng@iscas.ac.cn [1]



Best wishes
Mingzheng

> 
> Thanks
> Baoquan