Re: [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task() comment

[Jun Miao]

On 8/6/21 10:27 PM, Lukas Bulwahn wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Fri, Aug 6, 2021 at 1:41 PM Jun Miao <jun.miao@xxxxxxxxxxxxx> wrote:
> > It's "mustn't", not "musn't". Let's fix that.
> >
> > Signed-off-by: Jun Miao <jun.miao@xxxxxxxxxxxxx>
> > ---
> >   kernel/hung_task.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> > index 9888e2bc8c76..ea5ba912db06 100644
> > --- a/kernel/hung_task.c
> > +++ b/kernel/hung_task.c
> > @@ -99,7 +99,7 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout)
> >          /*
> >           * When a freshly created task is scheduled once, changes its state to
> >           * TASK_UNINTERRUPTIBLE without having ever been switched out once, it
> > -        * musn't be checked.
> > +        * mustn't be checked.
> I cannot even parse this comment.
>
> Does "When a freshly created task is scheduled once, changes its state
> to TASK_UNINTERRUPTIBLE" mean "When a freshly created task is
> scheduled once and it changes its state to TASK_UNINTERRUPTIBLE"?
>
> Does this "it must not be checked" read as "it shall not be checked"
> (as in "because if you check it, something goes wrong") or "it is not
> required to be checked" (as in "usually, you need to check it
> (otherwise something goes wrong), but here in this case, you do not
> need to check it, because it cannot go wrong in this case")?
>
> Fixing spelling mistakes is okay, but it is even better to check the
> sentence you are correcting and try to comprehend it.

Hi Lukas, thanks for your advice from my heart.

From the context of code:
---
 90         unsigned long switch_count = t->nvcsw + t->nivcsw;
 91
 ... ...
 99         /*
100          * When a freshly created task is scheduled once, changes 
its state to
101          * TASK_UNINTERRUPTIBLE without having ever been switched 
out once, it
102          * mustn't be checked.
103          */
104         if (unlikely(!switch_count))
105                 return;
---

It should read as "it shall not be checked" (as in "because if you check 
it, something goes wrong")
. When create a task and set it as "TASK_UNINTERRUPTIBLE" we don`t need 
to check "swtich_count=0".
If check will report a false positive.

From a history commit cf2592f59c0e, there is a detail explain:
    - the task is freshly created and scheduled
    - it puts its state to TASK_UNINTERRUPTIBLE and is not yet switched out
    - check_hung_task() scans this task and will report a false 
positive because
      t->nvcsw + t->nivcsw == t->last_switch_count == 0

    Add a check for such cases.

Thanks
Jun
> Lukas