* [PATCH] mm: offlining memory may block forever
@ 2012-06-20 16:23 ` Aaditya Kumar
0 siblings, 0 replies; 10+ 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)
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] mm: offlining memory may block forever
2012-06-20 16:23 ` Aaditya Kumar
@ 2012-06-20 17:13 ` Greg KH
-1 siblings, 0 replies; 10+ 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] 10+ messages in thread* Re: [PATCH] mm: offlining memory may block forever
@ 2012-06-20 17:13 ` Greg KH
0 siblings, 0 replies; 10+ 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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm: offlining memory may block forever
2012-06-20 16:23 ` Aaditya Kumar
@ 2012-06-20 18:49 ` KOSAKI Motohiro
-1 siblings, 0 replies; 10+ 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] 10+ messages in thread* Re: [PATCH] mm: offlining memory may block forever
@ 2012-06-20 18:49 ` KOSAKI Motohiro
0 siblings, 0 replies; 10+ 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)
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm: offlining memory may block forever
2012-06-20 16:23 ` Aaditya Kumar
@ 2012-06-21 1:37 ` Minchan Kim
-1 siblings, 0 replies; 10+ 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] 10+ messages in thread* Re: [PATCH] mm: offlining memory may block forever
@ 2012-06-21 1:37 ` Minchan Kim
0 siblings, 0 replies; 10+ 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] mm: offlining memory may block forever
2012-06-20 16:23 ` Aaditya Kumar
@ 2012-06-21 13:04 ` Mel Gorman
-1 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Re: [PATCH] mm: offlining memory may block forever
@ 2012-06-21 13:04 ` Mel Gorman
0 siblings, 0 replies; 10+ 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
^ permalink raw reply [flat|nested] 10+ messages in thread