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

[Jun Miao]

Hi,

    What about this spelling mistakes ?

Thanks
Jun


On 8/7/21 8:21 PM, Jun Miao wrote:
> 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