lkml.org 
[lkml]   [2023]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] softlockup: serialized softlockup's log
    On Wed, 22 Nov 2023 18:02:12 +0800 lizhe.67@bytedance.com wrote:

    > From: Li Zhe <lizhe.67@bytedance.com>
    >
    > If multiple CPUs trigger softlockup at the same time with
    > 'softlockup_all_cpu_backtrace=0', the softlockup's logs will appear
    > staggeredly in dmesg, which will affect the viewing of the logs for
    > developer. Since the code path for outputting softlockup logs is not
    > a kernel hotspot and the performance requirements for the code are
    > not strict, locks are used to serialize the softlockup log output to
    > improve the readability of the logs.

    Seems reasonable, but...

    > --- a/kernel/watchdog.c
    > +++ b/kernel/watchdog.c
    > @@ -28,6 +28,8 @@
    > #include <linux/kvm_para.h>
    >
    > static DEFINE_MUTEX(watchdog_mutex);
    > +/* This lock is used to prevent concurrent actions of softlockup output logs */
    > +static DEFINE_SPINLOCK(watchdog_output_lock);

    It would be a little neater to reduce the scope of this - move the
    definition into that little code block in watchdog_timer_fn() where it
    is actually used.

    > #if defined(CONFIG_HARDLOCKUP_DETECTOR) || defined(CONFIG_HARDLOCKUP_DETECTOR_SPARC64)
    > # define WATCHDOG_HARDLOCKUP_DEFAULT 1
    > @@ -514,6 +516,7 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
    > /* Start period for the next softlockup warning. */
    > update_report_ts();
    >
    > + spin_lock(&watchdog_output_lock);

    The hrtimer callout function is called from [soft]irq context, yes?
    Doesn't lockdep get upset when we take a spinlock in such a context?

    > pr_emerg("BUG: soft lockup - CPU#%d stuck for %us! [%s:%d]\n",
    > smp_processor_id(), duration,
    > current->comm, task_pid_nr(current));
    > @@ -523,6 +526,7 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
    > show_regs(regs);
    > else
    > dump_stack();
    > + spin_unlock(&watchdog_output_lock);
    >
    > if (softlockup_all_cpu_backtrace) {
    > trigger_allbutcpu_cpu_backtrace(smp_processor_id());

    \
     
     \ /
      Last update: 2023-11-22 23:06    [W:2.784 / U:0.884 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site