[PATCH] softlockup: ensure the task has been switched out once

[PATCH] softlockup: ensure the task has been switched out once

[Frederic Weisbecker]

When we check if a task has been switched out since the last scan, we might
have a race condition on the following scenario:

_ 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.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/hung_task.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index 0c924de..022a492 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -72,7 +72,13 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout)
 {
 	unsigned long switch_count = t->nvcsw + t->nivcsw;
 
-	if (t->flags & PF_FROZEN)
+	/*
+	 * Ensure the task is not frozen.
+	 * Also, 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.
+	 */
+	if (unlikely(t->flags & PF_FROZEN || !switch_count))
 		return;
 
 	if (switch_count != t->last_switch_count) {
-- 
1.6.1

Re: [PATCH] softlockup: ensure the task has been switched out once

[Mandeep Singh Baines]

Frederic Weisbecker (fweisbec@gmail.com) wrote:
> When we check if a task has been switched out since the last scan, we might
> have a race condition on the following scenario:
> 
> _ 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.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> ---
>  kernel/hung_task.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index 0c924de..022a492 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -72,7 +72,13 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout)
>  {
>  	unsigned long switch_count = t->nvcsw + t->nivcsw;
>  
> -	if (t->flags & PF_FROZEN)
> +	/*
> +	 * Ensure the task is not frozen.
> +	 * Also, 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.
> +	 */
> +	if (unlikely(t->flags & PF_FROZEN || !switch_count))
>  		return;
>  
>  	if (switch_count != t->last_switch_count) {
> -- 
> 1.6.1
> 
> 


Looks good to me. Thanks for fixing this!

Acked-by: Mandeep Singh Baines <msb@google.com>

Re: [PATCH] softlockup: ensure the task has been switched out once

[Ingo Molnar]

* Mandeep Singh Baines <msb@google.com> wrote:

> Frederic Weisbecker (fweisbec@gmail.com) wrote:
> > When we check if a task has been switched out since the last scan, we might
> > have a race condition on the following scenario:
> > 
> > _ 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.
> > 
> > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> > ---
> >  kernel/hung_task.c |    8 +++++++-
> >  1 files changed, 7 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> > index 0c924de..022a492 100644
> > --- a/kernel/hung_task.c
> > +++ b/kernel/hung_task.c
> > @@ -72,7 +72,13 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout)
> >  {
> >  	unsigned long switch_count = t->nvcsw + t->nivcsw;
> >  
> > -	if (t->flags & PF_FROZEN)
> > +	/*
> > +	 * Ensure the task is not frozen.
> > +	 * Also, 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.
> > +	 */
> > +	if (unlikely(t->flags & PF_FROZEN || !switch_count))
> >  		return;
> >  
> >  	if (switch_count != t->last_switch_count) {
> > -- 
> > 1.6.1
> > 
> > 
> 
> 
> Looks good to me. Thanks for fixing this!
> 
> Acked-by: Mandeep Singh Baines <msb@google.com>

Applied to tip:core/softlockup, thanks guys!

	Ingo