[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAD=FV=VGt-M-CGBOWDc1a3g-t2mUyHkCQ1TFcgvrUbmJDyV0Mg@mail.gmail.com>
Date: Wed, 11 Sep 2024 15:35:57 -0700
From: Doug Anderson <dianders@...omium.org>
To: dianders@...omium.org, akpm@...ux-foundation.org, bristot@...hat.com,
bsegall@...gle.com, dietmar.eggemann@....com, john.ogness@...utronix.de,
juri.lelli@...hat.com, kjlx@...pleofstupid.com, linux-kernel@...r.kernel.org,
linux@...ssschuh.net, lizhe.67@...edance.com, mcgrof@...nel.org,
mgorman@...e.de, mingo@...hat.com, peterz@...radead.org, rostedt@...dmis.org,
tglx@...utronix.de, vincent.guittot@...aro.org, vschneid@...hat.com,
zyhtheonly@...il.com, zyhtheonly@...h.net
Cc: tiozhang@...iglobal.com
Subject: Re: [PATCH v2 1/1] kernel/watchdog: always restore
watchdog_softlockup(,hardlockup)_user_enabled after proc show
Hi,
On Fri, Sep 6, 2024 at 2:47 AM Tio Zhang <tiozhang@...iglobal.com> wrote:
>
> Otherwise when watchdog_enabled becomes 0,
> watchdog_softlockup(,hardlockup)_user_enabled will changes to 0 after
> proc show.
>
> Steps to reproduce:
>
> step 1:
> # cat /proc/sys/kernel/*watchdog
> 1
> 1
> 1
>
> | name | value
> |----------------------------------|--------------------------
> | watchdog_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_hardlockup_user_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_softlockup_user_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_user_enabled | 1
> |----------------------------------|--------------------------
>
> step 2:
> # echo 0 > /proc/sys/kernel/watchdog
>
> | name | value
> |----------------------------------|--------------------------
> | watchdog_enabled | 0
> |----------------------------------|--------------------------
> | watchdog_hardlockup_user_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_softlockup_user_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_user_enabled | 0
> |----------------------------------|--------------------------
>
> step 3:
> # cat /proc/sys/kernel/*watchdog
> 0
> 0
> 0
>
> | name | value
> |----------------------------------|--------------------------
> | watchdog_enabled | 0
> |----------------------------------|--------------------------
> | watchdog_hardlockup_user_enabled | 0
> |----------------------------------|--------------------------
> | watchdog_softlockup_user_enabled | 0
> |----------------------------------|--------------------------
> | watchdog_user_enabled | 0
> |----------------------------------|--------------------------
>
> step 4:
> # echo 1 > /proc/sys/kernel/watchdog
>
> | name | value
> |----------------------------------|--------------------------
> | watchdog_enabled | 0
> |----------------------------------|--------------------------
> | watchdog_hardlockup_user_enabled | 0
> |----------------------------------|--------------------------
> | watchdog_softlockup_user_enabled | 0
> |----------------------------------|--------------------------
> | watchdog_user_enabled | 0
> |----------------------------------|--------------------------
>
> step 5:
> # cat /proc/sys/kernel/*watchdog
> 0
> 0
> 0
>
> If we dont do "step 3", do "step 4" right after "step 2", it will be
>
> | name | value
> |----------------------------------|--------------------------
> | watchdog_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_hardlockup_user_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_softlockup_user_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_user_enabled | 1
> |----------------------------------|--------------------------
>
> then everything works correctly.
>
> So this patch fix "step 3"'s value into
>
> | name | value
> |----------------------------------|--------------------------
> | watchdog_enabled | 0
> |----------------------------------|--------------------------
> | watchdog_hardlockup_user_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_softlockup_user_enabled | 1
> |----------------------------------|--------------------------
> | watchdog_user_enabled | 0
> |----------------------------------|--------------------------
>
> And still print 0 as before.
>
> Signed-off-by: Tio Zhang <tiozhang@...iglobal.com>
> ---
> kernel/watchdog.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
The description is pretty verbose but also much clearer. The patch
looks good to me now.
Reviewed-by: Douglas Anderson <dianders@...omium.org>
Powered by blists - more mailing lists