diff mbox series

[1/7] kexec_file: add kexec_file flag to control debug printing

Message ID 20231114153253.241262-2-bhe@redhat.com (mailing list archive)
State Handled Elsewhere
Headers show
Series kexec_file: print out debugging message if required | expand

Commit Message

Baoquan He Nov. 14, 2023, 3:32 p.m. UTC
When specifying 'kexec -c -d', kexec_load interface will print loading
information, e.g the regions where kernel/initrd/purgatory/cmdline
are put, the memmap passed to 2nd kernel taken as system RAM ranges,
and printing all contents of struct kexec_segment, etc. These are
very helpful for analyzing or positioning what's happening when
kexec/kdump itself failed. The debugging printing for kexec_load
interface is made in user space utility kexec-tools.

Whereas, with kexec_file_load interface, 'kexec -s -d' print nothing.
Because kexec_file code is mostly implemented in kernel space, and the
debugging printing functionality is missed. It's not convenient when
debugging kexec/kdump loading and jumping with kexec_file_load
interface.

Now add KEXEC_FILE_DEBUG to kexec_file flag to control the debugging
message printing. And add global variable kexec_file_dbg_print and
macro kexec_dprintk() to facilitate the printing.

This is a preparation, later kexec_dprintk() will be used to replace the
existing pr_debug(). Once 'kexec -s -d' is specified, it will print out
kexec/kdump loading information. If '-d' is not specified, it regresses
to pr_debug().

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 include/linux/kexec.h      | 14 +++++++++++++-
 include/uapi/linux/kexec.h |  1 +
 kernel/kexec_file.c        |  5 +++++
 3 files changed, 19 insertions(+), 1 deletion(-)

Comments

Joe Perches Nov. 14, 2023, 4:03 p.m. UTC | #1
On Tue, 2023-11-14 at 23:32 +0800, Baoquan He wrote:
> When specifying 'kexec -c -d', kexec_load interface will print loading
> information, e.g the regions where kernel/initrd/purgatory/cmdline
> are put, the memmap passed to 2nd kernel taken as system RAM ranges,
> and printing all contents of struct kexec_segment, etc. These are
> very helpful for analyzing or positioning what's happening when
> kexec/kdump itself failed. The debugging printing for kexec_load
> interface is made in user space utility kexec-tools.
> 
> Whereas, with kexec_file_load interface, 'kexec -s -d' print nothing.
> Because kexec_file code is mostly implemented in kernel space, and the
> debugging printing functionality is missed. It's not convenient when
> debugging kexec/kdump loading and jumping with kexec_file_load
> interface.
> 
> Now add KEXEC_FILE_DEBUG to kexec_file flag to control the debugging
> message printing. And add global variable kexec_file_dbg_print and
> macro kexec_dprintk() to facilitate the printing.
> 
> This is a preparation, later kexec_dprintk() will be used to replace the
> existing pr_debug(). Once 'kexec -s -d' is specified, it will print out
> kexec/kdump loading information. If '-d' is not specified, it regresses
> to pr_debug().

Not quite as pr_debug is completely eliminated with
zero object size when DEBUG is not #defined.

Now the object size will be larger and contain the
formats in .text.

[]
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
[]
> @@ -264,6 +264,18 @@ arch_kexec_apply_relocations(struct purgatory_info *pi, Elf_Shdr *section,
>  	return -ENOEXEC;
>  }
>  #endif
> +
> +extern bool kexec_file_dbg_print;
> +
> +#define kexec_dprintk(fmt, args...)			\
> +	do {						\
> +		if (kexec_file_dbg_print)		\
> +			printk(KERN_INFO fmt, ##args);	\
> +		else					\
> +			printk(KERN_DEBUG fmt, ##args);	\
> +	} while (0)
> +
> +

I don't know how many of these printks exist and if
overall object size matters but using

#define kexec_dprintkfmt, ...)					\
	printk("%s" fmt,					\
	       kexec_file_dbg_print ? KERN_INFO : KERN_DEBUG,	\
	       ##__VA_ARGS__)

should reduce overall object size by eliminating the
mostly duplicated format in .text which differs only
by the KERN_<PREFIX>
Baoquan He Nov. 15, 2023, 12:58 a.m. UTC | #2
On 11/14/23 at 08:03am, Joe Perches wrote:
> On Tue, 2023-11-14 at 23:32 +0800, Baoquan He wrote:
> > When specifying 'kexec -c -d', kexec_load interface will print loading
> > information, e.g the regions where kernel/initrd/purgatory/cmdline
> > are put, the memmap passed to 2nd kernel taken as system RAM ranges,
> > and printing all contents of struct kexec_segment, etc. These are
> > very helpful for analyzing or positioning what's happening when
> > kexec/kdump itself failed. The debugging printing for kexec_load
> > interface is made in user space utility kexec-tools.
> > 
> > Whereas, with kexec_file_load interface, 'kexec -s -d' print nothing.
> > Because kexec_file code is mostly implemented in kernel space, and the
> > debugging printing functionality is missed. It's not convenient when
> > debugging kexec/kdump loading and jumping with kexec_file_load
> > interface.
> > 
> > Now add KEXEC_FILE_DEBUG to kexec_file flag to control the debugging
> > message printing. And add global variable kexec_file_dbg_print and
> > macro kexec_dprintk() to facilitate the printing.
> > 
> > This is a preparation, later kexec_dprintk() will be used to replace the
> > existing pr_debug(). Once 'kexec -s -d' is specified, it will print out
> > kexec/kdump loading information. If '-d' is not specified, it regresses
> > to pr_debug().
> 
> Not quite as pr_debug is completely eliminated with
> zero object size when DEBUG is not #defined.
> 
> Now the object size will be larger and contain the
> formats in .text.

Ah, I didn't realize that. Thanks for telling. I didn't take pr_info()
and pr_debug because I want to avoid printing the pr_fmt() string in
each file.

> 
> []
> > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> []
> > @@ -264,6 +264,18 @@ arch_kexec_apply_relocations(struct purgatory_info *pi, Elf_Shdr *section,
> >  	return -ENOEXEC;
> >  }
> >  #endif
> > +
> > +extern bool kexec_file_dbg_print;
> > +
> > +#define kexec_dprintk(fmt, args...)			\
> > +	do {						\
> > +		if (kexec_file_dbg_print)		\
> > +			printk(KERN_INFO fmt, ##args);	\
> > +		else					\
> > +			printk(KERN_DEBUG fmt, ##args);	\
> > +	} while (0)
> > +
> > +
> 
> I don't know how many of these printks exist and if
> overall object size matters but using

Not too much because they are spread in different arch.
> 
> #define kexec_dprintkfmt, ...)					\
> 	printk("%s" fmt,					\
> 	       kexec_file_dbg_print ? KERN_INFO : KERN_DEBUG,	\
> 	       ##__VA_ARGS__)
> 
> should reduce overall object size by eliminating the
> mostly duplicated format in .text which differs only
> by the KERN_<PREFIX>

Sure, the new one looks great to me, I will update code to take it.
Thanks a lot for your great suggestion.

Thanks
Baoquan
Baoquan He Nov. 15, 2023, 12:51 p.m. UTC | #3
When specifying 'kexec -c -d', kexec_load interface will print loading
information, e.g the regions where kernel/initrd/purgatory/cmdline
are put, the memmap passed to 2nd kernel taken as system RAM ranges,
and printing all contents of struct kexec_segment, etc. These are
very helpful for analyzing or positioning what's happening when
kexec/kdump itself failed. The debugging printing for kexec_load
interface is made in user space utility kexec-tools.

Whereas, with kexec_file_load interface, 'kexec -s -d' print nothing.
Because kexec_file code is mostly implemented in kernel space, and the
debugging printing functionality is missed. It's not convenient when
debugging kexec/kdump loading and jumping with kexec_file_load
interface.

Now add KEXEC_FILE_DEBUG to kexec_file flag to control the debugging
message printing. And add global variable kexec_file_dbg_print and
macro kexec_dprintk() to facilitate the printing.

This is a preparation, later kexec_dprintk() will be used to replace the
existing pr_debug(). Once 'kexec -s -d' is specified, it will print out
kexec/kdump loading information at KERN_INFO level. If '-d' is not
specified, it regresses to print message at KERN_DEBUG level.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
v1->v2:
- Rewrite the kexec_dprintk() macro as Joe suggested to reduce kernel
  text size.

 include/linux/kexec.h      | 10 +++++++++-
 include/uapi/linux/kexec.h |  1 +
 kernel/kexec_file.c        |  5 +++++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 8227455192b7..66997efe36f1 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -264,6 +264,14 @@ arch_kexec_apply_relocations(struct purgatory_info *pi, Elf_Shdr *section,
 	return -ENOEXEC;
 }
 #endif
+
+extern bool kexec_file_dbg_print;
+
+#define kexec_dprintk(fmt, ...)					\
+	printk("%s" fmt,					\
+	       kexec_file_dbg_print ? KERN_INFO : KERN_DEBUG,	\
+	       ##__VA_ARGS__)
+
 #endif /* CONFIG_KEXEC_FILE */
 
 #ifdef CONFIG_KEXEC_ELF
@@ -403,7 +411,7 @@ bool kexec_load_permitted(int kexec_image_type);
 
 /* List of defined/legal kexec file flags */
 #define KEXEC_FILE_FLAGS	(KEXEC_FILE_UNLOAD | KEXEC_FILE_ON_CRASH | \
-				 KEXEC_FILE_NO_INITRAMFS)
+				 KEXEC_FILE_NO_INITRAMFS | KEXEC_FILE_DEBUG)
 
 /* flag to track if kexec reboot is in progress */
 extern bool kexec_in_progress;
diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
index 01766dd839b0..c17bb096ea68 100644
--- a/include/uapi/linux/kexec.h
+++ b/include/uapi/linux/kexec.h
@@ -25,6 +25,7 @@
 #define KEXEC_FILE_UNLOAD	0x00000001
 #define KEXEC_FILE_ON_CRASH	0x00000002
 #define KEXEC_FILE_NO_INITRAMFS	0x00000004
+#define KEXEC_FILE_DEBUG	0x00000008
 
 /* These values match the ELF architecture values.
  * Unless there is a good reason that should continue to be the case.
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f9a419cd22d4..4c35500ae40a 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -38,6 +38,8 @@ void set_kexec_sig_enforced(void)
 }
 #endif
 
+bool kexec_file_dbg_print;
+
 static int kexec_calculate_store_digests(struct kimage *image);
 
 /* Maximum size in bytes for kernel/initrd files. */
@@ -123,6 +125,8 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 	 */
 	kfree(image->image_loader_data);
 	image->image_loader_data = NULL;
+
+	kexec_file_dbg_print = false;
 }
 
 #ifdef CONFIG_KEXEC_SIG
@@ -278,6 +282,7 @@ kimage_file_alloc_init(struct kimage **rimage, int kernel_fd,
 	if (!image)
 		return -ENOMEM;
 
+	kexec_file_dbg_print = !!(flags & KEXEC_FILE_DEBUG);
 	image->file_mode = 1;
 
 	if (kexec_on_panic) {
diff mbox series

Patch

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 8227455192b7..189a6c5bec84 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -264,6 +264,18 @@  arch_kexec_apply_relocations(struct purgatory_info *pi, Elf_Shdr *section,
 	return -ENOEXEC;
 }
 #endif
+
+extern bool kexec_file_dbg_print;
+
+#define kexec_dprintk(fmt, args...)			\
+	do {						\
+		if (kexec_file_dbg_print)		\
+			printk(KERN_INFO fmt, ##args);	\
+		else					\
+			printk(KERN_DEBUG fmt, ##args);	\
+	} while (0)
+
+
 #endif /* CONFIG_KEXEC_FILE */
 
 #ifdef CONFIG_KEXEC_ELF
@@ -403,7 +415,7 @@  bool kexec_load_permitted(int kexec_image_type);
 
 /* List of defined/legal kexec file flags */
 #define KEXEC_FILE_FLAGS	(KEXEC_FILE_UNLOAD | KEXEC_FILE_ON_CRASH | \
-				 KEXEC_FILE_NO_INITRAMFS)
+				 KEXEC_FILE_NO_INITRAMFS | KEXEC_FILE_DEBUG)
 
 /* flag to track if kexec reboot is in progress */
 extern bool kexec_in_progress;
diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h
index 01766dd839b0..c17bb096ea68 100644
--- a/include/uapi/linux/kexec.h
+++ b/include/uapi/linux/kexec.h
@@ -25,6 +25,7 @@ 
 #define KEXEC_FILE_UNLOAD	0x00000001
 #define KEXEC_FILE_ON_CRASH	0x00000002
 #define KEXEC_FILE_NO_INITRAMFS	0x00000004
+#define KEXEC_FILE_DEBUG	0x00000008
 
 /* These values match the ELF architecture values.
  * Unless there is a good reason that should continue to be the case.
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f9a419cd22d4..4c35500ae40a 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -38,6 +38,8 @@  void set_kexec_sig_enforced(void)
 }
 #endif
 
+bool kexec_file_dbg_print;
+
 static int kexec_calculate_store_digests(struct kimage *image);
 
 /* Maximum size in bytes for kernel/initrd files. */
@@ -123,6 +125,8 @@  void kimage_file_post_load_cleanup(struct kimage *image)
 	 */
 	kfree(image->image_loader_data);
 	image->image_loader_data = NULL;
+
+	kexec_file_dbg_print = false;
 }
 
 #ifdef CONFIG_KEXEC_SIG
@@ -278,6 +282,7 @@  kimage_file_alloc_init(struct kimage **rimage, int kernel_fd,
 	if (!image)
 		return -ENOMEM;
 
+	kexec_file_dbg_print = !!(flags & KEXEC_FILE_DEBUG);
 	image->file_mode = 1;
 
 	if (kexec_on_panic) {