* [PATCH v5 0/4] kgdb: Fix kgdb_roundup_cpus() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: linux-arm-kernel This series was originally part of the series ("serial: Finish kgdb on qcom_geni; fix many lockdep splats w/ kgdb") but it made sense to split it up. It's believed that dropping into kgdb should be more robust once these patches are applied. Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: - Removed smp_mb() calls. - Also clear out .debuggerinfo. - Also clear out .debuggerinfo and .task for the master. - Remove clearing out in kdb_stub for offline CPUs; it's now redundant. Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. - Don't round up a CPU that failed rounding up before new for v3. - Don't back trace on a cpu that didn't round up new for v3. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). Douglas Anderson (4): kgdb: Remove irq flags from roundup kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() kgdb: Don't round up a CPU that failed rounding up before kdb: Don't back trace on a cpu that didn't round up arch/arc/kernel/kgdb.c | 10 +---- arch/arm/kernel/kgdb.c | 12 ------ arch/arm64/kernel/kgdb.c | 12 ------ arch/hexagon/kernel/kgdb.c | 32 ---------------- arch/mips/kernel/kgdb.c | 9 +---- arch/powerpc/kernel/kgdb.c | 6 +-- arch/sh/kernel/kgdb.c | 12 ------ arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 +---- include/linux/kgdb.h | 22 +++++++---- kernel/debug/debug_core.c | 65 ++++++++++++++++++++++++++++++++- kernel/debug/debug_core.h | 1 + kernel/debug/kdb/kdb_bt.c | 11 +++++- kernel/debug/kdb/kdb_debugger.c | 7 ---- 14 files changed, 98 insertions(+), 112 deletions(-) -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 0/4] kgdb: Fix kgdb_roundup_cpus() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: Will Deacon, kgdb-bugreport, Peter Zijlstra, Douglas Anderson, linux-mips, linux-sh, Borislav Petkov, linux-kernel, sparclinux, Catalin Marinas, James Hogan, linux-hexagon, x86, Vineet Gupta, Thomas Gleixner, Michal Hocko, Ralf Baechle, linux-snps-arc, Andrew Morton, H. Peter Anvin, Yoshinori Sato, Benjamin Herrenschmidt This series was originally part of the series ("serial: Finish kgdb on qcom_geni; fix many lockdep splats w/ kgdb") but it made sense to split it up. It's believed that dropping into kgdb should be more robust once these patches are applied. Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: - Removed smp_mb() calls. - Also clear out .debuggerinfo. - Also clear out .debuggerinfo and .task for the master. - Remove clearing out in kdb_stub for offline CPUs; it's now redundant. Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. - Don't round up a CPU that failed rounding up before new for v3. - Don't back trace on a cpu that didn't round up new for v3. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). Douglas Anderson (4): kgdb: Remove irq flags from roundup kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() kgdb: Don't round up a CPU that failed rounding up before kdb: Don't back trace on a cpu that didn't round up arch/arc/kernel/kgdb.c | 10 +---- arch/arm/kernel/kgdb.c | 12 ------ arch/arm64/kernel/kgdb.c | 12 ------ arch/hexagon/kernel/kgdb.c | 32 ---------------- arch/mips/kernel/kgdb.c | 9 +---- arch/powerpc/kernel/kgdb.c | 6 +-- arch/sh/kernel/kgdb.c | 12 ------ arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 +---- include/linux/kgdb.h | 22 +++++++---- kernel/debug/debug_core.c | 65 ++++++++++++++++++++++++++++++++- kernel/debug/debug_core.h | 1 + kernel/debug/kdb/kdb_bt.c | 11 +++++- kernel/debug/kdb/kdb_debugger.c | 7 ---- 14 files changed, 98 insertions(+), 112 deletions(-) -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 0/4] kgdb: Fix kgdb_roundup_cpus() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: linux-arm-kernel This series was originally part of the series ("serial: Finish kgdb on qcom_geni; fix many lockdep splats w/ kgdb") but it made sense to split it up. It's believed that dropping into kgdb should be more robust once these patches are applied. Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: - Removed smp_mb() calls. - Also clear out .debuggerinfo. - Also clear out .debuggerinfo and .task for the master. - Remove clearing out in kdb_stub for offline CPUs; it's now redundant. Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. - Don't round up a CPU that failed rounding up before new for v3. - Don't back trace on a cpu that didn't round up new for v3. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). Douglas Anderson (4): kgdb: Remove irq flags from roundup kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() kgdb: Don't round up a CPU that failed rounding up before kdb: Don't back trace on a cpu that didn't round up arch/arc/kernel/kgdb.c | 10 +---- arch/arm/kernel/kgdb.c | 12 ------ arch/arm64/kernel/kgdb.c | 12 ------ arch/hexagon/kernel/kgdb.c | 32 ---------------- arch/mips/kernel/kgdb.c | 9 +---- arch/powerpc/kernel/kgdb.c | 6 +-- arch/sh/kernel/kgdb.c | 12 ------ arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 +---- include/linux/kgdb.h | 22 +++++++---- kernel/debug/debug_core.c | 65 ++++++++++++++++++++++++++++++++- kernel/debug/debug_core.h | 1 + kernel/debug/kdb/kdb_bt.c | 11 +++++- kernel/debug/kdb/kdb_debugger.c | 7 ---- 14 files changed, 98 insertions(+), 112 deletions(-) -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 0/4] kgdb: Fix kgdb_roundup_cpus() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: linux-snps-arc This series was originally part of the series ("serial: Finish kgdb on qcom_geni; fix many lockdep splats w/ kgdb") but it made sense to split it up. It's believed that dropping into kgdb should be more robust once these patches are applied. Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: - Removed smp_mb() calls. - Also clear out .debuggerinfo. - Also clear out .debuggerinfo and .task for the master. - Remove clearing out in kdb_stub for offline CPUs; it's now redundant. Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. - Don't round up a CPU that failed rounding up before new for v3. - Don't back trace on a cpu that didn't round up new for v3. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). Douglas Anderson (4): kgdb: Remove irq flags from roundup kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() kgdb: Don't round up a CPU that failed rounding up before kdb: Don't back trace on a cpu that didn't round up arch/arc/kernel/kgdb.c | 10 +---- arch/arm/kernel/kgdb.c | 12 ------ arch/arm64/kernel/kgdb.c | 12 ------ arch/hexagon/kernel/kgdb.c | 32 ---------------- arch/mips/kernel/kgdb.c | 9 +---- arch/powerpc/kernel/kgdb.c | 6 +-- arch/sh/kernel/kgdb.c | 12 ------ arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 +---- include/linux/kgdb.h | 22 +++++++---- kernel/debug/debug_core.c | 65 ++++++++++++++++++++++++++++++++- kernel/debug/debug_core.h | 1 + kernel/debug/kdb/kdb_bt.c | 11 +++++- kernel/debug/kdb/kdb_debugger.c | 7 ---- 14 files changed, 98 insertions(+), 112 deletions(-) -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 0/4] kgdb: Fix kgdb_roundup_cpus() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: linux-mips, Michal Hocko, Catalin Marinas, linux-sh, Peter Zijlstra, kgdb-bugreport, Will Deacon, linux-kernel, Rich Felker, Paul Mackerras, H. Peter Anvin, sparclinux, Yoshinori Sato, linux-hexagon, x86, Russell King, Ingo Molnar, James Hogan, linux-snps-arc, Huang Ying, Mike Rapoport, Borislav Petkov, Thomas Gleixner, linux-arm-kernel, Vineet Gupta, Douglas Anderson, Ralf Baechle, Richard Kuo, Paul Burton, Andrew Morton, linuxppc-dev, David S. Miller This series was originally part of the series ("serial: Finish kgdb on qcom_geni; fix many lockdep splats w/ kgdb") but it made sense to split it up. It's believed that dropping into kgdb should be more robust once these patches are applied. Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: - Removed smp_mb() calls. - Also clear out .debuggerinfo. - Also clear out .debuggerinfo and .task for the master. - Remove clearing out in kdb_stub for offline CPUs; it's now redundant. Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. - Don't round up a CPU that failed rounding up before new for v3. - Don't back trace on a cpu that didn't round up new for v3. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). Douglas Anderson (4): kgdb: Remove irq flags from roundup kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() kgdb: Don't round up a CPU that failed rounding up before kdb: Don't back trace on a cpu that didn't round up arch/arc/kernel/kgdb.c | 10 +---- arch/arm/kernel/kgdb.c | 12 ------ arch/arm64/kernel/kgdb.c | 12 ------ arch/hexagon/kernel/kgdb.c | 32 ---------------- arch/mips/kernel/kgdb.c | 9 +---- arch/powerpc/kernel/kgdb.c | 6 +-- arch/sh/kernel/kgdb.c | 12 ------ arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 +---- include/linux/kgdb.h | 22 +++++++---- kernel/debug/debug_core.c | 65 ++++++++++++++++++++++++++++++++- kernel/debug/debug_core.h | 1 + kernel/debug/kdb/kdb_bt.c | 11 +++++- kernel/debug/kdb/kdb_debugger.c | 7 ---- 14 files changed, 98 insertions(+), 112 deletions(-) -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 0/4] kgdb: Fix kgdb_roundup_cpus() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: Will Deacon, kgdb-bugreport, Peter Zijlstra, Douglas Anderson, linux-mips, linux-sh, Borislav Petkov, linux-kernel, sparclinux, Catalin Marinas, James Hogan, linux-hexagon, x86, Vineet Gupta, Thomas Gleixner, Michal Hocko, Ralf Baechle, linux-snps-arc, Andrew Morton, H. Peter Anvin, Yoshinori Sato, Benjamin Herrenschmidt, Paul Burton, Ingo Molnar, Russell King, linux-arm-kernel, Christophe Leroy, Huang Ying, David S. Miller, Rich Felker, Michael Ellerman, Mike Rapoport, Paul Mackerras, Richard Kuo, linuxppc-dev This series was originally part of the series ("serial: Finish kgdb on qcom_geni; fix many lockdep splats w/ kgdb") but it made sense to split it up. It's believed that dropping into kgdb should be more robust once these patches are applied. Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: - Removed smp_mb() calls. - Also clear out .debuggerinfo. - Also clear out .debuggerinfo and .task for the master. - Remove clearing out in kdb_stub for offline CPUs; it's now redundant. Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. - Don't round up a CPU that failed rounding up before new for v3. - Don't back trace on a cpu that didn't round up new for v3. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). Douglas Anderson (4): kgdb: Remove irq flags from roundup kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() kgdb: Don't round up a CPU that failed rounding up before kdb: Don't back trace on a cpu that didn't round up arch/arc/kernel/kgdb.c | 10 +---- arch/arm/kernel/kgdb.c | 12 ------ arch/arm64/kernel/kgdb.c | 12 ------ arch/hexagon/kernel/kgdb.c | 32 ---------------- arch/mips/kernel/kgdb.c | 9 +---- arch/powerpc/kernel/kgdb.c | 6 +-- arch/sh/kernel/kgdb.c | 12 ------ arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 +---- include/linux/kgdb.h | 22 +++++++---- kernel/debug/debug_core.c | 65 ++++++++++++++++++++++++++++++++- kernel/debug/debug_core.h | 1 + kernel/debug/kdb/kdb_bt.c | 11 +++++- kernel/debug/kdb/kdb_debugger.c | 7 ---- 14 files changed, 98 insertions(+), 112 deletions(-) -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 1/4] kgdb: Remove irq flags from roundup 2018-11-27 3:51 ` Douglas Anderson ` (3 preceding siblings ...) (?) @ 2018-11-27 3:51 ` Douglas Anderson -1 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: linux-arm-kernel The function kgdb_roundup_cpus() was passed a parameter that was documented as: > the flags that will be used when restoring the interrupts. There is > local_irq_save() call before kgdb_roundup_cpus(). Nobody used those flags. Anyone who wanted to temporarily turn on interrupts just did local_irq_enable() and local_irq_disable() without looking at them. So we can definitely remove the flags. Signed-off-by: Douglas Anderson <dianders@chromium.org> Acked-by: Will Deacon <will.deacon@arm.com> --- Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: - Removing irq flags separated from fixing lockdep splat. arch/arc/kernel/kgdb.c | 2 +- arch/arm/kernel/kgdb.c | 2 +- arch/arm64/kernel/kgdb.c | 2 +- arch/hexagon/kernel/kgdb.c | 9 ++------- arch/mips/kernel/kgdb.c | 2 +- arch/powerpc/kernel/kgdb.c | 2 +- arch/sh/kernel/kgdb.c | 2 +- arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 ++------- include/linux/kgdb.h | 9 ++------- kernel/debug/debug_core.c | 2 +- 11 files changed, 14 insertions(+), 29 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 9a3c34af2ae8..0932851028e0 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -197,7 +197,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index caa0dbe3dc61..f21077b077be 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -175,7 +175,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index a20de58061a8..12c339ff6e75 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -289,7 +289,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 16c24b22d0b2..012e0e230ac2 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -119,17 +119,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ @@ -139,7 +134,7 @@ static void hexagon_kgdb_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index eb6c0d582626..2b05effc17b4 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -219,7 +219,7 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index 59c578f865aa..b0e804844be0 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -124,7 +124,7 @@ static int kgdb_call_nmi_hook(struct pt_regs *regs) } #ifdef CONFIG_SMP -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_send_debugger_break(); } diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index 4f04c6638a4d..cc57630f6bf2 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -319,7 +319,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c index 4792e08ad36b..f45d876983f1 100644 --- a/arch/sparc/kernel/smp_64.c +++ b/arch/sparc/kernel/smp_64.c @@ -1014,7 +1014,7 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page) } #ifdef CONFIG_KGDB -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_cross_call(&xcall_kgdb_capture, 0, 0, 0); } diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c index 8e36f249646e..ac6291a4178d 100644 --- a/arch/x86/kernel/kgdb.c +++ b/arch/x86/kernel/kgdb.c @@ -422,21 +422,16 @@ static void kgdb_disable_hw_debug(struct pt_regs *regs) #ifdef CONFIG_SMP /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { apic->send_IPI_allbutself(APIC_DM_NMI); } diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index e465bb15912d..05e5b2eb0d32 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -178,21 +178,16 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -extern void kgdb_roundup_cpus(unsigned long flags); +extern void kgdb_roundup_cpus(void); /** * kgdb_arch_set_pc - Generic call back to the program counter diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 65c0f1363788..f3cadda45f07 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -593,7 +593,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs, /* Signal the other CPUs to enter kgdb_wait() */ else if ((!kgdb_single_step) && kgdb_do_roundup) - kgdb_roundup_cpus(flags); + kgdb_roundup_cpus(); #endif /* -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 1/4] kgdb: Remove irq flags from roundup @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: Will Deacon, kgdb-bugreport, Peter Zijlstra, Douglas Anderson, linux-mips, linux-sh, Borislav Petkov, linux-kernel, sparclinux, Catalin Marinas, James Hogan, linux-hexagon, x86, Vineet Gupta, Thomas Gleixner, Michal Hocko, Ralf Baechle, linux-snps-arc, Andrew Morton, H. Peter Anvin, Yoshinori Sato, Benjamin Herrenschmidt The function kgdb_roundup_cpus() was passed a parameter that was documented as: > the flags that will be used when restoring the interrupts. There is > local_irq_save() call before kgdb_roundup_cpus(). Nobody used those flags. Anyone who wanted to temporarily turn on interrupts just did local_irq_enable() and local_irq_disable() without looking at them. So we can definitely remove the flags. Signed-off-by: Douglas Anderson <dianders@chromium.org> Acked-by: Will Deacon <will.deacon@arm.com> --- Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: - Removing irq flags separated from fixing lockdep splat. arch/arc/kernel/kgdb.c | 2 +- arch/arm/kernel/kgdb.c | 2 +- arch/arm64/kernel/kgdb.c | 2 +- arch/hexagon/kernel/kgdb.c | 9 ++------- arch/mips/kernel/kgdb.c | 2 +- arch/powerpc/kernel/kgdb.c | 2 +- arch/sh/kernel/kgdb.c | 2 +- arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 ++------- include/linux/kgdb.h | 9 ++------- kernel/debug/debug_core.c | 2 +- 11 files changed, 14 insertions(+), 29 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 9a3c34af2ae8..0932851028e0 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -197,7 +197,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index caa0dbe3dc61..f21077b077be 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -175,7 +175,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index a20de58061a8..12c339ff6e75 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -289,7 +289,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 16c24b22d0b2..012e0e230ac2 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -119,17 +119,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ @@ -139,7 +134,7 @@ static void hexagon_kgdb_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index eb6c0d582626..2b05effc17b4 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -219,7 +219,7 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index 59c578f865aa..b0e804844be0 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -124,7 +124,7 @@ static int kgdb_call_nmi_hook(struct pt_regs *regs) } #ifdef CONFIG_SMP -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_send_debugger_break(); } diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index 4f04c6638a4d..cc57630f6bf2 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -319,7 +319,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c index 4792e08ad36b..f45d876983f1 100644 --- a/arch/sparc/kernel/smp_64.c +++ b/arch/sparc/kernel/smp_64.c @@ -1014,7 +1014,7 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page) } #ifdef CONFIG_KGDB -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_cross_call(&xcall_kgdb_capture, 0, 0, 0); } diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c index 8e36f249646e..ac6291a4178d 100644 --- a/arch/x86/kernel/kgdb.c +++ b/arch/x86/kernel/kgdb.c @@ -422,21 +422,16 @@ static void kgdb_disable_hw_debug(struct pt_regs *regs) #ifdef CONFIG_SMP /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { apic->send_IPI_allbutself(APIC_DM_NMI); } diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index e465bb15912d..05e5b2eb0d32 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -178,21 +178,16 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -extern void kgdb_roundup_cpus(unsigned long flags); +extern void kgdb_roundup_cpus(void); /** * kgdb_arch_set_pc - Generic call back to the program counter diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 65c0f1363788..f3cadda45f07 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -593,7 +593,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs, /* Signal the other CPUs to enter kgdb_wait() */ else if ((!kgdb_single_step) && kgdb_do_roundup) - kgdb_roundup_cpus(flags); + kgdb_roundup_cpus(); #endif /* -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 1/4] kgdb: Remove irq flags from roundup @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: linux-arm-kernel The function kgdb_roundup_cpus() was passed a parameter that was documented as: > the flags that will be used when restoring the interrupts. There is > local_irq_save() call before kgdb_roundup_cpus(). Nobody used those flags. Anyone who wanted to temporarily turn on interrupts just did local_irq_enable() and local_irq_disable() without looking at them. So we can definitely remove the flags. Signed-off-by: Douglas Anderson <dianders@chromium.org> Acked-by: Will Deacon <will.deacon@arm.com> --- Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: - Removing irq flags separated from fixing lockdep splat. arch/arc/kernel/kgdb.c | 2 +- arch/arm/kernel/kgdb.c | 2 +- arch/arm64/kernel/kgdb.c | 2 +- arch/hexagon/kernel/kgdb.c | 9 ++------- arch/mips/kernel/kgdb.c | 2 +- arch/powerpc/kernel/kgdb.c | 2 +- arch/sh/kernel/kgdb.c | 2 +- arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 ++------- include/linux/kgdb.h | 9 ++------- kernel/debug/debug_core.c | 2 +- 11 files changed, 14 insertions(+), 29 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 9a3c34af2ae8..0932851028e0 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -197,7 +197,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index caa0dbe3dc61..f21077b077be 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -175,7 +175,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index a20de58061a8..12c339ff6e75 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -289,7 +289,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 16c24b22d0b2..012e0e230ac2 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -119,17 +119,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ @@ -139,7 +134,7 @@ static void hexagon_kgdb_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index eb6c0d582626..2b05effc17b4 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -219,7 +219,7 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index 59c578f865aa..b0e804844be0 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -124,7 +124,7 @@ static int kgdb_call_nmi_hook(struct pt_regs *regs) } #ifdef CONFIG_SMP -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_send_debugger_break(); } diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index 4f04c6638a4d..cc57630f6bf2 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -319,7 +319,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c index 4792e08ad36b..f45d876983f1 100644 --- a/arch/sparc/kernel/smp_64.c +++ b/arch/sparc/kernel/smp_64.c @@ -1014,7 +1014,7 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page) } #ifdef CONFIG_KGDB -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_cross_call(&xcall_kgdb_capture, 0, 0, 0); } diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c index 8e36f249646e..ac6291a4178d 100644 --- a/arch/x86/kernel/kgdb.c +++ b/arch/x86/kernel/kgdb.c @@ -422,21 +422,16 @@ static void kgdb_disable_hw_debug(struct pt_regs *regs) #ifdef CONFIG_SMP /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { apic->send_IPI_allbutself(APIC_DM_NMI); } diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index e465bb15912d..05e5b2eb0d32 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -178,21 +178,16 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -extern void kgdb_roundup_cpus(unsigned long flags); +extern void kgdb_roundup_cpus(void); /** * kgdb_arch_set_pc - Generic call back to the program counter diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 65c0f1363788..f3cadda45f07 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -593,7 +593,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs, /* Signal the other CPUs to enter kgdb_wait() */ else if ((!kgdb_single_step) && kgdb_do_roundup) - kgdb_roundup_cpus(flags); + kgdb_roundup_cpus(); #endif /* -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 1/4] kgdb: Remove irq flags from roundup @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: linux-snps-arc The function kgdb_roundup_cpus() was passed a parameter that was documented as: > the flags that will be used when restoring the interrupts. There is > local_irq_save() call before kgdb_roundup_cpus(). Nobody used those flags. Anyone who wanted to temporarily turn on interrupts just did local_irq_enable() and local_irq_disable() without looking at them. So we can definitely remove the flags. Signed-off-by: Douglas Anderson <dianders at chromium.org> Acked-by: Will Deacon <will.deacon at arm.com> --- Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: - Removing irq flags separated from fixing lockdep splat. arch/arc/kernel/kgdb.c | 2 +- arch/arm/kernel/kgdb.c | 2 +- arch/arm64/kernel/kgdb.c | 2 +- arch/hexagon/kernel/kgdb.c | 9 ++------- arch/mips/kernel/kgdb.c | 2 +- arch/powerpc/kernel/kgdb.c | 2 +- arch/sh/kernel/kgdb.c | 2 +- arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 ++------- include/linux/kgdb.h | 9 ++------- kernel/debug/debug_core.c | 2 +- 11 files changed, 14 insertions(+), 29 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 9a3c34af2ae8..0932851028e0 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -197,7 +197,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index caa0dbe3dc61..f21077b077be 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -175,7 +175,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index a20de58061a8..12c339ff6e75 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -289,7 +289,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 16c24b22d0b2..012e0e230ac2 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -119,17 +119,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ @@ -139,7 +134,7 @@ static void hexagon_kgdb_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index eb6c0d582626..2b05effc17b4 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -219,7 +219,7 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index 59c578f865aa..b0e804844be0 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -124,7 +124,7 @@ static int kgdb_call_nmi_hook(struct pt_regs *regs) } #ifdef CONFIG_SMP -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_send_debugger_break(); } diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index 4f04c6638a4d..cc57630f6bf2 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -319,7 +319,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c index 4792e08ad36b..f45d876983f1 100644 --- a/arch/sparc/kernel/smp_64.c +++ b/arch/sparc/kernel/smp_64.c @@ -1014,7 +1014,7 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page) } #ifdef CONFIG_KGDB -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_cross_call(&xcall_kgdb_capture, 0, 0, 0); } diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c index 8e36f249646e..ac6291a4178d 100644 --- a/arch/x86/kernel/kgdb.c +++ b/arch/x86/kernel/kgdb.c @@ -422,21 +422,16 @@ static void kgdb_disable_hw_debug(struct pt_regs *regs) #ifdef CONFIG_SMP /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { apic->send_IPI_allbutself(APIC_DM_NMI); } diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index e465bb15912d..05e5b2eb0d32 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -178,21 +178,16 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -extern void kgdb_roundup_cpus(unsigned long flags); +extern void kgdb_roundup_cpus(void); /** * kgdb_arch_set_pc - Generic call back to the program counter diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 65c0f1363788..f3cadda45f07 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -593,7 +593,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs, /* Signal the other CPUs to enter kgdb_wait() */ else if ((!kgdb_single_step) && kgdb_do_roundup) - kgdb_roundup_cpus(flags); + kgdb_roundup_cpus(); #endif /* -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 1/4] kgdb: Remove irq flags from roundup @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: linux-mips, Michal Hocko, Catalin Marinas, linux-sh, Peter Zijlstra, kgdb-bugreport, Will Deacon, linux-kernel, Rich Felker, Paul Mackerras, H. Peter Anvin, sparclinux, Yoshinori Sato, linux-hexagon, x86, Russell King, Ingo Molnar, James Hogan, linux-snps-arc, Huang Ying, Mike Rapoport, Borislav Petkov, Thomas Gleixner, linux-arm-kernel, Vineet Gupta, Douglas Anderson, Ralf Baechle, Richard Kuo, Paul Burton, Andrew Morton, linuxppc-dev, David S. Miller The function kgdb_roundup_cpus() was passed a parameter that was documented as: > the flags that will be used when restoring the interrupts. There is > local_irq_save() call before kgdb_roundup_cpus(). Nobody used those flags. Anyone who wanted to temporarily turn on interrupts just did local_irq_enable() and local_irq_disable() without looking at them. So we can definitely remove the flags. Signed-off-by: Douglas Anderson <dianders@chromium.org> Acked-by: Will Deacon <will.deacon@arm.com> --- Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: - Removing irq flags separated from fixing lockdep splat. arch/arc/kernel/kgdb.c | 2 +- arch/arm/kernel/kgdb.c | 2 +- arch/arm64/kernel/kgdb.c | 2 +- arch/hexagon/kernel/kgdb.c | 9 ++------- arch/mips/kernel/kgdb.c | 2 +- arch/powerpc/kernel/kgdb.c | 2 +- arch/sh/kernel/kgdb.c | 2 +- arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 ++------- include/linux/kgdb.h | 9 ++------- kernel/debug/debug_core.c | 2 +- 11 files changed, 14 insertions(+), 29 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 9a3c34af2ae8..0932851028e0 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -197,7 +197,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index caa0dbe3dc61..f21077b077be 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -175,7 +175,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index a20de58061a8..12c339ff6e75 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -289,7 +289,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 16c24b22d0b2..012e0e230ac2 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -119,17 +119,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ @@ -139,7 +134,7 @@ static void hexagon_kgdb_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index eb6c0d582626..2b05effc17b4 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -219,7 +219,7 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index 59c578f865aa..b0e804844be0 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -124,7 +124,7 @@ static int kgdb_call_nmi_hook(struct pt_regs *regs) } #ifdef CONFIG_SMP -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_send_debugger_break(); } diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index 4f04c6638a4d..cc57630f6bf2 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -319,7 +319,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c index 4792e08ad36b..f45d876983f1 100644 --- a/arch/sparc/kernel/smp_64.c +++ b/arch/sparc/kernel/smp_64.c @@ -1014,7 +1014,7 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page) } #ifdef CONFIG_KGDB -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_cross_call(&xcall_kgdb_capture, 0, 0, 0); } diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c index 8e36f249646e..ac6291a4178d 100644 --- a/arch/x86/kernel/kgdb.c +++ b/arch/x86/kernel/kgdb.c @@ -422,21 +422,16 @@ static void kgdb_disable_hw_debug(struct pt_regs *regs) #ifdef CONFIG_SMP /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { apic->send_IPI_allbutself(APIC_DM_NMI); } diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index e465bb15912d..05e5b2eb0d32 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -178,21 +178,16 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -extern void kgdb_roundup_cpus(unsigned long flags); +extern void kgdb_roundup_cpus(void); /** * kgdb_arch_set_pc - Generic call back to the program counter diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 65c0f1363788..f3cadda45f07 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -593,7 +593,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs, /* Signal the other CPUs to enter kgdb_wait() */ else if ((!kgdb_single_step) && kgdb_do_roundup) - kgdb_roundup_cpus(flags); + kgdb_roundup_cpus(); #endif /* -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 1/4] kgdb: Remove irq flags from roundup @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: Will Deacon, kgdb-bugreport, Peter Zijlstra, Douglas Anderson, linux-mips, linux-sh, Borislav Petkov, linux-kernel, sparclinux, Catalin Marinas, James Hogan, linux-hexagon, x86, Vineet Gupta, Thomas Gleixner, Michal Hocko, Ralf Baechle, linux-snps-arc, Andrew Morton, H. Peter Anvin, Yoshinori Sato, Benjamin Herrenschmidt, Paul Burton, Ingo Molnar, Russell King, linux-arm-kernel, Christophe Leroy, Huang Ying, David S. Miller, Rich Felker, Michael Ellerman, Mike Rapoport, Paul Mackerras, Richard Kuo, linuxppc-dev The function kgdb_roundup_cpus() was passed a parameter that was documented as: > the flags that will be used when restoring the interrupts. There is > local_irq_save() call before kgdb_roundup_cpus(). Nobody used those flags. Anyone who wanted to temporarily turn on interrupts just did local_irq_enable() and local_irq_disable() without looking at them. So we can definitely remove the flags. Signed-off-by: Douglas Anderson <dianders@chromium.org> Acked-by: Will Deacon <will.deacon@arm.com> --- Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: - Removing irq flags separated from fixing lockdep splat. arch/arc/kernel/kgdb.c | 2 +- arch/arm/kernel/kgdb.c | 2 +- arch/arm64/kernel/kgdb.c | 2 +- arch/hexagon/kernel/kgdb.c | 9 ++------- arch/mips/kernel/kgdb.c | 2 +- arch/powerpc/kernel/kgdb.c | 2 +- arch/sh/kernel/kgdb.c | 2 +- arch/sparc/kernel/smp_64.c | 2 +- arch/x86/kernel/kgdb.c | 9 ++------- include/linux/kgdb.h | 9 ++------- kernel/debug/debug_core.c | 2 +- 11 files changed, 14 insertions(+), 29 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 9a3c34af2ae8..0932851028e0 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -197,7 +197,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index caa0dbe3dc61..f21077b077be 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -175,7 +175,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index a20de58061a8..12c339ff6e75 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -289,7 +289,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 16c24b22d0b2..012e0e230ac2 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -119,17 +119,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ @@ -139,7 +134,7 @@ static void hexagon_kgdb_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index eb6c0d582626..2b05effc17b4 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -219,7 +219,7 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index 59c578f865aa..b0e804844be0 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -124,7 +124,7 @@ static int kgdb_call_nmi_hook(struct pt_regs *regs) } #ifdef CONFIG_SMP -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_send_debugger_break(); } diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index 4f04c6638a4d..cc57630f6bf2 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -319,7 +319,7 @@ static void kgdb_call_nmi_hook(void *ignored) kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); } -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { local_irq_enable(); smp_call_function(kgdb_call_nmi_hook, NULL, 0); diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c index 4792e08ad36b..f45d876983f1 100644 --- a/arch/sparc/kernel/smp_64.c +++ b/arch/sparc/kernel/smp_64.c @@ -1014,7 +1014,7 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page) } #ifdef CONFIG_KGDB -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { smp_cross_call(&xcall_kgdb_capture, 0, 0, 0); } diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c index 8e36f249646e..ac6291a4178d 100644 --- a/arch/x86/kernel/kgdb.c +++ b/arch/x86/kernel/kgdb.c @@ -422,21 +422,16 @@ static void kgdb_disable_hw_debug(struct pt_regs *regs) #ifdef CONFIG_SMP /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them be in a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -void kgdb_roundup_cpus(unsigned long flags) +void kgdb_roundup_cpus(void) { apic->send_IPI_allbutself(APIC_DM_NMI); } diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index e465bb15912d..05e5b2eb0d32 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -178,21 +178,16 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * @flags: Current IRQ state * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. In - * this case, we have to make sure that interrupts are enabled before - * calling smp_call_function(). The argument to this function is - * the flags that will be used when restoring the interrupts. There is - * local_irq_save() call before kgdb_roundup_cpus(). + * in case of MIPS, smp_call_function() is used to roundup CPUs. * * On non-SMP systems, this is not called. */ -extern void kgdb_roundup_cpus(unsigned long flags); +extern void kgdb_roundup_cpus(void); /** * kgdb_arch_set_pc - Generic call back to the program counter diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 65c0f1363788..f3cadda45f07 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -593,7 +593,7 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs, /* Signal the other CPUs to enter kgdb_wait() */ else if ((!kgdb_single_step) && kgdb_do_roundup) - kgdb_roundup_cpus(flags); + kgdb_roundup_cpus(); #endif /* -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() 2018-11-27 3:51 ` Douglas Anderson ` (3 preceding siblings ...) (?) @ 2018-11-27 3:51 ` Douglas Anderson -1 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: linux-arm-kernel When I had lockdep turned on and dropped into kgdb I got a nice splat on my system. Specifically it hit: DEBUG_LOCKS_WARN_ON(current->hardirq_context) Specifically it looked like this: 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 ]--- Looking closely at it, it seems like a really bad idea to be calling local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems like it could violate spinlock semantics and cause a deadlock. Instead, let's use a private csd alongside smp_call_function_single_async() to round up the other CPUs. Using smp_call_function_single_async() doesn't require interrupts to be enabled so we can remove the offending bit of code. In order to avoid duplicating this across all the architectures that use the default kgdb_roundup_cpus(), we'll add a "weak" implementation to debug_core.c. Looking at all the people who previously had copies of this code, there were a few variants. I've attempted to keep the variants working like they used to. Specifically: * For arch/arc we passed NULL to kgdb_nmicallback() instead of get_irq_regs(). * For arch/mips there was a bit of extra code around kgdb_nmicallback() NOTE: In this patch we will still get into trouble if we try to round up a CPU that failed to round up before. We'll try to round it up again and potentially hang when we try to grab the csd lock. That's not new behavior but we'll still try to do better in a future patch. Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: None Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). arch/arc/kernel/kgdb.c | 10 ++------- arch/arm/kernel/kgdb.c | 12 ----------- arch/arm64/kernel/kgdb.c | 12 ----------- arch/hexagon/kernel/kgdb.c | 27 ----------------------- arch/mips/kernel/kgdb.c | 9 +------- arch/powerpc/kernel/kgdb.c | 4 ++-- arch/sh/kernel/kgdb.c | 12 ----------- include/linux/kgdb.h | 15 +++++++++++-- kernel/debug/debug_core.c | 44 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 62 insertions(+), 83 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 0932851028e0..68d9fe4b5aa7 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -192,18 +192,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long ip) instruction_pointer(regs) = ip; } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { + /* Default implementation passes get_irq_regs() but we don't */ kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - struct kgdb_arch arch_kgdb_ops = { /* breakpoint instruction: TRAP_S 0x3 */ #ifdef CONFIG_CPU_BIG_ENDIAN diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index f21077b077be..d9a69e941463 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -170,18 +170,6 @@ static struct undef_hook kgdb_compiled_brkpt_hook = { .fn = kgdb_compiled_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index 12c339ff6e75..da880247c734 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { .fn = kgdb_step_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 012e0e230ac2..b95d12038a4e 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -115,33 +115,6 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) instruction_pointer(regs) = pc; } -#ifdef CONFIG_SMP - -/** - * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * - * On SMP systems, we need to get the attention of the other CPUs - * and get them be in a known state. This should do what is needed - * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. - * - * On non-SMP systems, this is not called. - */ - -static void hexagon_kgdb_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); - local_irq_disable(); -} -#endif - /* Not yet working */ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index 2b05effc17b4..42f057a6c215 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -207,7 +207,7 @@ void arch_kgdb_breakpoint(void) ".set\treorder"); } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { mm_segment_t old_fs; @@ -219,13 +219,6 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int compute_signal(int tt) { struct hard_trap_info *ht; diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index b0e804844be0..b4ce54d73337 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -117,7 +117,7 @@ int kgdb_skipexception(int exception, struct pt_regs *regs) return kgdb_isremovedbreak(regs->nip); } -static int kgdb_call_nmi_hook(struct pt_regs *regs) +static int kgdb_debugger_ipi(struct pt_regs *regs) { kgdb_nmicallback(raw_smp_processor_id(), regs); return 0; @@ -502,7 +502,7 @@ int kgdb_arch_init(void) old__debugger_break_match = __debugger_break_match; old__debugger_fault_handler = __debugger_fault_handler; - __debugger_ipi = kgdb_call_nmi_hook; + __debugger_ipi = kgdb_debugger_ipi; __debugger = kgdb_debugger; __debugger_bpt = kgdb_handle_breakpoint; __debugger_sstep = kgdb_singlestep; diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index cc57630f6bf2..14e012ad7c57 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -314,18 +314,6 @@ BUILD_TRAP_HANDLER(singlestep) local_irq_restore(flags); } -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { int ret; diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 05e5b2eb0d32..24422865cd18 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -176,14 +176,25 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, char *remcom_out_buffer, struct pt_regs *regs); +/** + * kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU + * @ignored: This parameter is only here to match the prototype. + * + * If you're using the default implementation of kgdb_roundup_cpus() + * this function will be called per CPU. If you don't implement + * kgdb_call_nmi_hook() a default will be used. + */ + +extern void kgdb_call_nmi_hook(void *ignored); + /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. + * the NMI approach is not used for rounding up all the CPUs. Normally + * those architectures can just not implement this and get the default. * * On non-SMP systems, this is not called. */ diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index f3cadda45f07..d7444e6ead99 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -55,6 +55,7 @@ #include <linux/mm.h> #include <linux/vmacache.h> #include <linux/rcupdate.h> +#include <linux/irq.h> #include <asm/cacheflush.h> #include <asm/byteorder.h> @@ -220,6 +221,49 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs) return 0; } +#ifdef CONFIG_SMP + +/* + * Default (weak) implementation for kgdb_roundup_cpus + */ + +static DEFINE_PER_CPU(call_single_data_t, kgdb_roundup_csd); + +void __weak kgdb_call_nmi_hook(void *ignored) +{ + /* + * NOTE: get_irq_regs() is supposed to get the registers from + * before the IPI interrupt happened and so is supposed to + * show where the processor was. In some situations it's + * possible we might be called without an IPI, so it might be + * safer to figure out how to make kgdb_breakpoint() work + * properly here. + */ + kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); +} + +void __weak kgdb_roundup_cpus(void) +{ + call_single_data_t *csd; + int this_cpu = raw_smp_processor_id(); + int cpu; + int ret; + + for_each_online_cpu(cpu) { + /* No need to roundup ourselves */ + if (cpu = this_cpu) + continue; + + csd = &per_cpu(kgdb_roundup_csd, cpu); + csd->func = kgdb_call_nmi_hook; + ret = smp_call_function_single_async(cpu, csd); + if (ret) + kgdb_info[cpu].rounding_up = false; + } +} + +#endif + /* * Some architectures need cache flushes when we set/clear a * breakpoint: -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: Will Deacon, kgdb-bugreport, Peter Zijlstra, Douglas Anderson, linux-mips, linux-sh, linux-kernel, Catalin Marinas, James Hogan, linux-hexagon, Vineet Gupta, Rich Felker, Ralf Baechle, linux-snps-arc, Yoshinori Sato, Benjamin Herrenschmidt, Paul Mackerras, Russell King, linux-arm-kernel, Christophe Leroy, Michael Ellerman When I had lockdep turned on and dropped into kgdb I got a nice splat on my system. Specifically it hit: DEBUG_LOCKS_WARN_ON(current->hardirq_context) Specifically it looked like this: 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 ]--- Looking closely at it, it seems like a really bad idea to be calling local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems like it could violate spinlock semantics and cause a deadlock. Instead, let's use a private csd alongside smp_call_function_single_async() to round up the other CPUs. Using smp_call_function_single_async() doesn't require interrupts to be enabled so we can remove the offending bit of code. In order to avoid duplicating this across all the architectures that use the default kgdb_roundup_cpus(), we'll add a "weak" implementation to debug_core.c. Looking at all the people who previously had copies of this code, there were a few variants. I've attempted to keep the variants working like they used to. Specifically: * For arch/arc we passed NULL to kgdb_nmicallback() instead of get_irq_regs(). * For arch/mips there was a bit of extra code around kgdb_nmicallback() NOTE: In this patch we will still get into trouble if we try to round up a CPU that failed to round up before. We'll try to round it up again and potentially hang when we try to grab the csd lock. That's not new behavior but we'll still try to do better in a future patch. Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: None Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). arch/arc/kernel/kgdb.c | 10 ++------- arch/arm/kernel/kgdb.c | 12 ----------- arch/arm64/kernel/kgdb.c | 12 ----------- arch/hexagon/kernel/kgdb.c | 27 ----------------------- arch/mips/kernel/kgdb.c | 9 +------- arch/powerpc/kernel/kgdb.c | 4 ++-- arch/sh/kernel/kgdb.c | 12 ----------- include/linux/kgdb.h | 15 +++++++++++-- kernel/debug/debug_core.c | 44 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 62 insertions(+), 83 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 0932851028e0..68d9fe4b5aa7 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -192,18 +192,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long ip) instruction_pointer(regs) = ip; } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { + /* Default implementation passes get_irq_regs() but we don't */ kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - struct kgdb_arch arch_kgdb_ops = { /* breakpoint instruction: TRAP_S 0x3 */ #ifdef CONFIG_CPU_BIG_ENDIAN diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index f21077b077be..d9a69e941463 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -170,18 +170,6 @@ static struct undef_hook kgdb_compiled_brkpt_hook = { .fn = kgdb_compiled_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index 12c339ff6e75..da880247c734 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { .fn = kgdb_step_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 012e0e230ac2..b95d12038a4e 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -115,33 +115,6 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) instruction_pointer(regs) = pc; } -#ifdef CONFIG_SMP - -/** - * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * - * On SMP systems, we need to get the attention of the other CPUs - * and get them be in a known state. This should do what is needed - * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. - * - * On non-SMP systems, this is not called. - */ - -static void hexagon_kgdb_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); - local_irq_disable(); -} -#endif - /* Not yet working */ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index 2b05effc17b4..42f057a6c215 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -207,7 +207,7 @@ void arch_kgdb_breakpoint(void) ".set\treorder"); } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { mm_segment_t old_fs; @@ -219,13 +219,6 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int compute_signal(int tt) { struct hard_trap_info *ht; diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index b0e804844be0..b4ce54d73337 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -117,7 +117,7 @@ int kgdb_skipexception(int exception, struct pt_regs *regs) return kgdb_isremovedbreak(regs->nip); } -static int kgdb_call_nmi_hook(struct pt_regs *regs) +static int kgdb_debugger_ipi(struct pt_regs *regs) { kgdb_nmicallback(raw_smp_processor_id(), regs); return 0; @@ -502,7 +502,7 @@ int kgdb_arch_init(void) old__debugger_break_match = __debugger_break_match; old__debugger_fault_handler = __debugger_fault_handler; - __debugger_ipi = kgdb_call_nmi_hook; + __debugger_ipi = kgdb_debugger_ipi; __debugger = kgdb_debugger; __debugger_bpt = kgdb_handle_breakpoint; __debugger_sstep = kgdb_singlestep; diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index cc57630f6bf2..14e012ad7c57 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -314,18 +314,6 @@ BUILD_TRAP_HANDLER(singlestep) local_irq_restore(flags); } -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { int ret; diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 05e5b2eb0d32..24422865cd18 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -176,14 +176,25 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, char *remcom_out_buffer, struct pt_regs *regs); +/** + * kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU + * @ignored: This parameter is only here to match the prototype. + * + * If you're using the default implementation of kgdb_roundup_cpus() + * this function will be called per CPU. If you don't implement + * kgdb_call_nmi_hook() a default will be used. + */ + +extern void kgdb_call_nmi_hook(void *ignored); + /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. + * the NMI approach is not used for rounding up all the CPUs. Normally + * those architectures can just not implement this and get the default. * * On non-SMP systems, this is not called. */ diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index f3cadda45f07..d7444e6ead99 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -55,6 +55,7 @@ #include <linux/mm.h> #include <linux/vmacache.h> #include <linux/rcupdate.h> +#include <linux/irq.h> #include <asm/cacheflush.h> #include <asm/byteorder.h> @@ -220,6 +221,49 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs) return 0; } +#ifdef CONFIG_SMP + +/* + * Default (weak) implementation for kgdb_roundup_cpus + */ + +static DEFINE_PER_CPU(call_single_data_t, kgdb_roundup_csd); + +void __weak kgdb_call_nmi_hook(void *ignored) +{ + /* + * NOTE: get_irq_regs() is supposed to get the registers from + * before the IPI interrupt happened and so is supposed to + * show where the processor was. In some situations it's + * possible we might be called without an IPI, so it might be + * safer to figure out how to make kgdb_breakpoint() work + * properly here. + */ + kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); +} + +void __weak kgdb_roundup_cpus(void) +{ + call_single_data_t *csd; + int this_cpu = raw_smp_processor_id(); + int cpu; + int ret; + + for_each_online_cpu(cpu) { + /* No need to roundup ourselves */ + if (cpu == this_cpu) + continue; + + csd = &per_cpu(kgdb_roundup_csd, cpu); + csd->func = kgdb_call_nmi_hook; + ret = smp_call_function_single_async(cpu, csd); + if (ret) + kgdb_info[cpu].rounding_up = false; + } +} + +#endif + /* * Some architectures need cache flushes when we set/clear a * breakpoint: -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: linux-arm-kernel When I had lockdep turned on and dropped into kgdb I got a nice splat on my system. Specifically it hit: DEBUG_LOCKS_WARN_ON(current->hardirq_context) Specifically it looked like this: 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 ]--- Looking closely at it, it seems like a really bad idea to be calling local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems like it could violate spinlock semantics and cause a deadlock. Instead, let's use a private csd alongside smp_call_function_single_async() to round up the other CPUs. Using smp_call_function_single_async() doesn't require interrupts to be enabled so we can remove the offending bit of code. In order to avoid duplicating this across all the architectures that use the default kgdb_roundup_cpus(), we'll add a "weak" implementation to debug_core.c. Looking at all the people who previously had copies of this code, there were a few variants. I've attempted to keep the variants working like they used to. Specifically: * For arch/arc we passed NULL to kgdb_nmicallback() instead of get_irq_regs(). * For arch/mips there was a bit of extra code around kgdb_nmicallback() NOTE: In this patch we will still get into trouble if we try to round up a CPU that failed to round up before. We'll try to round it up again and potentially hang when we try to grab the csd lock. That's not new behavior but we'll still try to do better in a future patch. Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: None Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). arch/arc/kernel/kgdb.c | 10 ++------- arch/arm/kernel/kgdb.c | 12 ----------- arch/arm64/kernel/kgdb.c | 12 ----------- arch/hexagon/kernel/kgdb.c | 27 ----------------------- arch/mips/kernel/kgdb.c | 9 +------- arch/powerpc/kernel/kgdb.c | 4 ++-- arch/sh/kernel/kgdb.c | 12 ----------- include/linux/kgdb.h | 15 +++++++++++-- kernel/debug/debug_core.c | 44 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 62 insertions(+), 83 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 0932851028e0..68d9fe4b5aa7 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -192,18 +192,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long ip) instruction_pointer(regs) = ip; } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { + /* Default implementation passes get_irq_regs() but we don't */ kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - struct kgdb_arch arch_kgdb_ops = { /* breakpoint instruction: TRAP_S 0x3 */ #ifdef CONFIG_CPU_BIG_ENDIAN diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index f21077b077be..d9a69e941463 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -170,18 +170,6 @@ static struct undef_hook kgdb_compiled_brkpt_hook = { .fn = kgdb_compiled_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index 12c339ff6e75..da880247c734 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { .fn = kgdb_step_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 012e0e230ac2..b95d12038a4e 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -115,33 +115,6 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) instruction_pointer(regs) = pc; } -#ifdef CONFIG_SMP - -/** - * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * - * On SMP systems, we need to get the attention of the other CPUs - * and get them be in a known state. This should do what is needed - * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. - * - * On non-SMP systems, this is not called. - */ - -static void hexagon_kgdb_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); - local_irq_disable(); -} -#endif - /* Not yet working */ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index 2b05effc17b4..42f057a6c215 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -207,7 +207,7 @@ void arch_kgdb_breakpoint(void) ".set\treorder"); } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { mm_segment_t old_fs; @@ -219,13 +219,6 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int compute_signal(int tt) { struct hard_trap_info *ht; diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index b0e804844be0..b4ce54d73337 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -117,7 +117,7 @@ int kgdb_skipexception(int exception, struct pt_regs *regs) return kgdb_isremovedbreak(regs->nip); } -static int kgdb_call_nmi_hook(struct pt_regs *regs) +static int kgdb_debugger_ipi(struct pt_regs *regs) { kgdb_nmicallback(raw_smp_processor_id(), regs); return 0; @@ -502,7 +502,7 @@ int kgdb_arch_init(void) old__debugger_break_match = __debugger_break_match; old__debugger_fault_handler = __debugger_fault_handler; - __debugger_ipi = kgdb_call_nmi_hook; + __debugger_ipi = kgdb_debugger_ipi; __debugger = kgdb_debugger; __debugger_bpt = kgdb_handle_breakpoint; __debugger_sstep = kgdb_singlestep; diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index cc57630f6bf2..14e012ad7c57 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -314,18 +314,6 @@ BUILD_TRAP_HANDLER(singlestep) local_irq_restore(flags); } -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { int ret; diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 05e5b2eb0d32..24422865cd18 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -176,14 +176,25 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, char *remcom_out_buffer, struct pt_regs *regs); +/** + * kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU + * @ignored: This parameter is only here to match the prototype. + * + * If you're using the default implementation of kgdb_roundup_cpus() + * this function will be called per CPU. If you don't implement + * kgdb_call_nmi_hook() a default will be used. + */ + +extern void kgdb_call_nmi_hook(void *ignored); + /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. + * the NMI approach is not used for rounding up all the CPUs. Normally + * those architectures can just not implement this and get the default. * * On non-SMP systems, this is not called. */ diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index f3cadda45f07..d7444e6ead99 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -55,6 +55,7 @@ #include <linux/mm.h> #include <linux/vmacache.h> #include <linux/rcupdate.h> +#include <linux/irq.h> #include <asm/cacheflush.h> #include <asm/byteorder.h> @@ -220,6 +221,49 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs) return 0; } +#ifdef CONFIG_SMP + +/* + * Default (weak) implementation for kgdb_roundup_cpus + */ + +static DEFINE_PER_CPU(call_single_data_t, kgdb_roundup_csd); + +void __weak kgdb_call_nmi_hook(void *ignored) +{ + /* + * NOTE: get_irq_regs() is supposed to get the registers from + * before the IPI interrupt happened and so is supposed to + * show where the processor was. In some situations it's + * possible we might be called without an IPI, so it might be + * safer to figure out how to make kgdb_breakpoint() work + * properly here. + */ + kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); +} + +void __weak kgdb_roundup_cpus(void) +{ + call_single_data_t *csd; + int this_cpu = raw_smp_processor_id(); + int cpu; + int ret; + + for_each_online_cpu(cpu) { + /* No need to roundup ourselves */ + if (cpu == this_cpu) + continue; + + csd = &per_cpu(kgdb_roundup_csd, cpu); + csd->func = kgdb_call_nmi_hook; + ret = smp_call_function_single_async(cpu, csd); + if (ret) + kgdb_info[cpu].rounding_up = false; + } +} + +#endif + /* * Some architectures need cache flushes when we set/clear a * breakpoint: -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: linux-snps-arc When I had lockdep turned on and dropped into kgdb I got a nice splat on my system. Specifically it hit: DEBUG_LOCKS_WARN_ON(current->hardirq_context) Specifically it looked like this: 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 ]--- Looking closely at it, it seems like a really bad idea to be calling local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems like it could violate spinlock semantics and cause a deadlock. Instead, let's use a private csd alongside smp_call_function_single_async() to round up the other CPUs. Using smp_call_function_single_async() doesn't require interrupts to be enabled so we can remove the offending bit of code. In order to avoid duplicating this across all the architectures that use the default kgdb_roundup_cpus(), we'll add a "weak" implementation to debug_core.c. Looking at all the people who previously had copies of this code, there were a few variants. I've attempted to keep the variants working like they used to. Specifically: * For arch/arc we passed NULL to kgdb_nmicallback() instead of get_irq_regs(). * For arch/mips there was a bit of extra code around kgdb_nmicallback() NOTE: In this patch we will still get into trouble if we try to round up a CPU that failed to round up before. We'll try to round it up again and potentially hang when we try to grab the csd lock. That's not new behavior but we'll still try to do better in a future patch. Suggested-by: Daniel Thompson <daniel.thompson at linaro.org> Signed-off-by: Douglas Anderson <dianders at chromium.org> --- Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: None Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). arch/arc/kernel/kgdb.c | 10 ++------- arch/arm/kernel/kgdb.c | 12 ----------- arch/arm64/kernel/kgdb.c | 12 ----------- arch/hexagon/kernel/kgdb.c | 27 ----------------------- arch/mips/kernel/kgdb.c | 9 +------- arch/powerpc/kernel/kgdb.c | 4 ++-- arch/sh/kernel/kgdb.c | 12 ----------- include/linux/kgdb.h | 15 +++++++++++-- kernel/debug/debug_core.c | 44 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 62 insertions(+), 83 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 0932851028e0..68d9fe4b5aa7 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -192,18 +192,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long ip) instruction_pointer(regs) = ip; } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { + /* Default implementation passes get_irq_regs() but we don't */ kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - struct kgdb_arch arch_kgdb_ops = { /* breakpoint instruction: TRAP_S 0x3 */ #ifdef CONFIG_CPU_BIG_ENDIAN diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index f21077b077be..d9a69e941463 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -170,18 +170,6 @@ static struct undef_hook kgdb_compiled_brkpt_hook = { .fn = kgdb_compiled_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index 12c339ff6e75..da880247c734 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { .fn = kgdb_step_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 012e0e230ac2..b95d12038a4e 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -115,33 +115,6 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) instruction_pointer(regs) = pc; } -#ifdef CONFIG_SMP - -/** - * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * - * On SMP systems, we need to get the attention of the other CPUs - * and get them be in a known state. This should do what is needed - * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. - * - * On non-SMP systems, this is not called. - */ - -static void hexagon_kgdb_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); - local_irq_disable(); -} -#endif - /* Not yet working */ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index 2b05effc17b4..42f057a6c215 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -207,7 +207,7 @@ void arch_kgdb_breakpoint(void) ".set\treorder"); } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { mm_segment_t old_fs; @@ -219,13 +219,6 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int compute_signal(int tt) { struct hard_trap_info *ht; diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index b0e804844be0..b4ce54d73337 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -117,7 +117,7 @@ int kgdb_skipexception(int exception, struct pt_regs *regs) return kgdb_isremovedbreak(regs->nip); } -static int kgdb_call_nmi_hook(struct pt_regs *regs) +static int kgdb_debugger_ipi(struct pt_regs *regs) { kgdb_nmicallback(raw_smp_processor_id(), regs); return 0; @@ -502,7 +502,7 @@ int kgdb_arch_init(void) old__debugger_break_match = __debugger_break_match; old__debugger_fault_handler = __debugger_fault_handler; - __debugger_ipi = kgdb_call_nmi_hook; + __debugger_ipi = kgdb_debugger_ipi; __debugger = kgdb_debugger; __debugger_bpt = kgdb_handle_breakpoint; __debugger_sstep = kgdb_singlestep; diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index cc57630f6bf2..14e012ad7c57 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -314,18 +314,6 @@ BUILD_TRAP_HANDLER(singlestep) local_irq_restore(flags); } -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { int ret; diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 05e5b2eb0d32..24422865cd18 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -176,14 +176,25 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, char *remcom_out_buffer, struct pt_regs *regs); +/** + * kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU + * @ignored: This parameter is only here to match the prototype. + * + * If you're using the default implementation of kgdb_roundup_cpus() + * this function will be called per CPU. If you don't implement + * kgdb_call_nmi_hook() a default will be used. + */ + +extern void kgdb_call_nmi_hook(void *ignored); + /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. + * the NMI approach is not used for rounding up all the CPUs. Normally + * those architectures can just not implement this and get the default. * * On non-SMP systems, this is not called. */ diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index f3cadda45f07..d7444e6ead99 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -55,6 +55,7 @@ #include <linux/mm.h> #include <linux/vmacache.h> #include <linux/rcupdate.h> +#include <linux/irq.h> #include <asm/cacheflush.h> #include <asm/byteorder.h> @@ -220,6 +221,49 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs) return 0; } +#ifdef CONFIG_SMP + +/* + * Default (weak) implementation for kgdb_roundup_cpus + */ + +static DEFINE_PER_CPU(call_single_data_t, kgdb_roundup_csd); + +void __weak kgdb_call_nmi_hook(void *ignored) +{ + /* + * NOTE: get_irq_regs() is supposed to get the registers from + * before the IPI interrupt happened and so is supposed to + * show where the processor was. In some situations it's + * possible we might be called without an IPI, so it might be + * safer to figure out how to make kgdb_breakpoint() work + * properly here. + */ + kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); +} + +void __weak kgdb_roundup_cpus(void) +{ + call_single_data_t *csd; + int this_cpu = raw_smp_processor_id(); + int cpu; + int ret; + + for_each_online_cpu(cpu) { + /* No need to roundup ourselves */ + if (cpu == this_cpu) + continue; + + csd = &per_cpu(kgdb_roundup_csd, cpu); + csd->func = kgdb_call_nmi_hook; + ret = smp_call_function_single_async(cpu, csd); + if (ret) + kgdb_info[cpu].rounding_up = false; + } +} + +#endif + /* * Some architectures need cache flushes when we set/clear a * breakpoint: -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: linux-mips, Rich Felker, Catalin Marinas, linux-sh, Peter Zijlstra, Will Deacon, linux-kernel, Paul Mackerras, Yoshinori Sato, Russell King, kgdb-bugreport, James Hogan, linux-snps-arc, linux-arm-kernel, Vineet Gupta, Douglas Anderson, Ralf Baechle, Richard Kuo, Paul Burton, linux-hexagon, linuxppc-dev When I had lockdep turned on and dropped into kgdb I got a nice splat on my system. Specifically it hit: DEBUG_LOCKS_WARN_ON(current->hardirq_context) Specifically it looked like this: 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 ]--- Looking closely at it, it seems like a really bad idea to be calling local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems like it could violate spinlock semantics and cause a deadlock. Instead, let's use a private csd alongside smp_call_function_single_async() to round up the other CPUs. Using smp_call_function_single_async() doesn't require interrupts to be enabled so we can remove the offending bit of code. In order to avoid duplicating this across all the architectures that use the default kgdb_roundup_cpus(), we'll add a "weak" implementation to debug_core.c. Looking at all the people who previously had copies of this code, there were a few variants. I've attempted to keep the variants working like they used to. Specifically: * For arch/arc we passed NULL to kgdb_nmicallback() instead of get_irq_regs(). * For arch/mips there was a bit of extra code around kgdb_nmicallback() NOTE: In this patch we will still get into trouble if we try to round up a CPU that failed to round up before. We'll try to round it up again and potentially hang when we try to grab the csd lock. That's not new behavior but we'll still try to do better in a future patch. Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: None Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). arch/arc/kernel/kgdb.c | 10 ++------- arch/arm/kernel/kgdb.c | 12 ----------- arch/arm64/kernel/kgdb.c | 12 ----------- arch/hexagon/kernel/kgdb.c | 27 ----------------------- arch/mips/kernel/kgdb.c | 9 +------- arch/powerpc/kernel/kgdb.c | 4 ++-- arch/sh/kernel/kgdb.c | 12 ----------- include/linux/kgdb.h | 15 +++++++++++-- kernel/debug/debug_core.c | 44 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 62 insertions(+), 83 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 0932851028e0..68d9fe4b5aa7 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -192,18 +192,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long ip) instruction_pointer(regs) = ip; } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { + /* Default implementation passes get_irq_regs() but we don't */ kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - struct kgdb_arch arch_kgdb_ops = { /* breakpoint instruction: TRAP_S 0x3 */ #ifdef CONFIG_CPU_BIG_ENDIAN diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index f21077b077be..d9a69e941463 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -170,18 +170,6 @@ static struct undef_hook kgdb_compiled_brkpt_hook = { .fn = kgdb_compiled_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index 12c339ff6e75..da880247c734 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { .fn = kgdb_step_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 012e0e230ac2..b95d12038a4e 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -115,33 +115,6 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) instruction_pointer(regs) = pc; } -#ifdef CONFIG_SMP - -/** - * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * - * On SMP systems, we need to get the attention of the other CPUs - * and get them be in a known state. This should do what is needed - * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. - * - * On non-SMP systems, this is not called. - */ - -static void hexagon_kgdb_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); - local_irq_disable(); -} -#endif - /* Not yet working */ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index 2b05effc17b4..42f057a6c215 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -207,7 +207,7 @@ void arch_kgdb_breakpoint(void) ".set\treorder"); } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { mm_segment_t old_fs; @@ -219,13 +219,6 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int compute_signal(int tt) { struct hard_trap_info *ht; diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index b0e804844be0..b4ce54d73337 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -117,7 +117,7 @@ int kgdb_skipexception(int exception, struct pt_regs *regs) return kgdb_isremovedbreak(regs->nip); } -static int kgdb_call_nmi_hook(struct pt_regs *regs) +static int kgdb_debugger_ipi(struct pt_regs *regs) { kgdb_nmicallback(raw_smp_processor_id(), regs); return 0; @@ -502,7 +502,7 @@ int kgdb_arch_init(void) old__debugger_break_match = __debugger_break_match; old__debugger_fault_handler = __debugger_fault_handler; - __debugger_ipi = kgdb_call_nmi_hook; + __debugger_ipi = kgdb_debugger_ipi; __debugger = kgdb_debugger; __debugger_bpt = kgdb_handle_breakpoint; __debugger_sstep = kgdb_singlestep; diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index cc57630f6bf2..14e012ad7c57 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -314,18 +314,6 @@ BUILD_TRAP_HANDLER(singlestep) local_irq_restore(flags); } -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { int ret; diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 05e5b2eb0d32..24422865cd18 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -176,14 +176,25 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, char *remcom_out_buffer, struct pt_regs *regs); +/** + * kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU + * @ignored: This parameter is only here to match the prototype. + * + * If you're using the default implementation of kgdb_roundup_cpus() + * this function will be called per CPU. If you don't implement + * kgdb_call_nmi_hook() a default will be used. + */ + +extern void kgdb_call_nmi_hook(void *ignored); + /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. + * the NMI approach is not used for rounding up all the CPUs. Normally + * those architectures can just not implement this and get the default. * * On non-SMP systems, this is not called. */ diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index f3cadda45f07..d7444e6ead99 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -55,6 +55,7 @@ #include <linux/mm.h> #include <linux/vmacache.h> #include <linux/rcupdate.h> +#include <linux/irq.h> #include <asm/cacheflush.h> #include <asm/byteorder.h> @@ -220,6 +221,49 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs) return 0; } +#ifdef CONFIG_SMP + +/* + * Default (weak) implementation for kgdb_roundup_cpus + */ + +static DEFINE_PER_CPU(call_single_data_t, kgdb_roundup_csd); + +void __weak kgdb_call_nmi_hook(void *ignored) +{ + /* + * NOTE: get_irq_regs() is supposed to get the registers from + * before the IPI interrupt happened and so is supposed to + * show where the processor was. In some situations it's + * possible we might be called without an IPI, so it might be + * safer to figure out how to make kgdb_breakpoint() work + * properly here. + */ + kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); +} + +void __weak kgdb_roundup_cpus(void) +{ + call_single_data_t *csd; + int this_cpu = raw_smp_processor_id(); + int cpu; + int ret; + + for_each_online_cpu(cpu) { + /* No need to roundup ourselves */ + if (cpu == this_cpu) + continue; + + csd = &per_cpu(kgdb_roundup_csd, cpu); + csd->func = kgdb_call_nmi_hook; + ret = smp_call_function_single_async(cpu, csd); + if (ret) + kgdb_info[cpu].rounding_up = false; + } +} + +#endif + /* * Some architectures need cache flushes when we set/clear a * breakpoint: -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 3:51 ` Douglas Anderson 0 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: Will Deacon, kgdb-bugreport, Peter Zijlstra, Douglas Anderson, linux-mips, linux-sh, linux-kernel, Catalin Marinas, James Hogan, linux-hexagon, Vineet Gupta, Rich Felker, Ralf Baechle, linux-snps-arc, Yoshinori Sato, Benjamin Herrenschmidt, Paul Mackerras, Russell King, linux-arm-kernel, Christophe Leroy, Michael Ellerman, Paul Burton, Richard Kuo, linuxppc-dev When I had lockdep turned on and dropped into kgdb I got a nice splat on my system. Specifically it hit: DEBUG_LOCKS_WARN_ON(current->hardirq_context) Specifically it looked like this: 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 ]--- Looking closely at it, it seems like a really bad idea to be calling local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems like it could violate spinlock semantics and cause a deadlock. Instead, let's use a private csd alongside smp_call_function_single_async() to round up the other CPUs. Using smp_call_function_single_async() doesn't require interrupts to be enabled so we can remove the offending bit of code. In order to avoid duplicating this across all the architectures that use the default kgdb_roundup_cpus(), we'll add a "weak" implementation to debug_core.c. Looking at all the people who previously had copies of this code, there were a few variants. I've attempted to keep the variants working like they used to. Specifically: * For arch/arc we passed NULL to kgdb_nmicallback() instead of get_irq_regs(). * For arch/mips there was a bit of extra code around kgdb_nmicallback() NOTE: In this patch we will still get into trouble if we try to round up a CPU that failed to round up before. We'll try to round it up again and potentially hang when we try to grab the csd lock. That's not new behavior but we'll still try to do better in a future patch. Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v5: - Add a comment about get_irq_regs(). - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). - for_each_cpu() => for_each_online_cpu() - Error check smp_call_function_single_async() Changes in v4: None Changes in v3: - No separate init call. - Don't round up the CPU that is doing the rounding up. - Add "#ifdef CONFIG_SMP" to match the rest of the file. - Updated desc saying we don't solve the "failed to roundup" case. - Document the ignored parameter. Changes in v2: - Removing irq flags separated from fixing lockdep splat. - Don't use smp_call_function (Daniel). arch/arc/kernel/kgdb.c | 10 ++------- arch/arm/kernel/kgdb.c | 12 ----------- arch/arm64/kernel/kgdb.c | 12 ----------- arch/hexagon/kernel/kgdb.c | 27 ----------------------- arch/mips/kernel/kgdb.c | 9 +------- arch/powerpc/kernel/kgdb.c | 4 ++-- arch/sh/kernel/kgdb.c | 12 ----------- include/linux/kgdb.h | 15 +++++++++++-- kernel/debug/debug_core.c | 44 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 62 insertions(+), 83 deletions(-) diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c index 0932851028e0..68d9fe4b5aa7 100644 --- a/arch/arc/kernel/kgdb.c +++ b/arch/arc/kernel/kgdb.c @@ -192,18 +192,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long ip) instruction_pointer(regs) = ip; } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { + /* Default implementation passes get_irq_regs() but we don't */ kgdb_nmicallback(raw_smp_processor_id(), NULL); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - struct kgdb_arch arch_kgdb_ops = { /* breakpoint instruction: TRAP_S 0x3 */ #ifdef CONFIG_CPU_BIG_ENDIAN diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c index f21077b077be..d9a69e941463 100644 --- a/arch/arm/kernel/kgdb.c +++ b/arch/arm/kernel/kgdb.c @@ -170,18 +170,6 @@ static struct undef_hook kgdb_compiled_brkpt_hook = { .fn = kgdb_compiled_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index 12c339ff6e75..da880247c734 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { .fn = kgdb_step_brk_fn }; -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { struct pt_regs *regs = args->regs; diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c index 012e0e230ac2..b95d12038a4e 100644 --- a/arch/hexagon/kernel/kgdb.c +++ b/arch/hexagon/kernel/kgdb.c @@ -115,33 +115,6 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc) instruction_pointer(regs) = pc; } -#ifdef CONFIG_SMP - -/** - * kgdb_roundup_cpus - Get other CPUs into a holding pattern - * - * On SMP systems, we need to get the attention of the other CPUs - * and get them be in a known state. This should do what is needed - * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. - * - * On non-SMP systems, this is not called. - */ - -static void hexagon_kgdb_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0); - local_irq_disable(); -} -#endif - /* Not yet working */ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c index 2b05effc17b4..42f057a6c215 100644 --- a/arch/mips/kernel/kgdb.c +++ b/arch/mips/kernel/kgdb.c @@ -207,7 +207,7 @@ void arch_kgdb_breakpoint(void) ".set\treorder"); } -static void kgdb_call_nmi_hook(void *ignored) +void kgdb_call_nmi_hook(void *ignored) { mm_segment_t old_fs; @@ -219,13 +219,6 @@ static void kgdb_call_nmi_hook(void *ignored) set_fs(old_fs); } -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int compute_signal(int tt) { struct hard_trap_info *ht; diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c index b0e804844be0..b4ce54d73337 100644 --- a/arch/powerpc/kernel/kgdb.c +++ b/arch/powerpc/kernel/kgdb.c @@ -117,7 +117,7 @@ int kgdb_skipexception(int exception, struct pt_regs *regs) return kgdb_isremovedbreak(regs->nip); } -static int kgdb_call_nmi_hook(struct pt_regs *regs) +static int kgdb_debugger_ipi(struct pt_regs *regs) { kgdb_nmicallback(raw_smp_processor_id(), regs); return 0; @@ -502,7 +502,7 @@ int kgdb_arch_init(void) old__debugger_break_match = __debugger_break_match; old__debugger_fault_handler = __debugger_fault_handler; - __debugger_ipi = kgdb_call_nmi_hook; + __debugger_ipi = kgdb_debugger_ipi; __debugger = kgdb_debugger; __debugger_bpt = kgdb_handle_breakpoint; __debugger_sstep = kgdb_singlestep; diff --git a/arch/sh/kernel/kgdb.c b/arch/sh/kernel/kgdb.c index cc57630f6bf2..14e012ad7c57 100644 --- a/arch/sh/kernel/kgdb.c +++ b/arch/sh/kernel/kgdb.c @@ -314,18 +314,6 @@ BUILD_TRAP_HANDLER(singlestep) local_irq_restore(flags); } -static void kgdb_call_nmi_hook(void *ignored) -{ - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); -} - -void kgdb_roundup_cpus(void) -{ - local_irq_enable(); - smp_call_function(kgdb_call_nmi_hook, NULL, 0); - local_irq_disable(); -} - static int __kgdb_notify(struct die_args *args, unsigned long cmd) { int ret; diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h index 05e5b2eb0d32..24422865cd18 100644 --- a/include/linux/kgdb.h +++ b/include/linux/kgdb.h @@ -176,14 +176,25 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code, char *remcom_out_buffer, struct pt_regs *regs); +/** + * kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU + * @ignored: This parameter is only here to match the prototype. + * + * If you're using the default implementation of kgdb_roundup_cpus() + * this function will be called per CPU. If you don't implement + * kgdb_call_nmi_hook() a default will be used. + */ + +extern void kgdb_call_nmi_hook(void *ignored); + /** * kgdb_roundup_cpus - Get other CPUs into a holding pattern * * On SMP systems, we need to get the attention of the other CPUs * and get them into a known state. This should do what is needed * to get the other CPUs to call kgdb_wait(). Note that on some arches, - * the NMI approach is not used for rounding up all the CPUs. For example, - * in case of MIPS, smp_call_function() is used to roundup CPUs. + * the NMI approach is not used for rounding up all the CPUs. Normally + * those architectures can just not implement this and get the default. * * On non-SMP systems, this is not called. */ diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index f3cadda45f07..d7444e6ead99 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -55,6 +55,7 @@ #include <linux/mm.h> #include <linux/vmacache.h> #include <linux/rcupdate.h> +#include <linux/irq.h> #include <asm/cacheflush.h> #include <asm/byteorder.h> @@ -220,6 +221,49 @@ int __weak kgdb_skipexception(int exception, struct pt_regs *regs) return 0; } +#ifdef CONFIG_SMP + +/* + * Default (weak) implementation for kgdb_roundup_cpus + */ + +static DEFINE_PER_CPU(call_single_data_t, kgdb_roundup_csd); + +void __weak kgdb_call_nmi_hook(void *ignored) +{ + /* + * NOTE: get_irq_regs() is supposed to get the registers from + * before the IPI interrupt happened and so is supposed to + * show where the processor was. In some situations it's + * possible we might be called without an IPI, so it might be + * safer to figure out how to make kgdb_breakpoint() work + * properly here. + */ + kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); +} + +void __weak kgdb_roundup_cpus(void) +{ + call_single_data_t *csd; + int this_cpu = raw_smp_processor_id(); + int cpu; + int ret; + + for_each_online_cpu(cpu) { + /* No need to roundup ourselves */ + if (cpu == this_cpu) + continue; + + csd = &per_cpu(kgdb_roundup_csd, cpu); + csd->func = kgdb_call_nmi_hook; + ret = smp_call_function_single_async(cpu, csd); + if (ret) + kgdb_info[cpu].rounding_up = false; + } +} + +#endif + /* * Some architectures need cache flushes when we set/clear a * breakpoint: -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() 2018-11-27 3:51 ` Douglas Anderson ` (4 preceding siblings ...) (?) @ 2018-11-27 5:38 ` kbuild test robot -1 siblings, 0 replies; 39+ messages in thread From: kbuild test robot @ 2018-11-27 5:38 UTC (permalink / raw) To: linux-sh [-- Attachment #1: Type: text/plain, Size: 2023 bytes --] Hi Douglas, Thank you for the patch! Yet something to improve: [auto build test ERROR on kgdb/kgdb-next] [also build test ERROR on v4.20-rc4 next-20181126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64 Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' kgdb_info[cpu].rounding_up = false; ^ vim +261 kernel/debug/debug_core.c 244 245 void __weak kgdb_roundup_cpus(void) 246 { 247 call_single_data_t *csd; 248 int this_cpu = raw_smp_processor_id(); 249 int cpu; 250 int ret; 251 252 for_each_online_cpu(cpu) { 253 /* No need to roundup ourselves */ 254 if (cpu == this_cpu) 255 continue; 256 257 csd = &per_cpu(kgdb_roundup_csd, cpu); 258 csd->func = kgdb_call_nmi_hook; 259 ret = smp_call_function_single_async(cpu, csd); 260 if (ret) > 261 kgdb_info[cpu].rounding_up = false; 262 } 263 } 264 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 55426 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 5:38 ` kbuild test robot 0 siblings, 0 replies; 39+ messages in thread From: kbuild test robot @ 2018-11-27 5:38 UTC (permalink / raw) To: Douglas Anderson Cc: linux-mips, Rich Felker, Benjamin Herrenschmidt, linux-sh, Peter Zijlstra, Catalin Marinas, Will Deacon, linux-kernel, Paul Mackerras, Michael Ellerman, Daniel Thompson, Yoshinori Sato, linux-hexagon, Russell King, kgdb-bugreport, James Hogan, linux-snps-arc, linux-arm-kernel, Christophe Leroy, Vineet Gupta, Douglas Anderson, Ralf Baechle, Richard Kuo, Paul Burton [-- Attachment #1: Type: text/plain, Size: 2023 bytes --] Hi Douglas, Thank you for the patch! Yet something to improve: [auto build test ERROR on kgdb/kgdb-next] [also build test ERROR on v4.20-rc4 next-20181126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64 Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' kgdb_info[cpu].rounding_up = false; ^ vim +261 kernel/debug/debug_core.c 244 245 void __weak kgdb_roundup_cpus(void) 246 { 247 call_single_data_t *csd; 248 int this_cpu = raw_smp_processor_id(); 249 int cpu; 250 int ret; 251 252 for_each_online_cpu(cpu) { 253 /* No need to roundup ourselves */ 254 if (cpu == this_cpu) 255 continue; 256 257 csd = &per_cpu(kgdb_roundup_csd, cpu); 258 csd->func = kgdb_call_nmi_hook; 259 ret = smp_call_function_single_async(cpu, csd); 260 if (ret) > 261 kgdb_info[cpu].rounding_up = false; 262 } 263 } 264 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 55426 bytes --] [-- Attachment #3: Type: text/plain, Size: 169 bytes --] _______________________________________________ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 5:38 ` kbuild test robot 0 siblings, 0 replies; 39+ messages in thread From: kbuild test robot @ 2018-11-27 5:38 UTC (permalink / raw) To: linux-arm-kernel Hi Douglas, Thank you for the patch! Yet something to improve: [auto build test ERROR on kgdb/kgdb-next] [also build test ERROR on v4.20-rc4 next-20181126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64 Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' kgdb_info[cpu].rounding_up = false; ^ vim +261 kernel/debug/debug_core.c 244 245 void __weak kgdb_roundup_cpus(void) 246 { 247 call_single_data_t *csd; 248 int this_cpu = raw_smp_processor_id(); 249 int cpu; 250 int ret; 251 252 for_each_online_cpu(cpu) { 253 /* No need to roundup ourselves */ 254 if (cpu == this_cpu) 255 continue; 256 257 csd = &per_cpu(kgdb_roundup_csd, cpu); 258 csd->func = kgdb_call_nmi_hook; 259 ret = smp_call_function_single_async(cpu, csd); 260 if (ret) > 261 kgdb_info[cpu].rounding_up = false; 262 } 263 } 264 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -------------- next part -------------- A non-text attachment was scrubbed... Name: .config.gz Type: application/gzip Size: 55426 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20181127/a09a6228/attachment-0001.gz> ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 5:38 ` kbuild test robot 0 siblings, 0 replies; 39+ messages in thread From: kbuild test robot @ 2018-11-27 5:38 UTC (permalink / raw) To: linux-snps-arc Hi Douglas, Thank you for the patch! Yet something to improve: [auto build test ERROR on kgdb/kgdb-next] [also build test ERROR on v4.20-rc4 next-20181126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64 Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' kgdb_info[cpu].rounding_up = false; ^ vim +261 kernel/debug/debug_core.c 244 245 void __weak kgdb_roundup_cpus(void) 246 { 247 call_single_data_t *csd; 248 int this_cpu = raw_smp_processor_id(); 249 int cpu; 250 int ret; 251 252 for_each_online_cpu(cpu) { 253 /* No need to roundup ourselves */ 254 if (cpu == this_cpu) 255 continue; 256 257 csd = &per_cpu(kgdb_roundup_csd, cpu); 258 csd->func = kgdb_call_nmi_hook; 259 ret = smp_call_function_single_async(cpu, csd); 260 if (ret) > 261 kgdb_info[cpu].rounding_up = false; 262 } 263 } 264 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -------------- next part -------------- A non-text attachment was scrubbed... Name: .config.gz Type: application/gzip Size: 55426 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-snps-arc/attachments/20181127/a09a6228/attachment-0001.gz> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 5:38 ` kbuild test robot 0 siblings, 0 replies; 39+ messages in thread From: kbuild test robot @ 2018-11-27 5:38 UTC (permalink / raw) To: Douglas Anderson Cc: linux-mips, Rich Felker, linux-sh, Peter Zijlstra, Catalin Marinas, Will Deacon, linux-kernel, Paul Mackerras, Daniel Thompson, Yoshinori Sato, linux-hexagon, Russell King, kgdb-bugreport, James Hogan, linux-snps-arc, linux-arm-kernel, Vineet Gupta, Douglas Anderson, Ralf Baechle, Richard Kuo, Paul Burton, kbuild-all, Jason Wessel, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 2023 bytes --] Hi Douglas, Thank you for the patch! Yet something to improve: [auto build test ERROR on kgdb/kgdb-next] [also build test ERROR on v4.20-rc4 next-20181126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64 Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' kgdb_info[cpu].rounding_up = false; ^ vim +261 kernel/debug/debug_core.c 244 245 void __weak kgdb_roundup_cpus(void) 246 { 247 call_single_data_t *csd; 248 int this_cpu = raw_smp_processor_id(); 249 int cpu; 250 int ret; 251 252 for_each_online_cpu(cpu) { 253 /* No need to roundup ourselves */ 254 if (cpu == this_cpu) 255 continue; 256 257 csd = &per_cpu(kgdb_roundup_csd, cpu); 258 csd->func = kgdb_call_nmi_hook; 259 ret = smp_call_function_single_async(cpu, csd); 260 if (ret) > 261 kgdb_info[cpu].rounding_up = false; 262 } 263 } 264 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 55426 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 5:38 ` kbuild test robot 0 siblings, 0 replies; 39+ messages in thread From: kbuild test robot @ 2018-11-27 5:38 UTC (permalink / raw) To: Douglas Anderson Cc: kbuild-all, Jason Wessel, Daniel Thompson, Will Deacon, kgdb-bugreport, Peter Zijlstra, linux-mips, linux-sh, linux-kernel, Catalin Marinas, James Hogan, linux-hexagon, Vineet Gupta, Rich Felker, Ralf Baechle, linux-snps-arc, Yoshinori Sato, Benjamin Herrenschmidt, Paul Mackerras, Russell King, linux-arm-kernel, Christophe Leroy, Michael Ellerman, Paul Burton, Richard Kuo, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 2023 bytes --] Hi Douglas, Thank you for the patch! Yet something to improve: [auto build test ERROR on kgdb/kgdb-next] [also build test ERROR on v4.20-rc4 next-20181126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64 Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' kgdb_info[cpu].rounding_up = false; ^ vim +261 kernel/debug/debug_core.c 244 245 void __weak kgdb_roundup_cpus(void) 246 { 247 call_single_data_t *csd; 248 int this_cpu = raw_smp_processor_id(); 249 int cpu; 250 int ret; 251 252 for_each_online_cpu(cpu) { 253 /* No need to roundup ourselves */ 254 if (cpu == this_cpu) 255 continue; 256 257 csd = &per_cpu(kgdb_roundup_csd, cpu); 258 csd->func = kgdb_call_nmi_hook; 259 ret = smp_call_function_single_async(cpu, csd); 260 if (ret) > 261 kgdb_info[cpu].rounding_up = false; 262 } 263 } 264 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 55426 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 5:38 ` kbuild test robot 0 siblings, 0 replies; 39+ messages in thread From: kbuild test robot @ 2018-11-27 5:38 UTC (permalink / raw) To: Douglas Anderson Cc: kbuild-all, Jason Wessel, Daniel Thompson, Will Deacon, kgdb-bugreport, Peter Zijlstra, Douglas Anderson, linux-mips, linux-sh, linux-kernel, Catalin Marinas, James Hogan, linux-hexagon, Vineet Gupta, Rich Felker, Ralf Baechle, linux-snps-arc, Yoshinori Sato, Benjamin Herrenschmidt, Paul Mackerras, Russell King, linux-arm-kernel, Christophe Leroy, Michael Ellerman, Paul Burton, Richard Kuo, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 2023 bytes --] Hi Douglas, Thank you for the patch! Yet something to improve: [auto build test ERROR on kgdb/kgdb-next] [also build test ERROR on v4.20-rc4 next-20181126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next config: sparc64-allyesconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sparc64 Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' kgdb_info[cpu].rounding_up = false; ^ vim +261 kernel/debug/debug_core.c 244 245 void __weak kgdb_roundup_cpus(void) 246 { 247 call_single_data_t *csd; 248 int this_cpu = raw_smp_processor_id(); 249 int cpu; 250 int ret; 251 252 for_each_online_cpu(cpu) { 253 /* No need to roundup ourselves */ 254 if (cpu == this_cpu) 255 continue; 256 257 csd = &per_cpu(kgdb_roundup_csd, cpu); 258 csd->func = kgdb_call_nmi_hook; 259 ret = smp_call_function_single_async(cpu, csd); 260 if (ret) > 261 kgdb_info[cpu].rounding_up = false; 262 } 263 } 264 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 55426 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() 2018-11-27 5:38 ` kbuild test robot ` (3 preceding siblings ...) (?) @ 2018-11-27 17:39 ` Doug Anderson -1 siblings, 0 replies; 39+ messages in thread From: Doug Anderson @ 2018-11-27 17:39 UTC (permalink / raw) To: linux-arm-kernel Hi, On Mon, Nov 26, 2018 at 9:39 PM kbuild test robot <lkp@intel.com> wrote: > > Hi Douglas, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on kgdb/kgdb-next] > [also build test ERROR on v4.20-rc4 next-20181126] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 > base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next > config: sparc64-allyesconfig (attached as .config) > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=7.2.0 make.cross ARCH=sparc64 > > Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. > It only hurts bisectibility. > > All errors (new ones prefixed by >>): > > kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': > >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' > kgdb_info[cpu].rounding_up = false; > ^ Oops, I squashed this into the wrong patch. It should have been in patch #3. Sorry about that. I'll send out a quick v6. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 17:39 ` Doug Anderson 0 siblings, 0 replies; 39+ messages in thread From: Doug Anderson @ 2018-11-27 17:39 UTC (permalink / raw) To: kbuild test robot Cc: kbuild-all, Jason Wessel, Daniel Thompson, Will Deacon, kgdb-bugreport, Peter Zijlstra, linux-mips, linux-sh, LKML, Catalin Marinas, jhogan, linux-hexagon, Vineet Gupta, dalias, Ralf Baechle, linux-snps-arc, Yoshinori Sato, Benjamin Herrenschmidt, paulus, Russell King - ARM Linux, Linux ARM, christophe.leroy, mpe Hi, On Mon, Nov 26, 2018 at 9:39 PM kbuild test robot <lkp@intel.com> wrote: > > Hi Douglas, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on kgdb/kgdb-next] > [also build test ERROR on v4.20-rc4 next-20181126] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 > base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next > config: sparc64-allyesconfig (attached as .config) > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=7.2.0 make.cross ARCH=sparc64 > > Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. > It only hurts bisectibility. > > All errors (new ones prefixed by >>): > > kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': > >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' > kgdb_info[cpu].rounding_up = false; > ^ Oops, I squashed this into the wrong patch. It should have been in patch #3. Sorry about that. I'll send out a quick v6. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 17:39 ` Doug Anderson 0 siblings, 0 replies; 39+ messages in thread From: Doug Anderson @ 2018-11-27 17:39 UTC (permalink / raw) To: linux-arm-kernel Hi, On Mon, Nov 26, 2018 at 9:39 PM kbuild test robot <lkp@intel.com> wrote: > > Hi Douglas, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on kgdb/kgdb-next] > [also build test ERROR on v4.20-rc4 next-20181126] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 > base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next > config: sparc64-allyesconfig (attached as .config) > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=7.2.0 make.cross ARCH=sparc64 > > Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. > It only hurts bisectibility. > > All errors (new ones prefixed by >>): > > kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': > >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' > kgdb_info[cpu].rounding_up = false; > ^ Oops, I squashed this into the wrong patch. It should have been in patch #3. Sorry about that. I'll send out a quick v6. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 17:39 ` Doug Anderson 0 siblings, 0 replies; 39+ messages in thread From: Doug Anderson @ 2018-11-27 17:39 UTC (permalink / raw) To: linux-snps-arc Hi, On Mon, Nov 26, 2018@9:39 PM kbuild test robot <lkp@intel.com> wrote: > > Hi Douglas, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on kgdb/kgdb-next] > [also build test ERROR on v4.20-rc4 next-20181126] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 > base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next > config: sparc64-allyesconfig (attached as .config) > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=7.2.0 make.cross ARCH=sparc64 > > Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. > It only hurts bisectibility. > > All errors (new ones prefixed by >>): > > kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': > >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' > kgdb_info[cpu].rounding_up = false; > ^ Oops, I squashed this into the wrong patch. It should have been in patch #3. Sorry about that. I'll send out a quick v6. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 17:39 ` Doug Anderson 0 siblings, 0 replies; 39+ messages in thread From: Doug Anderson @ 2018-11-27 17:39 UTC (permalink / raw) To: kbuild test robot Cc: linux-mips, dalias, linux-sh, Peter Zijlstra, Catalin Marinas, Will Deacon, paulus, Daniel Thompson, Yoshinori Sato, linux-hexagon, Russell King - ARM Linux, kgdb-bugreport, jhogan, linux-snps-arc, Linux ARM, Vineet Gupta, LKML, Ralf Baechle, rkuo, paul.burton, kbuild-all, Jason Wessel, linuxppc-dev Hi, On Mon, Nov 26, 2018 at 9:39 PM kbuild test robot <lkp@intel.com> wrote: > > Hi Douglas, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on kgdb/kgdb-next] > [also build test ERROR on v4.20-rc4 next-20181126] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 > base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next > config: sparc64-allyesconfig (attached as .config) > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=7.2.0 make.cross ARCH=sparc64 > > Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. > It only hurts bisectibility. > > All errors (new ones prefixed by >>): > > kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': > >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' > kgdb_info[cpu].rounding_up = false; > ^ Oops, I squashed this into the wrong patch. It should have been in patch #3. Sorry about that. I'll send out a quick v6. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 17:39 ` Doug Anderson 0 siblings, 0 replies; 39+ messages in thread From: Doug Anderson @ 2018-11-27 17:39 UTC (permalink / raw) To: kbuild test robot Cc: kbuild-all, Jason Wessel, Daniel Thompson, Will Deacon, kgdb-bugreport, Peter Zijlstra, linux-mips, linux-sh, LKML, Catalin Marinas, jhogan, linux-hexagon, Vineet Gupta, dalias, Ralf Baechle, linux-snps-arc, Yoshinori Sato, Benjamin Herrenschmidt, paulus, Russell King - ARM Linux, Linux ARM, christophe.leroy, mpe, paul.burton, rkuo, linuxppc-dev Hi, On Mon, Nov 26, 2018 at 9:39 PM kbuild test robot <lkp@intel.com> wrote: > > Hi Douglas, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on kgdb/kgdb-next] > [also build test ERROR on v4.20-rc4 next-20181126] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 > base: https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git kgdb-next > config: sparc64-allyesconfig (attached as .config) > compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=7.2.0 make.cross ARCH=sparc64 > > Note: the linux-review/Douglas-Anderson/kgdb-Fix-kgdb_roundup_cpus/20181127-115425 HEAD b8d0502e65f2d7a2187baa69870146a6fbf18c9f builds fine. > It only hurts bisectibility. > > All errors (new ones prefixed by >>): > > kernel/debug/debug_core.c: In function 'kgdb_roundup_cpus': > >> kernel/debug/debug_core.c:261:18: error: 'struct debuggerinfo_struct' has no member named 'rounding_up' > kgdb_info[cpu].rounding_up = false; > ^ Oops, I squashed this into the wrong patch. It should have been in patch #3. Sorry about that. I'll send out a quick v6. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() 2018-11-27 3:51 ` Douglas Anderson ` (3 preceding siblings ...) (?) @ 2018-11-27 19:30 ` Will Deacon -1 siblings, 0 replies; 39+ messages in thread From: Will Deacon @ 2018-11-27 19:30 UTC (permalink / raw) To: linux-arm-kernel On Mon, Nov 26, 2018 at 07:51:31PM -0800, Douglas Anderson wrote: > When I had lockdep turned on and dropped into kgdb I got a nice splat > on my system. Specifically it hit: > DEBUG_LOCKS_WARN_ON(current->hardirq_context) > > Specifically it looked like this: > 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 ]--- > > Looking closely at it, it seems like a really bad idea to be calling > local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems > like it could violate spinlock semantics and cause a deadlock. > > Instead, let's use a private csd alongside > smp_call_function_single_async() to round up the other CPUs. Using > smp_call_function_single_async() doesn't require interrupts to be > enabled so we can remove the offending bit of code. > > In order to avoid duplicating this across all the architectures that > use the default kgdb_roundup_cpus(), we'll add a "weak" implementation > to debug_core.c. > > Looking at all the people who previously had copies of this code, > there were a few variants. I've attempted to keep the variants > working like they used to. Specifically: > * For arch/arc we passed NULL to kgdb_nmicallback() instead of > get_irq_regs(). > * For arch/mips there was a bit of extra code around > kgdb_nmicallback() > > NOTE: In this patch we will still get into trouble if we try to round > up a CPU that failed to round up before. We'll try to round it up > again and potentially hang when we try to grab the csd lock. That's > not new behavior but we'll still try to do better in a future patch. > > Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v5: > - Add a comment about get_irq_regs(). > - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). > - for_each_cpu() => for_each_online_cpu() > - Error check smp_call_function_single_async() For the arm64 bit: > diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c > index 12c339ff6e75..da880247c734 100644 > --- a/arch/arm64/kernel/kgdb.c > +++ b/arch/arm64/kernel/kgdb.c > @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { > .fn = kgdb_step_brk_fn > }; > > -static void kgdb_call_nmi_hook(void *ignored) > -{ > - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); > -} > - > -void kgdb_roundup_cpus(void) > -{ > - local_irq_enable(); > - smp_call_function(kgdb_call_nmi_hook, NULL, 0); > - local_irq_disable(); > -} > - Acked-by: Will Deacon <will.deacon@arm.com> Will ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 19:30 ` Will Deacon 0 siblings, 0 replies; 39+ messages in thread From: Will Deacon @ 2018-11-27 19:30 UTC (permalink / raw) To: Douglas Anderson Cc: Jason Wessel, Daniel Thompson, kgdb-bugreport, Peter Zijlstra, linux-mips, linux-sh, linux-kernel, Catalin Marinas, James Hogan, linux-hexagon, Vineet Gupta, Rich Felker, Ralf Baechle, linux-snps-arc, Yoshinori Sato, Benjamin Herrenschmidt, Paul Mackerras, Russell King, linux-arm-kernel, Christophe Leroy, Michae On Mon, Nov 26, 2018 at 07:51:31PM -0800, Douglas Anderson wrote: > When I had lockdep turned on and dropped into kgdb I got a nice splat > on my system. Specifically it hit: > DEBUG_LOCKS_WARN_ON(current->hardirq_context) > > Specifically it looked like this: > 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 ]--- > > Looking closely at it, it seems like a really bad idea to be calling > local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems > like it could violate spinlock semantics and cause a deadlock. > > Instead, let's use a private csd alongside > smp_call_function_single_async() to round up the other CPUs. Using > smp_call_function_single_async() doesn't require interrupts to be > enabled so we can remove the offending bit of code. > > In order to avoid duplicating this across all the architectures that > use the default kgdb_roundup_cpus(), we'll add a "weak" implementation > to debug_core.c. > > Looking at all the people who previously had copies of this code, > there were a few variants. I've attempted to keep the variants > working like they used to. Specifically: > * For arch/arc we passed NULL to kgdb_nmicallback() instead of > get_irq_regs(). > * For arch/mips there was a bit of extra code around > kgdb_nmicallback() > > NOTE: In this patch we will still get into trouble if we try to round > up a CPU that failed to round up before. We'll try to round it up > again and potentially hang when we try to grab the csd lock. That's > not new behavior but we'll still try to do better in a future patch. > > Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v5: > - Add a comment about get_irq_regs(). > - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). > - for_each_cpu() => for_each_online_cpu() > - Error check smp_call_function_single_async() For the arm64 bit: > diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c > index 12c339ff6e75..da880247c734 100644 > --- a/arch/arm64/kernel/kgdb.c > +++ b/arch/arm64/kernel/kgdb.c > @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { > .fn = kgdb_step_brk_fn > }; > > -static void kgdb_call_nmi_hook(void *ignored) > -{ > - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); > -} > - > -void kgdb_roundup_cpus(void) > -{ > - local_irq_enable(); > - smp_call_function(kgdb_call_nmi_hook, NULL, 0); > - local_irq_disable(); > -} > - Acked-by: Will Deacon <will.deacon@arm.com> Will ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 19:30 ` Will Deacon 0 siblings, 0 replies; 39+ messages in thread From: Will Deacon @ 2018-11-27 19:30 UTC (permalink / raw) To: linux-arm-kernel On Mon, Nov 26, 2018 at 07:51:31PM -0800, Douglas Anderson wrote: > When I had lockdep turned on and dropped into kgdb I got a nice splat > on my system. Specifically it hit: > DEBUG_LOCKS_WARN_ON(current->hardirq_context) > > Specifically it looked like this: > 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 ]--- > > Looking closely at it, it seems like a really bad idea to be calling > local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems > like it could violate spinlock semantics and cause a deadlock. > > Instead, let's use a private csd alongside > smp_call_function_single_async() to round up the other CPUs. Using > smp_call_function_single_async() doesn't require interrupts to be > enabled so we can remove the offending bit of code. > > In order to avoid duplicating this across all the architectures that > use the default kgdb_roundup_cpus(), we'll add a "weak" implementation > to debug_core.c. > > Looking at all the people who previously had copies of this code, > there were a few variants. I've attempted to keep the variants > working like they used to. Specifically: > * For arch/arc we passed NULL to kgdb_nmicallback() instead of > get_irq_regs(). > * For arch/mips there was a bit of extra code around > kgdb_nmicallback() > > NOTE: In this patch we will still get into trouble if we try to round > up a CPU that failed to round up before. We'll try to round it up > again and potentially hang when we try to grab the csd lock. That's > not new behavior but we'll still try to do better in a future patch. > > Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v5: > - Add a comment about get_irq_regs(). > - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). > - for_each_cpu() => for_each_online_cpu() > - Error check smp_call_function_single_async() For the arm64 bit: > diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c > index 12c339ff6e75..da880247c734 100644 > --- a/arch/arm64/kernel/kgdb.c > +++ b/arch/arm64/kernel/kgdb.c > @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { > .fn = kgdb_step_brk_fn > }; > > -static void kgdb_call_nmi_hook(void *ignored) > -{ > - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); > -} > - > -void kgdb_roundup_cpus(void) > -{ > - local_irq_enable(); > - smp_call_function(kgdb_call_nmi_hook, NULL, 0); > - local_irq_disable(); > -} > - Acked-by: Will Deacon <will.deacon@arm.com> Will ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 19:30 ` Will Deacon 0 siblings, 0 replies; 39+ messages in thread From: Will Deacon @ 2018-11-27 19:30 UTC (permalink / raw) To: linux-snps-arc On Mon, Nov 26, 2018@07:51:31PM -0800, Douglas Anderson wrote: > When I had lockdep turned on and dropped into kgdb I got a nice splat > on my system. Specifically it hit: > DEBUG_LOCKS_WARN_ON(current->hardirq_context) > > Specifically it looked like this: > 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 ]--- > > Looking closely at it, it seems like a really bad idea to be calling > local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems > like it could violate spinlock semantics and cause a deadlock. > > Instead, let's use a private csd alongside > smp_call_function_single_async() to round up the other CPUs. Using > smp_call_function_single_async() doesn't require interrupts to be > enabled so we can remove the offending bit of code. > > In order to avoid duplicating this across all the architectures that > use the default kgdb_roundup_cpus(), we'll add a "weak" implementation > to debug_core.c. > > Looking at all the people who previously had copies of this code, > there were a few variants. I've attempted to keep the variants > working like they used to. Specifically: > * For arch/arc we passed NULL to kgdb_nmicallback() instead of > get_irq_regs(). > * For arch/mips there was a bit of extra code around > kgdb_nmicallback() > > NOTE: In this patch we will still get into trouble if we try to round > up a CPU that failed to round up before. We'll try to round it up > again and potentially hang when we try to grab the csd lock. That's > not new behavior but we'll still try to do better in a future patch. > > Suggested-by: Daniel Thompson <daniel.thompson at linaro.org> > Signed-off-by: Douglas Anderson <dianders at chromium.org> > --- > > Changes in v5: > - Add a comment about get_irq_regs(). > - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). > - for_each_cpu() => for_each_online_cpu() > - Error check smp_call_function_single_async() For the arm64 bit: > diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c > index 12c339ff6e75..da880247c734 100644 > --- a/arch/arm64/kernel/kgdb.c > +++ b/arch/arm64/kernel/kgdb.c > @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { > .fn = kgdb_step_brk_fn > }; > > -static void kgdb_call_nmi_hook(void *ignored) > -{ > - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); > -} > - > -void kgdb_roundup_cpus(void) > -{ > - local_irq_enable(); > - smp_call_function(kgdb_call_nmi_hook, NULL, 0); > - local_irq_disable(); > -} > - Acked-by: Will Deacon <will.deacon at arm.com> Will ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 19:30 ` Will Deacon 0 siblings, 0 replies; 39+ messages in thread From: Will Deacon @ 2018-11-27 19:30 UTC (permalink / raw) To: Douglas Anderson Cc: linux-mips, Rich Felker, linux-sh, Peter Zijlstra, Catalin Marinas, Paul Mackerras, linux-hexagon, Daniel Thompson, Yoshinori Sato, Russell King, kgdb-bugreport, James Hogan, linux-snps-arc, linux-arm-kernel, Vineet Gupta, linux-kernel, Ralf Baechle, Richard Kuo, Paul Burton, Jason Wessel, linuxppc-dev On Mon, Nov 26, 2018 at 07:51:31PM -0800, Douglas Anderson wrote: > When I had lockdep turned on and dropped into kgdb I got a nice splat > on my system. Specifically it hit: > DEBUG_LOCKS_WARN_ON(current->hardirq_context) > > Specifically it looked like this: > 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 ]--- > > Looking closely at it, it seems like a really bad idea to be calling > local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems > like it could violate spinlock semantics and cause a deadlock. > > Instead, let's use a private csd alongside > smp_call_function_single_async() to round up the other CPUs. Using > smp_call_function_single_async() doesn't require interrupts to be > enabled so we can remove the offending bit of code. > > In order to avoid duplicating this across all the architectures that > use the default kgdb_roundup_cpus(), we'll add a "weak" implementation > to debug_core.c. > > Looking at all the people who previously had copies of this code, > there were a few variants. I've attempted to keep the variants > working like they used to. Specifically: > * For arch/arc we passed NULL to kgdb_nmicallback() instead of > get_irq_regs(). > * For arch/mips there was a bit of extra code around > kgdb_nmicallback() > > NOTE: In this patch we will still get into trouble if we try to round > up a CPU that failed to round up before. We'll try to round it up > again and potentially hang when we try to grab the csd lock. That's > not new behavior but we'll still try to do better in a future patch. > > Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v5: > - Add a comment about get_irq_regs(). > - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). > - for_each_cpu() => for_each_online_cpu() > - Error check smp_call_function_single_async() For the arm64 bit: > diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c > index 12c339ff6e75..da880247c734 100644 > --- a/arch/arm64/kernel/kgdb.c > +++ b/arch/arm64/kernel/kgdb.c > @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { > .fn = kgdb_step_brk_fn > }; > > -static void kgdb_call_nmi_hook(void *ignored) > -{ > - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); > -} > - > -void kgdb_roundup_cpus(void) > -{ > - local_irq_enable(); > - smp_call_function(kgdb_call_nmi_hook, NULL, 0); > - local_irq_disable(); > -} > - Acked-by: Will Deacon <will.deacon@arm.com> Will ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() @ 2018-11-27 19:30 ` Will Deacon 0 siblings, 0 replies; 39+ messages in thread From: Will Deacon @ 2018-11-27 19:30 UTC (permalink / raw) To: Douglas Anderson Cc: Jason Wessel, Daniel Thompson, kgdb-bugreport, Peter Zijlstra, linux-mips, linux-sh, linux-kernel, Catalin Marinas, James Hogan, linux-hexagon, Vineet Gupta, Rich Felker, Ralf Baechle, linux-snps-arc, Yoshinori Sato, Benjamin Herrenschmidt, Paul Mackerras, Russell King, linux-arm-kernel, Christophe Leroy, Michael Ellerman, Paul Burton, Richard Kuo, linuxppc-dev On Mon, Nov 26, 2018 at 07:51:31PM -0800, Douglas Anderson wrote: > When I had lockdep turned on and dropped into kgdb I got a nice splat > on my system. Specifically it hit: > DEBUG_LOCKS_WARN_ON(current->hardirq_context) > > Specifically it looked like this: > 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 ]--- > > Looking closely at it, it seems like a really bad idea to be calling > local_irq_enable() in kgdb_roundup_cpus(). If nothing else that seems > like it could violate spinlock semantics and cause a deadlock. > > Instead, let's use a private csd alongside > smp_call_function_single_async() to round up the other CPUs. Using > smp_call_function_single_async() doesn't require interrupts to be > enabled so we can remove the offending bit of code. > > In order to avoid duplicating this across all the architectures that > use the default kgdb_roundup_cpus(), we'll add a "weak" implementation > to debug_core.c. > > Looking at all the people who previously had copies of this code, > there were a few variants. I've attempted to keep the variants > working like they used to. Specifically: > * For arch/arc we passed NULL to kgdb_nmicallback() instead of > get_irq_regs(). > * For arch/mips there was a bit of extra code around > kgdb_nmicallback() > > NOTE: In this patch we will still get into trouble if we try to round > up a CPU that failed to round up before. We'll try to round it up > again and potentially hang when we try to grab the csd lock. That's > not new behavior but we'll still try to do better in a future patch. > > Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v5: > - Add a comment about get_irq_regs(). > - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus(). > - for_each_cpu() => for_each_online_cpu() > - Error check smp_call_function_single_async() For the arm64 bit: > diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c > index 12c339ff6e75..da880247c734 100644 > --- a/arch/arm64/kernel/kgdb.c > +++ b/arch/arm64/kernel/kgdb.c > @@ -284,18 +284,6 @@ static struct step_hook kgdb_step_hook = { > .fn = kgdb_step_brk_fn > }; > > -static void kgdb_call_nmi_hook(void *ignored) > -{ > - kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs()); > -} > - > -void kgdb_roundup_cpus(void) > -{ > - local_irq_enable(); > - smp_call_function(kgdb_call_nmi_hook, NULL, 0); > - local_irq_disable(); > -} > - Acked-by: Will Deacon <will.deacon@arm.com> Will ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 3/4] kgdb: Don't round up a CPU that failed rounding up before 2018-11-27 3:51 ` Douglas Anderson ` (6 preceding siblings ...) (?) @ 2018-11-27 3:51 ` Douglas Anderson -1 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: Will Deacon, kgdb-bugreport, Peter Zijlstra, Douglas Anderson, linux-kernel If we're using the default implementation of kgdb_roundup_cpus() that uses smp_call_function_single_async() we can end up hanging kgdb_roundup_cpus() if we try to round up a CPU that failed to round up before. Specifically smp_call_function_single_async() will try to wait on the csd lock for the CPU that we're trying to round up. If the previous round up never finished then that lock could still be held and we'll just sit there hanging. There's not a lot of use trying to round up a CPU that failed to round up before. Let's keep a flag that indicates whether the CPU started but didn't finish to round up before. If we see that flag set then we'll skip the next round up. In general we have a few goals here: - We never want to end up calling smp_call_function_single_async() when the csd is still locked. This is accomplished because flush_smp_call_function_queue() unlocks the csd _before_ invoking the callback. That means that when kgdb_nmicallback() runs we know for sure the the csd is no longer locked. Thus when we set "rounding_up = false" we know for sure that the csd is unlocked. - If there are no timeouts rounding up we should never skip a round up. NOTE #1: In general trying to continue running after failing to round up CPUs doesn't appear to be supported in the debugger. When I simulate this I find that kdb reports "Catastrophic error detected" when I try to continue. I can overrule and continue anyway, but it should be noted that we may be entering the land of dragons here. Possibly the "Catastrophic error detected" was added _because_ of the future failure to round up, but even so this is an area of the code that hasn't been strongly tested. NOTE #2: I did a bit of testing before and after this change. I introduced a 10 second hang in the kernel while holding a spinlock that I could invoke on a certain CPU with 'taskset -c 3 cat /sys/...". Before this change if I did: - Invoke hang - Enter debugger - g (which warns about Catastrophic error, g again to go anyway) - g - Enter debugger ...I'd hang the rest of the 10 seconds without getting a debugger prompt. After this change I end up in the debugger the 2nd time after only 1 second with the standard warning about 'Timed out waiting for secondary CPUs.' I'll also note that once the CPU finished waiting I could actually debug it (aka "btc" worked) I won't promise that everything works perfectly if the errant CPU comes back at just the wrong time (like as we're entering or exiting the debugger) but it certainly seems like an improvement. NOTE #3: setting 'kgdb_info[cpu].rounding_up = false' is in kgdb_nmicallback() instead of kgdb_call_nmi_hook() because some implementations override kgdb_call_nmi_hook(). It shouldn't hurt to have it in kgdb_nmicallback() in any case. NOTE #4: this logic is really only needed because there is no API call like "smp_try_call_function_single_async()" or "smp_csd_is_locked()". If such an API existed then we'd use it instead, but it seemed a bit much to add an API like this just for kgdb. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v5: None Changes in v4: - Removed smp_mb() calls. Changes in v3: - Don't round up a CPU that failed rounding up before new for v3. Changes in v2: None kernel/debug/debug_core.c | 15 +++++++++++++++ kernel/debug/debug_core.h | 1 + 2 files changed, 16 insertions(+) diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index d7444e6ead99..1fb8b239e567 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -255,6 +255,19 @@ void __weak kgdb_roundup_cpus(void) continue; csd = &per_cpu(kgdb_roundup_csd, cpu); + + /* + * If it didn't round up last time, don't try again + * since smp_call_function_single_async() will block. + * + * If rounding_up is false then we know that the + * previous call must have at least started and that + * means smp_call_function_single_async() won't block. + */ + if (kgdb_info[cpu].rounding_up) + continue; + kgdb_info[cpu].rounding_up = true; + csd->func = kgdb_call_nmi_hook; ret = smp_call_function_single_async(cpu, csd); if (ret) @@ -791,6 +804,8 @@ int kgdb_nmicallback(int cpu, void *regs) struct kgdb_state kgdb_var; struct kgdb_state *ks = &kgdb_var; + kgdb_info[cpu].rounding_up = false; + memset(ks, 0, sizeof(struct kgdb_state)); ks->cpu = cpu; ks->linux_regs = regs; diff --git a/kernel/debug/debug_core.h b/kernel/debug/debug_core.h index 127d9bc49fb4..b4a7c326d546 100644 --- a/kernel/debug/debug_core.h +++ b/kernel/debug/debug_core.h @@ -42,6 +42,7 @@ struct debuggerinfo_struct { int ret_state; int irq_depth; int enter_kgdb; + bool rounding_up; }; extern struct debuggerinfo_struct kgdb_info[]; -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 4/4] kdb: Don't back trace on a cpu that didn't round up 2018-11-27 3:51 ` Douglas Anderson ` (7 preceding siblings ...) (?) @ 2018-11-27 3:51 ` Douglas Anderson -1 siblings, 0 replies; 39+ messages in thread From: Douglas Anderson @ 2018-11-27 3:51 UTC (permalink / raw) To: Jason Wessel, Daniel Thompson Cc: Will Deacon, kgdb-bugreport, Peter Zijlstra, Douglas Anderson, Christophe Leroy, linux-kernel If you have a CPU that fails to round up and then run 'btc' you'll end up crashing in kdb becaue we dereferenced NULL. Let's add a check. It's wise to also set the task to NULL when leaving the debugger so that if we fail to round up on a later entry into the debugger we won't backtrace a stale task. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v5: None Changes in v4: - Also clear out .debuggerinfo. - Also clear out .debuggerinfo and .task for the master. - Remove clearing out in kdb_stub for offline CPUs; it's now redundant. Changes in v3: - Don't back trace on a cpu that didn't round up new for v3. Changes in v2: None kernel/debug/debug_core.c | 4 ++++ kernel/debug/kdb/kdb_bt.c | 11 ++++++++++- kernel/debug/kdb/kdb_debugger.c | 7 ------- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c index 1fb8b239e567..5cc608de6883 100644 --- a/kernel/debug/debug_core.c +++ b/kernel/debug/debug_core.c @@ -592,6 +592,8 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs, arch_kgdb_ops.correct_hw_break(); if (trace_on) tracing_on(); + kgdb_info[cpu].debuggerinfo = NULL; + kgdb_info[cpu].task = NULL; kgdb_info[cpu].exception_state &= ~(DCPU_WANT_MASTER | DCPU_IS_SLAVE); kgdb_info[cpu].enter_kgdb--; @@ -724,6 +726,8 @@ static int kgdb_cpu_enter(struct kgdb_state *ks, struct pt_regs *regs, if (trace_on) tracing_on(); + kgdb_info[cpu].debuggerinfo = NULL; + kgdb_info[cpu].task = NULL; kgdb_info[cpu].exception_state &= ~(DCPU_WANT_MASTER | DCPU_IS_SLAVE); kgdb_info[cpu].enter_kgdb--; diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c index 7921ae4fca8d..7e2379aa0a1e 100644 --- a/kernel/debug/kdb/kdb_bt.c +++ b/kernel/debug/kdb/kdb_bt.c @@ -186,7 +186,16 @@ kdb_bt(int argc, const char **argv) kdb_printf("btc: cpu status: "); kdb_parse("cpu\n"); for_each_online_cpu(cpu) { - sprintf(buf, "btt 0x%px\n", KDB_TSK(cpu)); + void *kdb_tsk = KDB_TSK(cpu); + + /* If a CPU failed to round up we could be here */ + if (!kdb_tsk) { + kdb_printf("WARNING: no task for cpu %ld\n", + cpu); + continue; + } + + sprintf(buf, "btt 0x%px\n", kdb_tsk); kdb_parse(buf); touch_nmi_watchdog(); } diff --git a/kernel/debug/kdb/kdb_debugger.c b/kernel/debug/kdb/kdb_debugger.c index 15e1a7af5dd0..53a0df6e4d92 100644 --- a/kernel/debug/kdb/kdb_debugger.c +++ b/kernel/debug/kdb/kdb_debugger.c @@ -118,13 +118,6 @@ int kdb_stub(struct kgdb_state *ks) kdb_bp_remove(); KDB_STATE_CLEAR(DOING_SS); KDB_STATE_SET(PAGER); - /* zero out any offline cpu data */ - for_each_present_cpu(i) { - if (!cpu_online(i)) { - kgdb_info[i].debuggerinfo = NULL; - kgdb_info[i].task = NULL; - } - } if (ks->err_code == DIE_OOPS || reason == KDB_REASON_OOPS) { ks->pass_exception = 1; KDB_FLAG_SET(CATASTROPHIC); -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 39+ messages in thread
end of thread, other threads:[~2018-11-27 19:32 UTC | newest] Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-27 3:51 [PATCH v5 0/4] kgdb: Fix kgdb_roundup_cpus() Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` [PATCH v5 1/4] kgdb: Remove irq flags from roundup Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` [PATCH v5 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function() Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 3:51 ` Douglas Anderson 2018-11-27 5:38 ` kbuild test robot 2018-11-27 5:38 ` kbuild test robot 2018-11-27 5:38 ` kbuild test robot 2018-11-27 5:38 ` kbuild test robot 2018-11-27 5:38 ` kbuild test robot 2018-11-27 5:38 ` kbuild test robot 2018-11-27 5:38 ` kbuild test robot 2018-11-27 17:39 ` Doug Anderson 2018-11-27 17:39 ` Doug Anderson 2018-11-27 17:39 ` Doug Anderson 2018-11-27 17:39 ` Doug Anderson 2018-11-27 17:39 ` Doug Anderson 2018-11-27 17:39 ` Doug Anderson 2018-11-27 19:30 ` Will Deacon 2018-11-27 19:30 ` Will Deacon 2018-11-27 19:30 ` Will Deacon 2018-11-27 19:30 ` Will Deacon 2018-11-27 19:30 ` Will Deacon 2018-11-27 19:30 ` Will Deacon 2018-11-27 3:51 ` [PATCH v5 3/4] kgdb: Don't round up a CPU that failed rounding up before Douglas Anderson 2018-11-27 3:51 ` [PATCH v5 4/4] kdb: Don't back trace on a cpu that didn't round up Douglas Anderson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.