LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Daniel Thompson <daniel.thompson@linaro.org>
To: Douglas Anderson <dianders@chromium.org>
Cc: Jason Wessel <jason.wessel@windriver.com>,
	tglx@linutronix.de, mingo@kernel.org, gregkh@linuxfoundation.org,
	linux-arm-msm@vger.kernel.org,
	kgdb-bugreport@lists.sourceforge.net, linux-mips@linux-mips.org,
	linux-sh@vger.kernel.org, peterz@infradead.org,
	linux-hexagon@vger.kernel.org, frederic@kernel.org,
	riel@surriel.com, linux-kernel@vger.kernel.org, luto@kernel.org,
	sparclinux@vger.kernel.org, linux-snps-arc@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 6/7] smp: Don't yell about IRQs disabled in kgdb_roundup_cpus()
Date: Tue, 30 Oct 2018 09:41:59 +0000	[thread overview]
Message-ID: <20181030094159.zt6akmyxwrbzoe2x@holly.lan> (raw)
In-Reply-To: <20181029180707.207546-7-dianders@chromium.org>

On Mon, Oct 29, 2018 at 11:07:06AM -0700, Douglas Anderson wrote:
> In kgdb_roundup_cpus() we've got code that looks like:
>   local_irq_enable();
>   smp_call_function(kgdb_call_nmi_hook, NULL, 0);
>   local_irq_disable();
> 
> In certain cases when we drop into kgdb (like with sysrq-g on a serial
> console) we'll get a big yell that looks like:
> 
>   sysrq: SysRq : DEBUG
>   ------------[ cut here ]------------
>   DEBUG_LOCKS_WARN_ON(current->hardirq_context)
>   WARNING: CPU: 0 PID: 0 at .../kernel/locking/lockdep.c:2875 lockdep_hardirqs_on+0xf0/0x160
>   CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.19.0 #27
>   pstate: 604003c9 (nZCv DAIF +PAN -UAO)
>   pc : lockdep_hardirqs_on+0xf0/0x160
>   ...
>   Call trace:
>    lockdep_hardirqs_on+0xf0/0x160
>    trace_hardirqs_on+0x188/0x1ac
>    kgdb_roundup_cpus+0x14/0x3c
>    kgdb_cpu_enter+0x53c/0x5cc
>    kgdb_handle_exception+0x180/0x1d4
>    kgdb_compiled_brk_fn+0x30/0x3c
>    brk_handler+0x134/0x178
>    do_debug_exception+0xfc/0x178
>    el1_dbg+0x18/0x78
>    kgdb_breakpoint+0x34/0x58
>    sysrq_handle_dbg+0x54/0x5c
>    __handle_sysrq+0x114/0x21c
>    handle_sysrq+0x30/0x3c
>    qcom_geni_serial_isr+0x2dc/0x30c
>   ...
>   ...
>   irq event stamp: ...45
>   hardirqs last  enabled at (...44): [...] __do_softirq+0xd8/0x4e4
>   hardirqs last disabled at (...45): [...] el1_irq+0x74/0x130
>   softirqs last  enabled at (...42): [...] _local_bh_enable+0x2c/0x34
>   softirqs last disabled at (...43): [...] irq_exit+0xa8/0x100
>   ---[ end trace adf21f830c46e638 ]---
> 
> Let's add kgdb to the list of reasons not to warn in
> smp_call_function_many().  That will allow us (in a future patch) to
> stop calling local_irq_enable() which will get rid of the original
> splat.
> 
> NOTE: with this change comes the obvious question: will we start
> deadlocking more often now when we drop into the debugger.  I can't
> say that for sure one way or the other, but the fact that we do the
> same logic for "oops_in_progress" makes me feel like it shouldn't
> happen too often.  Also note that the old logic of turning on
> interrupts temporarily wasn't exactly safe since (I presume) that
> could have violated spin_lock_irqsave() semantics and ended up with a
> deadlock of its own.

This is part of the code to bring all the cores to a halt and since
the other cores are still running kgdb isn't yet able to use the fact
all the CPUs are halted to bend the rules. It is better for this code
to play by the rules if it can.

Is is possible to get the roundup functions to use a private csd
alongside smp_call_function_single_async()? We could add a helper
function to the debug core to avoid having to add cpu_online loops into
every kgdb_roundup_cpus() implementaton.


Daniel.



> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  kernel/smp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 163c451af42e..bb581e58c8dc 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -19,6 +19,7 @@
>  #include <linux/sched.h>
>  #include <linux/sched/idle.h>
>  #include <linux/hypervisor.h>
> +#include <linux/kgdb.h>
>  
>  #include "smpboot.h"
>  
> @@ -413,7 +414,8 @@ void smp_call_function_many(const struct cpumask *mask,
>  	 * can't happen.
>  	 */
>  	WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled()
> -		     && !oops_in_progress && !early_boot_irqs_disabled);
> +		     && !oops_in_progress && !early_boot_irqs_disabled
> +		     && !in_dbg_master());
>  
>  	/* Try to fastpath.  So, what's a CPU they want? Ignoring this one. */
>  	cpu = cpumask_first_and(mask, cpu_online_mask);
> -- 
> 2.19.1.568.g152ad8e336-goog
> 

  parent reply	other threads:[~2018-10-30  9:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-29 18:07 [PATCH 0/7] serial: Finish kgdb on qcom_geni; fix many lockdep splats w/ kgdb Douglas Anderson
2018-10-29 18:07 ` [PATCH 1/7] serial: qcom_geni_serial: Finish supporting sysrq Douglas Anderson
2018-11-02 16:47   ` Stephen Boyd
2018-10-29 18:07 ` [PATCH 2/7] serial: core: Allow processing sysrq at port unlock time Douglas Anderson
2018-10-30  5:31   ` Doug Anderson
2018-10-29 18:07 ` [PATCH 3/7] serial: qcom_geni_serial: Process " Douglas Anderson
2018-10-29 18:07 ` [PATCH 4/7] serial: core: Include console.h from serial_core.h Douglas Anderson
2018-10-29 18:07 ` [PATCH 5/7] serial: 8250: Process sysrq at port unlock time Douglas Anderson
2018-10-29 18:07 ` [PATCH 6/7] smp: Don't yell about IRQs disabled in kgdb_roundup_cpus() Douglas Anderson
2018-10-30  8:25   ` Peter Zijlstra
2018-10-30  9:41   ` Daniel Thompson [this message]
2018-10-30 22:21     ` Doug Anderson
2018-10-29 18:07 ` [PATCH 7/7] kgdb: Remove irq flags and local_irq_enable/disable from roundup Douglas Anderson
2018-10-30 11:46   ` Daniel Thompson
2018-10-30 22:22     ` Doug Anderson
2018-10-30 11:56 ` [PATCH 0/7] serial: Finish kgdb on qcom_geni; fix many lockdep splats w/ kgdb Daniel Thompson
2018-10-30 12:31   ` Russell King - ARM Linux
2018-10-30 12:36 ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181030094159.zt6akmyxwrbzoe2x@holly.lan \
    --to=daniel.thompson@linaro.org \
    --cc=dianders@chromium.org \
    --cc=frederic@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jason.wessel@windriver.com \
    --cc=kgdb-bugreport@lists.sourceforge.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-hexagon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=riel@surriel.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --subject='Re: [PATCH 6/7] smp: Don'\''t yell about IRQs disabled in kgdb_roundup_cpus()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).