Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230731091754.1.I501ab68cb926ee33a7c87e063d207abf09b9943c@changeid>
 <ZMkBY7K3Dn04YQ65@dhcp22.suse.cz> <CAD=FV=V5hx7Zy-XMB=sPYcD_h-iP5VknmEoJwvw3Akd_1wDnRw@mail.gmail.com>
 <ZMkkNpYcaYPAMj0Z@dhcp22.suse.cz> <CAD=FV=Ujmyq-1GAvNJsrp=mj_Vg=9b6fmfMfkHq3+8ZQ5KiaRw@mail.gmail.com>
 <ZMoFWK0uGdneJYVc@dhcp22.suse.cz> <CAD=FV=XQMH8sun7XCXJNjOC7tP1yt8=mt1NG3f8Xm9-x5TJFsA@mail.gmail.com>
 <ZMthXBpLzbbysTe5@alley> <ZMtliXUFGptYKEra@dhcp22.suse.cz>
In-Reply-To: <ZMtliXUFGptYKEra@dhcp22.suse.cz>
From:   Doug Anderson <dianders@chromium.org>
Date:   Thu, 3 Aug 2023 16:10:24 -0700
Message-ID: <CAD=FV=UFwBO-haMTFyZZbPqf-B0HHQ_aPqiaoVXPK-cQX3pnUg@mail.gmail.com>
Subject: Re: [PATCH] watchdog/hardlockup: Avoid large stack frames in watchdog_hardlockup_check()
To:     Michal Hocko <mhocko@suse.com>
Cc:     Petr Mladek <pmladek@suse.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        kernel test robot <lkp@intel.com>,
        Lecopzer Chen <lecopzer.chen@mediatek.com>,
        Pingfan Liu <kernelfans@gmail.com>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

On Thu, Aug 3, 2023 at 1:30=E2=80=AFAM Michal Hocko <mhocko@suse.com> wrote=
:
>
> On Thu 03-08-23 10:12:12, Petr Mladek wrote:
> > On Wed 2023-08-02 07:12:29, Doug Anderson wrote:
> > > Hi,
> > >
> > > On Wed, Aug 2, 2023 at 12:27=E2=80=AFAM Michal Hocko <mhocko@suse.com=
> wrote:
> > > >
> > > > On Tue 01-08-23 08:41:49, Doug Anderson wrote:
> > > > [...]
> > > > > Ah, I see what you mean. The one issue I have with your solution =
is
> > > > > that the ordering of the stack crawls is less ideal in the "dump =
all"
> > > > > case when cpu !=3D this_cpu. We really want to see the stack craw=
l of
> > > > > the locked up CPU first and _then_ see the stack crawls of other =
CPUs.
> > > > > With your solution the locked up CPU will be interspersed with al=
l the
> > > > > others and will be harder to find in the output (you've got to ma=
tch
> > > > > it up with the "Watchdog detected hard LOCKUP on cpu N" message).
> > > > > While that's probably not a huge deal, it's nicer to make the out=
put
> > > > > easy to understand for someone trying to parse it...
> > > >
> > > > Is it worth to waste memory for this arguably nicer output? Identif=
ying
> > > > the stack of the locked up CPU is trivial.
> > >
> > > I guess it's debatable, but as someone who has spent time staring at
> > > trawling through reports generated like this, I'd say "yes", it's
> > > super helpful in understanding the problem to have the hung CPU first=
.
> > > Putting the memory usage in perspective:
> >
> > nmi_trigger_cpumask_backtrace() has its own copy of the cpu mask.
> > What about changing the @exclude_self parameter to @exclude_cpu
> > and do:
> >
> >       if (exclude_cpu >=3D 0)
> >               cpumask_clear_cpu(exclude_cpu, to_cpumask(backtrace_mask)=
);
> >
> >
> > It would require changing also arch_trigger_cpumask_backtrace() to
> >
> >       void arch_trigger_cpumask_backtrace(const struct cpumask *mask,
> >                                   int exclude_cpu);
> >
> > but it looks doable.
>
> Yes, but sparc is doing its own thing so it would require changing that
> as well. But this looks reasonable as well.

OK. I've tried a v3 with that:

https://lore.kernel.org/r/20230803160649.v3.2.I501ab68cb926ee33a7c87e063d20=
7abf09b9943c@changeid

-Doug