LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task() comment
@ 2021-08-06 11:40 Jun Miao
  2021-08-06 13:04 ` Daniel Bristot de Oliveira
  2021-08-06 14:27 ` Lukas Bulwahn
  0 siblings, 2 replies; 5+ messages in thread
From: Jun Miao @ 2021-08-06 11:40 UTC (permalink / raw)
  To: akpm, andriy.shevchenko, deller, wei.liu, bristot
  Cc: peterz, lukas.bulwahn, jun.miao, linux-kernel

It's "mustn't", not "musn't". Let's fix that.

Signed-off-by: Jun Miao <jun.miao@windriver.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.
 	 */
 	if (unlikely(!switch_count))
 		return;
-- 
2.32.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task() comment
  2021-08-06 11:40 [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task() comment Jun Miao
@ 2021-08-06 13:04 ` Daniel Bristot de Oliveira
  2021-08-06 14:27 ` Lukas Bulwahn
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Bristot de Oliveira @ 2021-08-06 13:04 UTC (permalink / raw)
  To: Jun Miao, akpm, andriy.shevchenko, deller, wei.liu
  Cc: peterz, lukas.bulwahn, linux-kernel

On 8/6/21 1:40 PM, Jun Miao wrote:

> It's "mustn't", not "musn't". Let's fix that.
> 
> Signed-off-by: Jun Miao <jun.miao@windriver.com>

Reviewed-by: Daniel Bristot de Oliveira <bristot@kernel.org>

-- Daniel
> ---
>  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.
>  	 */
>  	if (unlikely(!switch_count))
>  		return;
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task() comment
  2021-08-06 11:40 [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task() comment Jun Miao
  2021-08-06 13:04 ` Daniel Bristot de Oliveira
@ 2021-08-06 14:27 ` Lukas Bulwahn
  2021-08-07 12:21   ` Jun Miao
  1 sibling, 1 reply; 5+ messages in thread
From: Lukas Bulwahn @ 2021-08-06 14:27 UTC (permalink / raw)
  To: Jun Miao
  Cc: Andrew Morton, Andy Shevchenko, deller, wei.liu,
	Daniel Bristot de Oliveira, Peter Zijlstra,
	Linux Kernel Mailing List

On Fri, Aug 6, 2021 at 1:41 PM Jun Miao <jun.miao@windriver.com> wrote:
>
> It's "mustn't", not "musn't". Let's fix that.
>
> Signed-off-by: Jun Miao <jun.miao@windriver.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.

Lukas

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task() comment
  2021-08-06 14:27 ` Lukas Bulwahn
@ 2021-08-07 12:21   ` Jun Miao
  2021-08-11  8:32     ` Jun Miao
  0 siblings, 1 reply; 5+ messages in thread
From: Jun Miao @ 2021-08-07 12:21 UTC (permalink / raw)
  To: Lukas Bulwahn
  Cc: Andrew Morton, Andy Shevchenko, deller, wei.liu,
	Daniel Bristot de Oliveira, Peter Zijlstra,
	Linux Kernel Mailing List


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task() comment
  2021-08-07 12:21   ` Jun Miao
@ 2021-08-11  8:32     ` Jun Miao
  0 siblings, 0 replies; 5+ messages in thread
From: Jun Miao @ 2021-08-11  8:32 UTC (permalink / raw)
  Cc: Andrew Morton, Andy Shevchenko, deller, wei.liu,
	Daniel Bristot de Oliveira, Peter Zijlstra,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-11  8:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-06 11:40 [PATCH] kernel/hung_task.c: Fix a typo in check_hung_task() comment Jun Miao
2021-08-06 13:04 ` Daniel Bristot de Oliveira
2021-08-06 14:27 ` Lukas Bulwahn
2021-08-07 12:21   ` Jun Miao
2021-08-11  8:32     ` Jun Miao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).