Re: [RFC][PATCHv2 4/7] printk: make alt_printk available when config printk set
From: Petr Mladek
Date: Thu Oct 06 2016 - 11:23:49 EST
On Sat 2016-10-01 00:17:55, Sergey Senozhatsky wrote:
> alt_printk must be enabled on systems that have CONFIG_PRINTK set,
> while NMI related functions must depend on CONFIG_PRINTK_NMI.
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@xxxxxxxxx>
> ---
> include/linux/printk.h | 21 +++++++++++++++------
> kernel/printk/Makefile | 2 +-
> kernel/printk/alt_printk.c | 30 +++++++++++++++++++++++++-----
> 3 files changed, 41 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index 7510613..068a124 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -131,17 +131,11 @@ void early_printk(const char *s, ...) { }
> #endif
>
> #ifdef CONFIG_PRINTK_NMI
> -extern void alt_printk_init(void);
> extern void printk_nmi_enter(void);
> extern void printk_nmi_exit(void);
> -extern void alt_printk_flush(void);
> -extern void alt_printk_flush_on_panic(void);
> #else
> -static inline void alt_printk_init(void) { }
> static inline void printk_nmi_enter(void) { }
> static inline void printk_nmi_exit(void) { }
> -static inline void alt_printk_flush(void) { }
> -static inline void alt_printk_flush_on_panic(void) { }
> #endif /* PRINTK_NMI */
>
> #ifdef CONFIG_PRINTK
> @@ -193,6 +187,9 @@ void __init setup_log_buf(int early);
> __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...);
> void dump_stack_print_info(const char *log_lvl);
> void show_regs_print_info(const char *log_lvl);
> +extern void alt_printk_init(void);
> +extern void alt_printk_flush(void);
> +extern void alt_printk_flush_on_panic(void);
> #else
> static inline __printf(1, 0)
> int vprintk(const char *s, va_list args)
> @@ -252,6 +249,18 @@ static inline void dump_stack_print_info(const char *log_lvl)
> static inline void show_regs_print_info(const char *log_lvl)
> {
> }
> +
> +static inline void alt_printk_init(void)
> +{
> +}
> +
> +static inline void alt_printk_flush(void)
> +{
> +}
> +
> +static inline void alt_printk_flush_on_panic(void)
> +{
> +}
> #endif
>
> extern asmlinkage void dump_stack(void) __cold;
> diff --git a/kernel/printk/Makefile b/kernel/printk/Makefile
> index 3fc5e4e..d11838c 100644
> --- a/kernel/printk/Makefile
> +++ b/kernel/printk/Makefile
> @@ -1,3 +1,3 @@
> obj-y = printk.o
> -obj-$(CONFIG_PRINTK_NMI) += alt_printk.o
> +obj-$(CONFIG_PRINTK) += alt_printk.o
> obj-$(CONFIG_A11Y_BRAILLE_CONSOLE) += braille.o
> diff --git a/kernel/printk/alt_printk.c b/kernel/printk/alt_printk.c
> index 4bc1e7d..db0bfc8 100644
> --- a/kernel/printk/alt_printk.c
> +++ b/kernel/printk/alt_printk.c
> @@ -40,7 +40,6 @@
> * were handled or when IRQs are blocked.
> */
> static int alt_printk_irq_ready;
> -atomic_t nmi_message_lost;
>
> #define ALT_LOG_BUF_LEN ((1 << CONFIG_ALT_PRINTK_LOG_BUF_SHIFT) - \
> sizeof(atomic_t) - sizeof(struct irq_work))
> @@ -50,12 +49,16 @@ struct alt_printk_seq_buf {
> struct irq_work work; /* IRQ work that flushes the buffer */
> unsigned char buffer[ALT_LOG_BUF_LEN];
> };
> -static DEFINE_PER_CPU(struct alt_printk_seq_buf, nmi_print_seq);
>
> static DEFINE_PER_CPU(struct alt_printk_seq_buf, alt_print_seq);
> static DEFINE_PER_CPU(int, alt_printk_ctx);
> static DEFINE_PER_CPU(unsigned long, alt_printk_irq_flags);
>
> +#ifdef CONFIG_PRINTK_NMI
> +static DEFINE_PER_CPU(struct alt_printk_seq_buf, nmi_print_seq);
> +atomic_t nmi_message_lost;
> +#endif
> +
> static int alt_printk_log_store(struct alt_printk_seq_buf *s,
> const char *fmt, va_list args)
> {
> @@ -204,8 +207,12 @@ void alt_printk_flush(void)
> {
> int cpu;
>
> - for_each_possible_cpu(cpu)
> + for_each_possible_cpu(cpu) {
> +#ifdef CONFIG_PRINTK_NMI
> __alt_printk_flush(&per_cpu(nmi_print_seq, cpu).work);
> +#endif
> + __alt_printk_flush(&per_cpu(alt_print_seq, cpu).work);
This line should be added already in the 3rd patch.
> + }
> }
>
> /**
> @@ -235,6 +242,8 @@ void alt_printk_flush_on_panic(void)
> alt_printk_flush();
> }
>
> +#ifdef CONFIG_PRINTK_NMI
> +
> /*
> * Safe printk() for NMI context. It uses a per-CPU buffer to
> * store the message. NMIs are not nested, so there is always only
> @@ -263,6 +272,15 @@ void printk_nmi_exit(void)
> this_cpu_and(alt_printk_ctx, ~ALT_PRINTK_NMI_CONTEXT_MASK);
> }
>
> +#else
> +
> +static int vprintk_nmi(const char *fmt, va_list args)
> +{
> + return 0;
> +}
> +
> +#endif /* CONFIG_PRINTK_NMI */
> +
> /*
> * Lockless printk(), to avoid deadlocks should the printk() recurse
> * into itself. It uses a per-CPU buffer to store the message, just like
> @@ -317,12 +335,14 @@ void __init alt_printk_init(void)
> int cpu;
>
> for_each_possible_cpu(cpu) {
> - struct alt_printk_seq_buf *s = &per_cpu(nmi_print_seq, cpu);
> + struct alt_printk_seq_buf *s = &per_cpu(alt_print_seq, cpu);
>
> init_irq_work(&s->work, __alt_printk_flush);
>
> - s = &per_cpu(alt_print_seq, cpu);
> +#ifdef CONFIG_PRINTK_NMI
> + s = &per_cpu(nmi_print_seq, cpu);
> init_irq_work(&s->work, __alt_printk_flush);
> +#endif
Also the initialization of both irq works should be done already
in the 3rd patch.
BTW: I have tried to simulate the compilation on an architecture
without NMI (commented out HAVE_NMI in arch/x86/Kconfig
and I have got the following build error with the first three
patches only:
LD init/built-in.o
kernel/built-in.o: In function `vprintk':
/prace/kernel/linux/kernel/printk/printk.c:1914: undefined reference to `vprintk_func'
kernel/built-in.o: In function `printk':
/prace/kernel/linux/kernel/printk/printk.c:1976: undefined reference to `vprintk_func'
Makefile:949: recipe for target 'vmlinux' failed
Where the two lines are in:
asmlinkage int vprintk(const char *fmt, va_list args)
{
return vprintk_func(fmt, args);
}
and
asmlinkage __visible int printk(const char *fmt, ...)
{
va_list args;
int r;
va_start(args, fmt);
r = vprintk_func(fmt, args);
va_end(args);
return r;
}
I know that it is a pain to make it all correctly. I suffered from headaches
when preparing the WARN_*DEFERRED() patchset :-)
Best Regards,
Petr