Hi,
On 13. 03. 24, 1:48, Baoquan He wrote:
Hi Jiri,
On 03/12/24 at 10:58am, Jiri Slaby wrote:
On 13. 12. 23, 6:57, Baoquan He wrote:
... snip...
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
...
@@ -500,6 +500,13 @@ static inline int crash_hotplug_memory_support(void) { return 0; }
static inline unsigned int crash_get_elfcorehdr_size(void) { return 0; }
#endif
+extern bool kexec_file_dbg_print;
+
+#define kexec_dprintk(fmt, ...) \
+ printk("%s" fmt, \
+ kexec_file_dbg_print ? KERN_INFO : KERN_DEBUG, \
+ ##__VA_ARGS__)
This means you dump it _always_. Only with different levels.
It dumped always too with pr_debug() before, I just add a switch to
control it's pr_info() or pr_debug().
Not really, see below.
And without any prefix whatsoever, so people see bloat like this in their
log now:
[ +0.000001] 0000000000001000-000000000009ffff (1)
[ +0.000002] 000000007f96d000-000000007f97efff (3)
[ +0.000002] 0000000000800000-0000000000807fff (4)
[ +0.000001] 000000000080b000-000000000080bfff (4)
[ +0.000002] 0000000000810000-00000000008fffff (4)
[ +0.000001] 000000007f97f000-000000007f9fefff (4)
[ +0.000001] 000000007ff00000-000000007fffffff (4)
[ +0.000002] 0000000000000000-0000000000000fff (2)
On which arch are you seeing this? There should be one line above these
range printing to tell what they are, like:
E820 memmap:
Ah this is there too. It's a lot of output, so I took it out of context,
apparently.
0000000000000000-000000000009a3ff (1)
000000000009a400-000000000009ffff (2)
00000000000e0000-00000000000fffff (2)
0000000000100000-000000006ff83fff (1)
000000006ff84000-000000007ac50fff (2)
It should all be prefixed like kdump: or kexec: in any way.
without actually knowing what that is.
There should be nothing logged if that is not asked for and especially if
kexec load went fine, right?
Right. Before this patch, those pr_debug() were already there. You need
enable them to print out like add '#define DEBUG' in *.c file, or enable
the dynamic debugging of the file or function.
I think it's perfectly fine for DEBUG builds to print this out. And many
(all major?) distros use dyndbg, so it used to print nothing by default.
With this patch applied,
you only need specify '-d' when you execute kexec command with
kexec_file load interface, like:
kexec -s -l -d /boot/vmlinuz-xxxx.img --initrd xxx.img --reuse-cmdline
Perhaps our (SUSE) tooling passes -d? But I am seeing this every time I
boot.
No, it does not seem so:
load.sh[915]: Starting kdump kernel load; kexec cmdline: /sbin/kexec -p
/var/lib/kdump/kernel --append=" loglevel=7 console=tty0 console=ttyS0
video=1920x1080,1024x768,800x600 oops=panic
lsm=lockdown,capability,integrity,selinux sysrq=yes reset_devices
acpi_no_memhotplug cgroup_disable=memory nokaslr numa=off irqpoll
nr_cpus=1 root=kdump rootflags=bind rd.udev.children-max=8
disable_cpu_apicid=0 panic=1" --initrd=/var/lib/kdump/initrd -a
For kexec_file load, it is not logging if not specifying '-d', unless
you take way to make pr_debug() work in that file.
So is -d detection malfunctioning under some circumstances?
Can this be redesigned, please?
Sure, after making clear what's going on with this, I will try.
Actually what was wrong on the pr_debug()s? Can you simply turn them on from
the kernel when -d is passed to kexec instead of all this?
Joe suggested this during v1 reviewing:
https://lore.kernel.org/all/1e7863ec4e4ab10b84fd0e64f30f8464d2e484a3.camel@xxxxxxxxxxx/T/#u
...
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -52,6 +52,8 @@ atomic_t __kexec_lock = ATOMIC_INIT(0);
/* Flag to indicate we are going to kexec a new kernel */
bool kexec_in_progress = false;
+bool kexec_file_dbg_print;
Ugh, and a global flag for this?
Yeah, kexec_file_dbg_print records if '-d' is specified when 'kexec'
command executed. Anything wrong with the global flag?
Global variables are frowned upon. To cite coding style: unless you
**really** need them. Here, it looks like you do not.
thanks,
--
js
suse labs
