Re: [PATCH 24/30] panic: Refactor the panic path

On 4/27/22 15:49, Guilherme G. Piccoli wrote: > + crash_kexec_post_notifiers > + This was DEPRECATED - users should always prefer the This is DEPRECATED - users should always prefer the > + parameter "panic_notifiers_level" -

Re: [PATCH 20/30] panic: Add the panic informational notifier list

On Wed, Apr 27, 2022 at 07:49:14PM -0300, Guilherme G. Piccoli wrote: > The goal of this new panic notifier is to allow its users to > register callbacks to run earlier in the panic path than they > currently do. This aims at informational mechanisms, like dumping > kernel offsets and showing devic

Re: [PATCH 9/9] ptrace: Don't change __state

"Eric W. Biederman" writes: > diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h > index 3c8b34876744..1947c85aa9d9 100644 > --- a/include/linux/sched/signal.h > +++ b/include/linux/sched/signal.h > @@ -437,7 +437,8 @@ extern void signal_wake_up_state(struct task_struct *t,

[PATCH 30/30] um: Avoid duplicate call to kmsg_dump()

Currently the panic notifier panic_exit() calls kmsg_dump() and some console flushing routines - this makes sense since such panic notifier exits UserMode Linux and never returns. Happens that after a panic refactor, kmsg_dump() is now always called *before* the pre_reboot list of panic notifiers,

[PATCH 29/30] powerpc: ps3, pseries: Avoid duplicate call to kmsg_dump() on panic

Currently both pseries and ps3 are platforms that define special panic notifiers that run as callbacks inside powerpc generic panic notifier. In both cases kmsg_dump() is called, and the reason seems to be that both of these callbacks aims to effectively stop the machine, so nothing would execute a

[PATCH 28/30] panic: Unexport crash_kexec_post_notifiers

There is no users anymore of this variable that requires it to be "exported" in the headers; also, it was deprecated by the kernel parameter "panic_notifiers_level". Signed-off-by: Guilherme G. Piccoli --- include/linux/panic.h | 2 -- include/linux/panic_notifier.h | 1 - 2 files chang

[PATCH 27/30] powerpc: Do not force all panic notifiers to execute before kdump

Commit 06e629c25daa ("powerpc/fadump: Fix inaccurate CPU state info in vmcore generated with panic") introduced a hardcoded setting of kernel parameter "crash_kexec_post_notifiers", effectively forcing all the panic notifiers to execute earlier in the panic path, before kdump. The reason for that

[PATCH 26/30] Drivers: hv: Do not force all panic notifiers to execute before kdump

Since commit a11589563e96 ("x86/Hyper-V: Report crash register data or kmsg before running crash kernel") Hyper-V forcibly sets the kernel parameter "crash_kexec_post_notifiers"; with that, it did enforce the execution of *all* panic notifiers before kdump. The main reason behind that is that Hyper

[PATCH 25/30] panic, printk: Add console flush parameter and convert panic_print to a notifier

Currently the parameter "panic_print" relies in a function called directly on panic path; one of the flags the users can set for panic_print triggers a console replay mechanism, to show the entire kernel log buffer (from the beginning) in a panic event. Two problems with that: the dual nature of t

[PATCH 24/30] panic: Refactor the panic path

The panic() function is somewhat convoluted - a lot of changes were made over the years, adding comments that might be misleading/outdated now, it has a code structure that is a bit complex to follow, with lots of conditionals, for example. The panic notifier list is something else - a single list,

[PATCH 23/30] printk: kmsg_dump: Introduce helper to inform number of dumpers

Currently we don't have a way to check if there are dumpers set, except counting the list members maybe. This patch introduces a very simple helper to provide this information, by just keeping track of registered/unregistered kmsg dumpers. It's going to be used on the panic path in the subsequent p

[PATCH 22/30] panic: Introduce the panic post-reboot notifier list

Currently we have 3 notifier lists in the panic path, which will be wired in a way to allow the notifier callbacks to run in different moments at panic time, in a subsequent patch. But there is also an odd set of architecture calls hardcoded in the end of panic path, after the restart machinery. T

[PATCH 21/30] panic: Introduce the panic pre-reboot notifier list

This patch renames the panic_notifier_list to panic_pre_reboot_list; the idea is that a subsequent patch will refactor the panic path in order to better split the notifiers, running some of them very early, some of them not so early [but still before kmsg_dump()] and finally, the rest should execut

[PATCH 20/30] panic: Add the panic informational notifier list

The goal of this new panic notifier is to allow its users to register callbacks to run earlier in the panic path than they currently do. This aims at informational mechanisms, like dumping kernel offsets and showing device error data (in case it's simple registers reading, for example) as well as m

[PATCH 19/30] panic: Add the panic hypervisor notifier list

The goal of this new panic notifier is to allow its users to register callbacks to run very early in the panic path. This aims hypervisor/FW notification mechanisms as well as simple LED functions, and any other simple and safe mechanism that should run early in the panic path; more dangerous callb

[PATCH 18/30] notifier: Show function names on notifier routines if DEBUG_NOTIFIERS is set

Currently we have a debug infrastructure in the notifiers file, but it's very simple/limited. This patch extends it by: (a) Showing all registered/unregistered notifiers' callback names; (b) Adding a dynamic debug tuning to allow showing called notifiers' function names. Notice that this should b

[PATCH 17/30] tracing: Improve panic/die notifiers

Currently the tracing dump_on_oops feature is implemented through separate notifiers, one for die/oops and the other for panic. With the addition of panic notifier "id", this patch makes use of such "id" to unify both functions. It also comments the function and changes the priority of the notifie

[PATCH 16/30] drivers/hv/vmbus, video/hyperv_fb: Untangle and refactor Hyper-V panic notifiers

Currently Hyper-V guests are among the most relevant users of the panic infrastructure, like panic notifiers, kmsg dumpers, etc. The reasons rely both in cleaning-up procedures (closing a hypervisor <-> guest connection, disabling a paravirtualized timer) as well as to data collection (sending pani

[PATCH 15/30] bus: brcmstb_gisb: Clean-up panic/die notifiers

This patch improves the panic/die notifiers in this driver by making use of a passed "id" instead of comparing pointer address; also, it removes an useless prototype declaration and unnecessary header inclusion. This is part of a panic notifiers refactor - this notifier in the future will be moved

[PATCH 14/30] panic: Properly identify the panic event to the notifiers' callbacks

The notifiers infrastructure provides a way to pass an "id" to the callbacks to determine what kind of event happened, i.e., what is the reason behind they getting called. The panic notifier currently pass 0, but this is soon to be used in a multi-targeted notifier, so let's pass a meaningful "id"

[PATCH 13/30] s390/consoles: Improve panic notifiers reliability

Currently many console drivers for s390 rely on panic/reboot notifiers to invoke callbacks on these events. The panic() function disables local IRQs, secondary CPUs and preemption, so callbacks invoked on panic are effectively running in atomic context. Happens that most of these console callbacks

[PATCH 12/30] parisc: Replace regular spinlock with spin_trylock on panic path

The panic notifiers' callbacks execute in an atomic context, with interrupts/preemption disabled, and all CPUs not running the panic function are off, so it's very dangerous to wait on a regular spinlock, there's a risk of deadlock. This patch refactors the panic notifier of parisc/power driver to

[PATCH 11/30] um: Improve panic notifiers consistency and ordering

Currently the panic notifiers from user mode linux don't follow the convention for most of the other notifiers present in the kernel (indentation, priority setting, numeric return). More important, the priorities could be improved, since it's a special case (userspace), hence we could run the notif

[PATCH 10/30] alpha: Clean-up the panic notifier code

The alpha panic notifier has some code issues, not following the conventions of other notifiers. Also, it might halt the machine but still it is set to run as early as possible, which doesn't seem to be a good idea. This patch cleans the code, and set the notifier to run as the latest, following t

[PATCH 09/30] coresight: cpu-debug: Replace mutex with mutex_trylock on panic notifier

The panic notifier infrastructure executes registered callbacks when a panic event happens - such callbacks are executed in atomic context, with interrupts and preemption disabled in the running CPU and all other CPUs disabled. That said, mutexes in such context are not a good idea. This patch rep

[PATCH 08/30] powerpc/setup: Refactor/untangle panic notifiers

The panic notifiers infrastructure is a bit limited in the scope of the callbacks - basically every kind of functionality is dropped in a list that runs in the same point during the kernel panic path. This is not really on par with the complexities and particularities of architecture / hypervisors'

[PATCH 07/30] mips: ip22: Reword PANICED to PANICKED and remove useless header

Many other place in the kernel prefer the latter, so let's keep it consistent in MIPS code as well. Also, removes a useless header. Cc: Thomas Bogendoerfer Signed-off-by: Guilherme G. Piccoli --- arch/mips/sgi-ip22/ip22-reset.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-)

[PATCH 06/30] soc: bcm: brcmstb: Document panic notifier action and remove useless header

The panic notifier of this driver is very simple code-wise, just a memory write to a special position with some numeric code. But this is not clear from the semantic point-of-view, and there is no public documentation about that either. After discussing this in the mailing-lists [0] and having Flo

[PATCH 05/30] misc/pvpanic: Convert regular spinlock into trylock on panic path

The pvpanic driver relies on panic notifiers to execute a callback on panic event. Such function is executed in atomic context - the panic function disables local IRQs, preemption and all other CPUs that aren't running the panic code. With that said, it's dangerous to use regular spinlocks in such

[PATCH 04/30] firmware: google: Convert regular spinlock into trylock on panic path

Currently the gsmi driver registers a panic notifier as well as reboot and die notifiers. The callbacks registered are called in atomic and very limited context - for instance, panic disables preemption, local IRQs and all other CPUs that aren't running the current panic function. With that said,

[PATCH 02/30] ARM: kexec: Disable IRQs/FIQs also on crash CPUs shutdown path

Currently the regular CPU shutdown path for ARM disables IRQs/FIQs in the secondary CPUs - smp_send_stop() calls ipi_cpu_stop(), which is responsible for that. This makes sense, since we're turning off such CPUs, putting them in an endless busy-wait loop. Problem is that there is an alternative pa

[PATCH 01/30] x86/crash, reboot: Avoid re-disabling VMX in all CPUs on crash/restart

In the panic path we have a list of functions to be called, the panic notifiers - such callbacks perform various actions in the machine's last breath, and sometimes users want them to run before kdump. We have the parameter "crash_kexec_post_notifiers" for that. When such parameter is used, the fun

[PATCH 03/30] notifier: Add panic notifiers info and purge trailing whitespaces

Although many notifiers are mentioned in the comments, the panic notifiers infrastructure is not. Also, the file contains some trailing whitespaces. This commit fix both issues. Cc: Arjan van de Ven Cc: Cong Wang Cc: Sebastian Andrzej Siewior Cc: Valentin Schneider Cc: Xiaoming Ni Signed-off-

Re: [PATCH 9/9] ptrace: Don't change __state

Oleg Nesterov writes: 2> On 04/26, Eric W. Biederman wrote: >> >> static void ptrace_unfreeze_traced(struct task_struct *task) >> { >> -if (READ_ONCE(task->__state) != __TASK_TRACED) >> +if (!(READ_ONCE(task->jobctl) & JOBCTL_DELAY_WAKEKILL)) >> return; >> >> WARN_ON(!

Re: [PATCH 6/9] signal: Always call do_notify_parent_cldstop with siglock held

Oleg Nesterov writes: > On 04/27, Oleg Nesterov wrote: >> >> On 04/26, Eric W. Biederman wrote: >> > >> > @@ -2209,6 +2213,34 @@ static int ptrace_stop(int exit_code, int why, int >> > clear_code, >> >spin_lock_irq(¤t->sighand->siglock); >> >} >> > >> > + /* Don't stop if curren

Re: [PATCH 9/9] ptrace: Don't change __state

Oleg Nesterov writes: > On 04/27, Oleg Nesterov wrote: >> >> On 04/27, Eric W. Biederman wrote: >> > >> > Oleg Nesterov writes: >> > >> > > On 04/26, Eric W. Biederman wrote: >> > >> >> > >> @@ -253,7 +252,7 @@ static int ptrace_check_attach(struct task_struct >> > >> *child, bool ignore_state)

Re: [PATCH 9/9] ptrace: Don't change __state

On 04/27, Oleg Nesterov wrote: > > On 04/27, Eric W. Biederman wrote: > > > > Oleg Nesterov writes: > > > > > On 04/26, Eric W. Biederman wrote: > > >> > > >> @@ -253,7 +252,7 @@ static int ptrace_check_attach(struct task_struct > > >> *child, bool ignore_state) > > >> */ > > >>

Re: [PATCH 9/9] ptrace: Don't change __state

On 04/27, Eric W. Biederman wrote: > > Oleg Nesterov writes: > > > On 04/26, Eric W. Biederman wrote: > >> > >> @@ -253,7 +252,7 @@ static int ptrace_check_attach(struct task_struct > >> *child, bool ignore_state) > >> */ > >>if (lock_task_sighand(child, &flags)) { > >>if (chi

Re: [PATCH 9/9] ptrace: Don't change __state

Oleg Nesterov writes: > On 04/26, Eric W. Biederman wrote: >> >> @@ -253,7 +252,7 @@ static int ptrace_check_attach(struct task_struct >> *child, bool ignore_state) >> */ >> if (lock_task_sighand(child, &flags)) { >> if (child->ptrace && child->parent == current) { >> -

Re: [PATCH 9/9] ptrace: Don't change __state

On 04/26, Eric W. Biederman wrote: > > @@ -253,7 +252,7 @@ static int ptrace_check_attach(struct task_struct *child, > bool ignore_state) >*/ > if (lock_task_sighand(child, &flags)) { > if (child->ptrace && child->parent == current) { > - WARN_ON(REA

Re: [PATCH 9/9] ptrace: Don't change __state

On 04/26, Eric W. Biederman wrote: > > static void ptrace_unfreeze_traced(struct task_struct *task) > { > - if (READ_ONCE(task->__state) != __TASK_TRACED) > + if (!(READ_ONCE(task->jobctl) & JOBCTL_DELAY_WAKEKILL)) > return; > > WARN_ON(!task->ptrace || task->parent !=

Re: [PATCH 8/9] ptrace: Use siglock instead of tasklist_lock in ptrace_check_attach

On 04/26, Eric W. Biederman wrote: > > + if (lock_task_sighand(child, &flags)) { > + if (child->ptrace && child->parent == current) { > + WARN_ON(READ_ONCE(child->__state) == __TASK_TRACED); > + /* > + * child->sighand can

Re: [PATCH 7/9] ptrace: Simplify the wait_task_inactive call in ptrace_check_attach

On 04/26, Eric W. Biederman wrote: > > Asking wait_task_inactive to verify that tsk->__state == __TASK_TRACED > was needed to detect the when ptrace_stop would decide not to stop > after calling "set_special_state(TASK_TRACED)". With the recent > cleanups ptrace_stop will always stop after calling

Re: [PATCH 6/9] signal: Always call do_notify_parent_cldstop with siglock held

On 04/27, Oleg Nesterov wrote: > > On 04/26, Eric W. Biederman wrote: > > > > @@ -2209,6 +2213,34 @@ static int ptrace_stop(int exit_code, int why, int > > clear_code, > > spin_lock_irq(¤t->sighand->siglock); > > } > > > > + /* Don't stop if current is not ptraced */ > > + if (

Re: [PATCH 6/9] signal: Always call do_notify_parent_cldstop with siglock held

On 04/26, Eric W. Biederman wrote: > > @@ -2209,6 +2213,34 @@ static int ptrace_stop(int exit_code, int why, int > clear_code, > spin_lock_irq(¤t->sighand->siglock); > } > > + /* Don't stop if current is not ptraced */ > + if (unlikely(!current->ptrace)) > +

Re: [PATCH 6/9] signal: Always call do_notify_parent_cldstop with siglock held

"Eric W. Biederman" writes: > Oleg Nesterov writes: > >> On 04/26, Eric W. Biederman wrote: >>> >>> @@ -2164,7 +2166,9 @@ static void do_notify_parent_cldstop(struct >>> task_struct *tsk, >>> } >>> >>> sighand = parent->sighand; >>> - spin_lock_irqsave(&sighand->siglock, flags); >>> +

Re: [PATCH 6/9] signal: Always call do_notify_parent_cldstop with siglock held

On 04/27, Eric W. Biederman wrote: > > The ptrace parental relationship definitely has the potential to be a > graph with cycles. Which as you point out is not fine. > > The result is very nice and I don't want to give it up. I suspect > something ptrace cycles are always a problem and can simply

Re: [PATCH 7/9] ptrace: Simplify the wait_task_inactive call in ptrace_check_attach

"Eric W. Biederman" writes: > "Eric W. Biederman" writes: > >> Asking wait_task_inactive to verify that tsk->__state == __TASK_TRACED >> was needed to detect the when ptrace_stop would decide not to stop >> after calling "set_special_state(TASK_TRACED)". With the recent >> cleanups ptrace_stop

Re: [PATCH 6/9] signal: Always call do_notify_parent_cldstop with siglock held

Oleg Nesterov writes: > On 04/26, Eric W. Biederman wrote: >> >> @@ -2164,7 +2166,9 @@ static void do_notify_parent_cldstop(struct >> task_struct *tsk, >> } >> >> sighand = parent->sighand; >> -spin_lock_irqsave(&sighand->siglock, flags); >> +lock = tsk->sighand != sighand; >>

Re: [PATCH 6/9] signal: Always call do_notify_parent_cldstop with siglock held

On 04/26, Eric W. Biederman wrote: > > @@ -2164,7 +2166,9 @@ static void do_notify_parent_cldstop(struct task_struct > *tsk, > } > > sighand = parent->sighand; > - spin_lock_irqsave(&sighand->siglock, flags); > + lock = tsk->sighand != sighand; > + if (lock) > +

Re: [PATCH 3/9] ptrace/um: Replace PT_DTRACE with TIF_SINGLESTEP

Johannes Berg writes: > On Tue, 2022-04-26 at 17:52 -0500, Eric W. Biederman wrote: >> User mode linux is the last user of the PT_DTRACE flag. Using the flag to >> indicate >> single stepping is a little confusing and worse changing tsk->ptrace without >> locking >> could potentionally cause p

Re: [PATCH 7/9] ptrace: Simplify the wait_task_inactive call in ptrace_check_attach

"Eric W. Biederman" writes: > Asking wait_task_inactive to verify that tsk->__state == __TASK_TRACED > was needed to detect the when ptrace_stop would decide not to stop > after calling "set_special_state(TASK_TRACED)". With the recent > cleanups ptrace_stop will always stop after calling set_sp

Re: [PATCH 5/9] signal: Protect parent child relationships by childs siglock

Sebastian Andrzej Siewior writes: > On 2022-04-26 17:52:07 [-0500], Eric W. Biederman wrote: >> The functions ptrace_stop and do_signal_stop have to drop siglock >> and grab tasklist_lock because the parent/child relation ship >> is guarded by siglock and not siglock. > > "is guarded by tasklist

Re: [PATCH 3/9] ptrace/um: Replace PT_DTRACE with TIF_SINGLESTEP

On Tue, 2022-04-26 at 17:52 -0500, Eric W. Biederman wrote: > User mode linux is the last user of the PT_DTRACE flag. Using the flag to > indicate > single stepping is a little confusing and worse changing tsk->ptrace without > locking > could potentionally cause problems. > > So use a thread i