linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: offlining memory may block forever
@ 2012-06-20 16:23 Aaditya Kumar
  2012-06-20 17:13 ` Greg KH
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Aaditya Kumar @ 2012-06-20 16:23 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: linux-kernel, linux-mm, stable, kosaki.motohiro, gregkh,
	Mel Gorman, KAMEZAWA Hiroyuki, Minchan Kim, Michal Hocko,
	tim.bird, frank.rowand, takuzo.ohara, kan.iibuchi, aaditya.kumar

Offlining memory may block forever, waiting for kswapd() to wake up because
kswapd() does not check the event kthread->should_stop before sleeping.

The proper pattern, from Documentation/memory-barriers.txt, is:
   ---  waker  ---
   event_indicated = 1;
   wake_up_process(event_daemon);

   ---  sleeper  ---
   for (;;) {
      set_current_state(TASK_UNINTERRUPTIBLE);
      if (event_indicated)
         break;
      schedule();
   }

   set_current_state() may be wrapped by:
      prepare_to_wait();

In the kswapd() case, event_indicated is kthread->should_stop.
---  offlining memory (waker)  ---
   kswapd_stop()
      kthread_stop()
         kthread->should_stop = 1
         wake_up_process()
         wait_for_completion()


---  kswapd_try_to_sleep (sleeper)  ---
   kswapd_try_to_sleep()
      prepare_to_wait()
           .
           .
      schedule()
           .
           .
      finish_wait()

   The schedule() needs to be protected by a test of kthread->should_stop,
   which is wrapped by kthread_should_stop().

Reproducer:
   Do heavy file I/O in background.
   Do a memory offline/online in a tight loop


Signed-off-by: Aaditya Kumar <aaditya.kumar@ap.sony.com>

---
diff --git a/mm/vmscan.c b/mm/vmscan.c
index eeb3bc9..b60691e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2688,7 +2688,10 @@ static void kswapd_try_to_sleep(pg_data_t
*pgdat, int order, int classzone_idx)
 		 * them before going back to sleep.
 		 */
 		set_pgdat_percpu_threshold(pgdat, calculate_normal_threshold);
-		schedule();
+
+		if (!kthread_should_stop())
+			schedule();
+
 		set_pgdat_percpu_threshold(pgdat, calculate_pressure_threshold);
 	} else {
 		if (remaining)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm: offlining memory may block forever
  2012-06-20 16:23 [PATCH] mm: offlining memory may block forever Aaditya Kumar
@ 2012-06-20 17:13 ` Greg KH
  2012-06-20 18:49 ` KOSAKI Motohiro
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2012-06-20 17:13 UTC (permalink / raw)
  To: Aaditya Kumar
  Cc: KOSAKI Motohiro, linux-kernel, linux-mm, stable, kosaki.motohiro,
	Mel Gorman, KAMEZAWA Hiroyuki, Minchan Kim, Michal Hocko,
	tim.bird, frank.rowand, takuzo.ohara, kan.iibuchi, aaditya.kumar

On Wed, Jun 20, 2012 at 09:53:31PM +0530, Aaditya Kumar wrote:
> Offlining memory may block forever, waiting for kswapd() to wake up because
> kswapd() does not check the event kthread->should_stop before sleeping.
> 
> The proper pattern, from Documentation/memory-barriers.txt, is:
>    ---  waker  ---
>    event_indicated = 1;
>    wake_up_process(event_daemon);
> 
>    ---  sleeper  ---
>    for (;;) {
>       set_current_state(TASK_UNINTERRUPTIBLE);
>       if (event_indicated)
>          break;
>       schedule();
>    }
> 
>    set_current_state() may be wrapped by:
>       prepare_to_wait();
> 
> In the kswapd() case, event_indicated is kthread->should_stop.
> ---  offlining memory (waker)  ---
>    kswapd_stop()
>       kthread_stop()
>          kthread->should_stop = 1
>          wake_up_process()
>          wait_for_completion()
> 
> 
> ---  kswapd_try_to_sleep (sleeper)  ---
>    kswapd_try_to_sleep()
>       prepare_to_wait()
>            .
>            .
>       schedule()
>            .
>            .
>       finish_wait()
> 
>    The schedule() needs to be protected by a test of kthread->should_stop,
>    which is wrapped by kthread_should_stop().
> 
> Reproducer:
>    Do heavy file I/O in background.
>    Do a memory offline/online in a tight loop
> 
> 
> Signed-off-by: Aaditya Kumar <aaditya.kumar@ap.sony.com>

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm: offlining memory may block forever
  2012-06-20 16:23 [PATCH] mm: offlining memory may block forever Aaditya Kumar
  2012-06-20 17:13 ` Greg KH
@ 2012-06-20 18:49 ` KOSAKI Motohiro
  2012-06-21  1:37 ` Minchan Kim
  2012-06-21 13:04 ` Mel Gorman
  3 siblings, 0 replies; 5+ messages in thread
From: KOSAKI Motohiro @ 2012-06-20 18:49 UTC (permalink / raw)
  To: aaditya.kumar.30
  Cc: kosaki.motohiro, linux-kernel, linux-mm, stable, kosaki.motohiro,
	gregkh, mel, kamezawa.hiroyu, minchan, mhocko, tim.bird,
	frank.rowand, takuzo.ohara, kan.iibuchi, aaditya.kumar

On 6/20/2012 12:23 PM, Aaditya Kumar wrote:
> Offlining memory may block forever, waiting for kswapd() to wake up because
> kswapd() does not check the event kthread->should_stop before sleeping.
> 
> The proper pattern, from Documentation/memory-barriers.txt, is:
>    ---  waker  ---
>    event_indicated = 1;
>    wake_up_process(event_daemon);
> 
>    ---  sleeper  ---
>    for (;;) {
>       set_current_state(TASK_UNINTERRUPTIBLE);
>       if (event_indicated)
>          break;
>       schedule();
>    }
> 
>    set_current_state() may be wrapped by:
>       prepare_to_wait();
> 
> In the kswapd() case, event_indicated is kthread->should_stop.
> ---  offlining memory (waker)  ---

Please avoid "---". This is used for a separator between a patch
description and code.

Other than that,
 Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>


>    kswapd_stop()
>       kthread_stop()
>          kthread->should_stop = 1
>          wake_up_process()
>          wait_for_completion()
> 
> 
> ---  kswapd_try_to_sleep (sleeper)  ---
>    kswapd_try_to_sleep()
>       prepare_to_wait()
>            .
>            .
>       schedule()
>            .
>            .
>       finish_wait()
> 
>    The schedule() needs to be protected by a test of kthread->should_stop,
>    which is wrapped by kthread_should_stop().
> 
> Reproducer:
>    Do heavy file I/O in background.
>    Do a memory offline/online in a tight loop
> 
> 
> Signed-off-by: Aaditya Kumar <aaditya.kumar@ap.sony.com>
> 
> ---
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index eeb3bc9..b60691e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2688,7 +2688,10 @@ static void kswapd_try_to_sleep(pg_data_t
> *pgdat, int order, int classzone_idx)
>  		 * them before going back to sleep.
>  		 */
>  		set_pgdat_percpu_threshold(pgdat, calculate_normal_threshold);
> -		schedule();
> +
> +		if (!kthread_should_stop())
> +			schedule();
> +
>  		set_pgdat_percpu_threshold(pgdat, calculate_pressure_threshold);
>  	} else {
>  		if (remaining)
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm: offlining memory may block forever
  2012-06-20 16:23 [PATCH] mm: offlining memory may block forever Aaditya Kumar
  2012-06-20 17:13 ` Greg KH
  2012-06-20 18:49 ` KOSAKI Motohiro
@ 2012-06-21  1:37 ` Minchan Kim
  2012-06-21 13:04 ` Mel Gorman
  3 siblings, 0 replies; 5+ messages in thread
From: Minchan Kim @ 2012-06-21  1:37 UTC (permalink / raw)
  To: Aaditya Kumar
  Cc: KOSAKI Motohiro, linux-kernel, linux-mm, stable, kosaki.motohiro,
	gregkh, Mel Gorman, KAMEZAWA Hiroyuki, Michal Hocko, tim.bird,
	frank.rowand, takuzo.ohara, kan.iibuchi, aaditya.kumar

On 06/21/2012 01:23 AM, Aaditya Kumar wrote:

> Offlining memory may block forever, waiting for kswapd() to wake up because
> kswapd() does not check the event kthread->should_stop before sleeping.

> 

> The proper pattern, from Documentation/memory-barriers.txt, is:
>    ---  waker  ---
>    event_indicated = 1;
>    wake_up_process(event_daemon);
> 
>    ---  sleeper  ---
>    for (;;) {
>       set_current_state(TASK_UNINTERRUPTIBLE);
>       if (event_indicated)
>          break;
>       schedule();
>    }
> 
>    set_current_state() may be wrapped by:
>       prepare_to_wait();
> 
> In the kswapd() case, event_indicated is kthread->should_stop.
> ---  offlining memory (waker)  ---
>    kswapd_stop()
>       kthread_stop()
>          kthread->should_stop = 1
>          wake_up_process()
>          wait_for_completion()
> 
> 
> ---  kswapd_try_to_sleep (sleeper)  ---
>    kswapd_try_to_sleep()
>       prepare_to_wait()
>            .
>            .
>       schedule()
>            .
>            .
>       finish_wait()
> 
>    The schedule() needs to be protected by a test of kthread->should_stop,
>    which is wrapped by kthread_should_stop().
> 
> Reproducer:
>    Do heavy file I/O in background.
>    Do a memory offline/online in a tight loop
> 
> 
> Signed-off-by: Aaditya Kumar <aaditya.kumar@ap.sony.com>

Reviewed-by: Minchan Kim <minchan@kernel.org>

Nitpick: We can remove kthread_should_stop check earlier in kswapd_try_to_sleep.
         But it's no biggie. And I hope you change patch title 
	 
	 Title : Fix loss of kswapd wakeup in kswapd_stop
	 Description: Offlining memory may block forever because blah, blah, blah.

-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH] mm: offlining memory may block forever
  2012-06-20 16:23 [PATCH] mm: offlining memory may block forever Aaditya Kumar
                   ` (2 preceding siblings ...)
  2012-06-21  1:37 ` Minchan Kim
@ 2012-06-21 13:04 ` Mel Gorman
  3 siblings, 0 replies; 5+ messages in thread
From: Mel Gorman @ 2012-06-21 13:04 UTC (permalink / raw)
  To: Aaditya Kumar
  Cc: KOSAKI Motohiro, linux-kernel, linux-mm, stable, kosaki.motohiro,
	gregkh, KAMEZAWA Hiroyuki, Minchan Kim, Michal Hocko, tim.bird,
	frank.rowand, takuzo.ohara, kan.iibuchi, aaditya.kumar

On Wed, Jun 20, 2012 at 09:53:31PM +0530, Aaditya Kumar wrote:
> Offlining memory may block forever, waiting for kswapd() to wake up because
> kswapd() does not check the event kthread->should_stop before sleeping.
> 

Acked-by: Mel Gorman <mel@csn.ul.ie>

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

end of thread, other threads:[~2012-06-21 13:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-20 16:23 [PATCH] mm: offlining memory may block forever Aaditya Kumar
2012-06-20 17:13 ` Greg KH
2012-06-20 18:49 ` KOSAKI Motohiro
2012-06-21  1:37 ` Minchan Kim
2012-06-21 13:04 ` Mel Gorman

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