Message-ID: <aTdxEvqY6Bcy7Wa1@MiWiFi-R3L-srv>
Date: Tue, 9 Dec 2025 08:45:06 +0800
From: Baoquan He <bhe@...hat.com>
To: Sourabh Jain <sourabhjain@...ux.ibm.com>,
	Qiang Ma <maqianga@...ontech.com>
Cc: akpm@...ux-foundation.org, kexec@...ts.infradead.org,
	linux-kernel@...r.kernel.org, Simon Horman <horms@...nel.org>
Subject: Re: [PATCH v3 0/3] kexec: print out debugging message if required
 for kexec_load

On 12/04/25 at 05:48pm, Sourabh Jain wrote:
> Hello All,
> 
> I read this patch series, including the older versions, and this is what I
> think about the changes.
> 
> There are two main reasons I see for why this patch series is proposed
> (these were not mentioned in the cover letter or commit messages):
> 
> 1. Commit eb7622d908a09 ("kexec_file, riscv: print out debugging message
>    if required") and commit 6f8c1da071a4 ("kexec_file, arm64: print out
>    debugging message if required") moved some kexec-related debug prints to
>    kexec_dprintk instead of pr_debug or pr_notice.
> 
>    These commits made it easier to control the updated debug prints using
>    the -d option of kexec tool for kexec_file_load system call. But since
>    kexec_dprintk is not available for the kexec_load system call, those
> prints
>    are no longer available for kexec_load at all. So patch series try to
> bring
>    those logs for kexec_load system call.
> 
> 2. Patch 03/03 prints additional logs about the kexec image type, start,
>     head, and flags via kexec_dprintk for the kexec_load system call.
> 
> This is done by enabling the kexec_dprintk for kexec_load, so that logs
> from point 1 and the new logs from point 2 can be printed for kexec_load
> system call.

Thanks a lot for checking these, Soubarh

I thought Qiang had found some missing information and he needed them
when debugging kexec_load interface until I noticed he printed the
segments too in kernel. The segments has been printed in userspace
kexec-tools. After talking, Qiang said he doesn't encounter missing
information when debugging kexec_load interface, he added the segment
printing antoher time in kernel for comparing them with the segments
printed out in userspace kexec-tools.

Basically, when I use 'kexec -d' to debug kexec_load loading, no extra
information is needed. Adding printing in kernel will make them shown in
console, but not in term which is not in the same place.

Anyway, the patch 2 and 3 adding more debug printing to kexec_load
interface in kernel is gilding the lily, drawing legs on a snake.

Qiang,

If you stick to add it, please post patch 1 separately because it's a
code fixing patch which is great. And split patch 2 and 3 to another
patchset and post separately and seek people's ACK.

> 
> However, the patch series does not explain why these additional debug logs
> are needed for kexec_load. Since most of the work involved in loading
> kexec images through kexec_load happens in the kexec userspace tool, and
> the tool already provides enough logs to debug issues, the need for these
> extra kernel logs is not clear.
> 
> So adding the need for the additional debug logs in the cover letter or
> commit message is needed to decide whether it is worth adding the code
> change proposed in this patch series Also, I found it hard to map the commit
> message to the changes introduced in the commits. And finally If I review
> the patches only based on the code changes: 01/03 The commit message refers
> to kexec_dbg_print as kexec_core_dbg_print. 02/03 I think kexec_dbg_print
> should be set to false at the end of kimage_free, not at the start. 03/03
> There is duplication of debug logs for architectures that already print the
> same information. For example, arm64. - Sourabh Jain
> On 26/11/25 14:14, Qiang Ma wrote:
> > Overview:
> > =========
> > The commit a85ee18c7900 ("kexec_file: print out debugging message
> > if required") has added general code printing in kexec_file_load(),
> > but not in kexec_load().
> > Since kexec_load and kexec_file_load are not triggered simultaneously,
> > we can unify the debug flag of kexec and kexec_file as kexec_dbg_print.
> > Next, we need to do some things in this patchset:
> > 1. rename kexec_file_dbg_print to kexec_dbg_print
> > 2. Add KEXEC_DEBUG
> > 3. Initialize kexec_dbg_print for kexec
> > 4. Fix uninitialized struct kimage *image pointer
> > 5. Set the reset of kexec_dbg_print to kimage_free
> > 
> > Testing:
> > =========
> > I did testing on x86_64, arm64 and loongarch. On x86_64, the printed messages
> > look like below:
> > 
> > unset CONFIG_KEXEC_FILE:
> > [   81.502374] kexec: kexec_load: type:0, start:0x23fff7700 head:0x10a4b9002 flags:0x3e0010
> > 
> > set CONFIG_KEXEC_FILE
> > [   36.774228] kexec_file: kernel: 0000000066c386c8 kernel_size: 0xd78400
> > [   36.821814] kexec-bzImage64: Loaded purgatory at 0x23fffb000
> > [   36.821826] kexec-bzImage64: Loaded boot_param, command line and misc at 0x23fff9000 bufsz=0x12d0 memsz=0x2000
> > [   36.821829] kexec-bzImage64: Loaded 64bit kernel at 0x23d400000 bufsz=0xd73400 memsz=0x2ab7000
> > [   36.821918] kexec-bzImage64: Loaded initrd at 0x23bd0b000 bufsz=0x16f40a8 memsz=0x16f40a8
> > [   36.821920] kexec-bzImage64: Final command line is: root=/dev/mapper/test-root crashkernel=auto rd.lvm.lv=test/root
> > [   36.821925] kexec-bzImage64: E820 memmap:
> > [   36.821926] kexec-bzImage64: 0000000000000000-000000000009ffff (1)
> > [   36.821928] kexec-bzImage64: 0000000000100000-0000000000811fff (1)
> > [   36.821930] kexec-bzImage64: 0000000000812000-0000000000812fff (2)
> > [   36.821931] kexec-bzImage64: 0000000000813000-00000000bee38fff (1)
> > [   36.821933] kexec-bzImage64: 00000000bee39000-00000000beec2fff (2)
> > [   36.821934] kexec-bzImage64: 00000000beec3000-00000000bf8ecfff (1)
> > [   36.821935] kexec-bzImage64: 00000000bf8ed000-00000000bfb6cfff (2)
> > [   36.821936] kexec-bzImage64: 00000000bfb6d000-00000000bfb7efff (3)
> > [   36.821937] kexec-bzImage64: 00000000bfb7f000-00000000bfbfefff (4)
> > [   36.821938] kexec-bzImage64: 00000000bfbff000-00000000bff7bfff (1)
> > [   36.821939] kexec-bzImage64: 00000000bff7c000-00000000bfffffff (2)
> > [   36.821940] kexec-bzImage64: 00000000feffc000-00000000feffffff (2)
> > [   36.821941] kexec-bzImage64: 00000000ffc00000-00000000ffffffff (2)
> > [   36.821942] kexec-bzImage64: 0000000100000000-000000023fffffff (1)
> > [   36.872348] kexec_file: nr_segments = 4
> > [   36.872356] kexec_file: segment[0]: buf=0x000000005314ece7 bufsz=0x4000 mem=0x23fffb000 memsz=0x5000
> > [   36.872370] kexec_file: segment[1]: buf=0x000000006e59b143 bufsz=0x12d0 mem=0x23fff9000 memsz=0x2000
> > [   36.872374] kexec_file: segment[2]: buf=0x00000000eb7b1fc3 bufsz=0xd73400 mem=0x23d400000 memsz=0x2ab7000
> > [   36.882172] kexec_file: segment[3]: buf=0x000000006af76441 bufsz=0x16f40a8 mem=0x23bd0b000 memsz=0x16f5000
> > [   36.889113] kexec_file: kexec_file_load: type:0, start:0x23fffb150 head:0x101a2e002 flags:0x8
> > 
> > Changes in v3:
> > ==========
> > - Rename kexec_core_dbg_print to kexec_dbg_print
> > - Remove unnecessary segments prints
> > - Remove patch "kexec_file: Fix the issue of mismatch between loop variable types"
> > 
> > Qiang Ma (3):
> >    kexec: Fix uninitialized struct kimage *image pointer
> >    kexec: add kexec flag to control debug printing
> >    kexec: print out debugging message if required for kexec_load
> > 
> >   include/linux/kexec.h      | 9 +++++----
> >   include/uapi/linux/kexec.h | 1 +
> >   kernel/kexec.c             | 8 +++++++-
> >   kernel/kexec_core.c        | 4 +++-
> >   kernel/kexec_file.c        | 4 +---
> >   5 files changed, 17 insertions(+), 9 deletions(-)
> > 
> 

Powered by blists - more mailing lists