2022-01-27 02:57:09

by Stephen Brennan

[permalink] [raw]
Subject: [PATCH v2 1/4] panic: Add panic_in_progress helper

This helper will be used in printk code to avoid deadlocks during
panic().

Suggested-by: Petr Mladek <[email protected]>
Signed-off-by: Stephen Brennan <[email protected]>
---

Notes:
v2: Switch to static inline function

include/linux/panic.h | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/include/linux/panic.h b/include/linux/panic.h
index f5844908a089..1022ec930d34 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -4,6 +4,7 @@

#include <linux/compiler_attributes.h>
#include <linux/types.h>
+#include <linux/atomic.h>

struct pt_regs;

@@ -45,6 +46,11 @@ extern bool crash_kexec_post_notifiers;
extern atomic_t panic_cpu;
#define PANIC_CPU_INVALID -1

+static inline bool panic_in_progress(void)
+{
+ return unlikely(atomic_read(&panic_cpu) != PANIC_CPU_INVALID);
+}
+
/*
* Only to be used by arch init code. If the user over-wrote the default
* CONFIG_PANIC_TIMEOUT, honor it.
--
2.30.2


2022-01-28 06:20:18

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] panic: Add panic_in_progress helper

On Wed 2022-01-26 15:02:33, Stephen Brennan wrote:
> This helper will be used in printk code to avoid deadlocks during
> panic().
>
> Suggested-by: Petr Mladek <[email protected]>
> Signed-off-by: Stephen Brennan <[email protected]>

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr

2022-01-28 08:38:36

by Stephen Brennan

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] panic: Add panic_in_progress helper

On 1/27/22 06:34, Petr Mladek wrote:
> On Wed 2022-01-26 15:02:33, Stephen Brennan wrote:
>> This helper will be used in printk code to avoid deadlocks during
>> panic().
>>
>> Suggested-by: Petr Mladek <[email protected]>
>> Signed-off-by: Stephen Brennan <[email protected]>
>
> Reviewed-by: Petr Mladek <[email protected]>
>
> Best Regards,
> Petr

Hi Petr,

Thanks for the review, however over the night I received two kernel test
robot emails. One indicating a new build error caused by this change on
m68k arch, and the other adding a new warning on riscv. From what I can
tell, the issues are circular dependencies in #includes. So it may be
better to either return to the macro, or move this static inline down to
kernel/printk/printk.c. I think moving it into kernel/printk/printk.c
makes most sense given that the macro requires the correct #includes anyway.

Thanks,
Stephen

------------- m68k

In file included from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from arch/m68k/include/asm/irqflags.h:6,
from include/linux/irqflags.h:16,
from arch/m68k/include/asm/atomic.h:6,
from include/linux/atomic.h:7,
from include/linux/panic.h:7,
from include/asm-generic/bug.h:21,
from arch/m68k/include/asm/bug.h:32,
from include/linux/bug.h:5,
from include/linux/page-flags.h:10,
from kernel/bounds.c:10:
include/linux/thread_info.h: In function 'copy_overflow':
>> include/linux/thread_info.h:214:9: error: implicit declaration of
function 'WARN' [-Werror=implicit-function-declaration]
214 | WARN(1, "Buffer overflow detected (%d < %lu)!\n",
size, count);
| ^~~~
include/linux/thread_info.h: In function 'check_copy_size':
>> include/linux/thread_info.h:230:13: error: implicit declaration of
function 'WARN_ON_ONCE' [-Werror=implicit-function-declaration]
230 | if (WARN_ON_ONCE(bytes > INT_MAX))
| ^~~~~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [scripts/Makefile.build:121: kernel/bounds.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1191: prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:219: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.

----------------- riscv:

In file included from arch/riscv/include/asm/atomic.h:19,
from include/linux/atomic.h:7,
from include/linux/panic.h:7,
from include/asm-generic/bug.h:21,
from arch/riscv/include/asm/bug.h:83,
from include/linux/bug.h:5,
from include/linux/page-flags.h:10,
from kernel/bounds.c:10:
arch/riscv/include/asm/atomic.h: In function 'arch_atomic_xchg_relaxed':
>> arch/riscv/include/asm/cmpxchg.h:35:17: error: implicit declaration
of function 'BUILD_BUG' [-Werror=implicit-function-declaration]
35 | BUILD_BUG();
\
| ^~~~~~~~~
arch/riscv/include/asm/atomic.h:249:16: note: in expansion of macro
'__xchg_relaxed'
249 | return __xchg_relaxed(&(v->counter), n, size);
\
| ^~~~~~~~~~~~~~
arch/riscv/include/asm/atomic.h:295:9: note: in expansion of macro
'ATOMIC_OP'
295 | ATOMIC_OP(int, , 4)
\
| ^~~~~~~~~
arch/riscv/include/asm/atomic.h:299:1: note: in expansion of macro
'ATOMIC_OPS'
299 | ATOMIC_OPS()
| ^~~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [scripts/Makefile.build:121: kernel/bounds.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1191: prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:219: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.

2022-01-30 14:40:27

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] panic: Add panic_in_progress helper

On Thu 2022-01-27 08:02:19, Stephen Brennan wrote:
> On 1/27/22 06:34, Petr Mladek wrote:
> > On Wed 2022-01-26 15:02:33, Stephen Brennan wrote:
> > > This helper will be used in printk code to avoid deadlocks during
> > > panic().
> > >
> > > Suggested-by: Petr Mladek <[email protected]>
> > > Signed-off-by: Stephen Brennan <[email protected]>
> >
> > Reviewed-by: Petr Mladek <[email protected]>
> >
> > Best Regards,
> > Petr
>
> Hi Petr,
>
> Thanks for the review, however over the night I received two kernel test
> robot emails. One indicating a new build error caused by this change on m68k
> arch, and the other adding a new warning on riscv. From what I can tell, the
> issues are circular dependencies in #includes. So it may be better to either
> return to the macro, or move this static inline down to
> kernel/printk/printk.c. I think moving it into kernel/printk/printk.c makes
> most sense given that the macro requires the correct #includes anyway.

Yes, I prefer moving the inline down to printk.c. It looks a bit
cleaner than the macro that would not work without another include.

Please, explain this in the commit message.

Best Regards,
Petr