linux-kernel - Re: [PATCH] kernel/hung_task.c: Fix a typo in check_hung

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <facd857b-6fb1-75c0-0b81-39176fff3e7a@windriver.com>
Date:   Wed, 11 Aug 2021 16:32:31 +0800
From:   Jun Miao <jun.miao@...driver.com>
To:     unlisted-recipients:; (no To-header on input)
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        deller@....de, wei.liu@...nel.org,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task()
 comment

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@...driver.com> wrote:
>>> It's "mustn't", not "musn't". Let's fix that.
>>>
>>> Signed-off-by: Jun Miao <jun.miao@...driver.com>
>>> ---
>>>   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