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@xxxxxxxxxxx>
>>>> ---
>>>
>>> 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
