* [PATCH] thp: do not adjust zone water marks if khugepaged is not started
@ 2015-03-27 11:39 Kirill A. Shutemov
2015-03-27 21:47 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2015-03-27 11:39 UTC (permalink / raw)
To: Andrew Morton, David Rientjes
Cc: Andrea Arcangeli, linux-mm, Kirill A. Shutemov
set_recommended_min_free_kbytes() adjusts zone water marks to be suitable
for khugepaged. We avoid doing this if khugepaged is disabled, but don't
catch the case when khugepaged is failed to start.
Let's address this by checking khugepaged_thread instead of
khugepaged_enabled() in set_recommended_min_free_kbytes().
It's NULL if the kernel thread is stopped or failed to start.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
mm/huge_memory.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index a1594b18bc1b..370a3bbc960d 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -110,7 +110,8 @@ static int set_recommended_min_free_kbytes(void)
int nr_zones = 0;
unsigned long recommended_min;
- if (!khugepaged_enabled())
+ /* khugepaged thread has stopped to failed to start */
+ if (!khugepaged_thread)
return 0;
for_each_populated_zone(zone)
--
2.1.4
--
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] 6+ messages in thread
* Re: [PATCH] thp: do not adjust zone water marks if khugepaged is not started
2015-03-27 11:39 [PATCH] thp: do not adjust zone water marks if khugepaged is not started Kirill A. Shutemov
@ 2015-03-27 21:47 ` Andrew Morton
2015-03-27 22:00 ` Andrew Morton
2015-03-27 22:00 ` Kirill A. Shutemov
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2015-03-27 21:47 UTC (permalink / raw)
To: Kirill A. Shutemov; +Cc: David Rientjes, Andrea Arcangeli, linux-mm
On Fri, 27 Mar 2015 13:39:38 +0200 "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote:
> set_recommended_min_free_kbytes() adjusts zone water marks to be suitable
> for khugepaged. We avoid doing this if khugepaged is disabled, but don't
> catch the case when khugepaged is failed to start.
>
> Let's address this by checking khugepaged_thread instead of
> khugepaged_enabled() in set_recommended_min_free_kbytes().
> It's NULL if the kernel thread is stopped or failed to start.
>
hm, why didn't khugepaged start up? Is this a theoretical
by-code-inspection thing or has the problem been observed in real life?
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -110,7 +110,8 @@ static int set_recommended_min_free_kbytes(void)
> int nr_zones = 0;
> unsigned long recommended_min;
>
> - if (!khugepaged_enabled())
> + /* khugepaged thread has stopped to failed to start */
> + if (!khugepaged_thread)
> return 0;
>
> for_each_populated_zone(zone)
Fair enough, but take a look at start_khugepaged():
: static int start_khugepaged(void)
: {
: int err = 0;
: if (khugepaged_enabled()) {
: if (!khugepaged_thread)
: khugepaged_thread = kthread_run(khugepaged, NULL,
: "khugepaged");
: if (unlikely(IS_ERR(khugepaged_thread))) {
: pr_err("khugepaged: kthread_run(khugepaged) failed\n");
: err = PTR_ERR(khugepaged_thread);
: khugepaged_thread = NULL;
-->> stop here
: }
:
: if (!list_empty(&khugepaged_scan.mm_head))
: wake_up_interruptible(&khugepaged_wait);
:
: set_recommended_min_free_kbytes();
: } else if (khugepaged_thread) {
: kthread_stop(khugepaged_thread);
: khugepaged_thread = NULL;
: }
:
: return err;
: }
--
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] 6+ messages in thread
* Re: [PATCH] thp: do not adjust zone water marks if khugepaged is not started
2015-03-27 21:47 ` Andrew Morton
@ 2015-03-27 22:00 ` Andrew Morton
2015-03-27 22:08 ` Kirill A. Shutemov
2015-04-02 12:08 ` Kirill A. Shutemov
2015-03-27 22:00 ` Kirill A. Shutemov
1 sibling, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2015-03-27 22:00 UTC (permalink / raw)
To: Kirill A. Shutemov, David Rientjes, Andrea Arcangeli, linux-mm
On Fri, 27 Mar 2015 14:47:08 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> Fair enough, but take a look at start_khugepaged():
>
> : static int start_khugepaged(void)
> : {
> : int err = 0;
> : if (khugepaged_enabled()) {
> : if (!khugepaged_thread)
> : khugepaged_thread = kthread_run(khugepaged, NULL,
> : "khugepaged");
> : if (unlikely(IS_ERR(khugepaged_thread))) {
> : pr_err("khugepaged: kthread_run(khugepaged) failed\n");
> : err = PTR_ERR(khugepaged_thread);
> : khugepaged_thread = NULL;
>
> -->> stop here
>
> : }
> :
> : if (!list_empty(&khugepaged_scan.mm_head))
> : wake_up_interruptible(&khugepaged_wait);
> :
> : set_recommended_min_free_kbytes();
> : } else if (khugepaged_thread) {
> : kthread_stop(khugepaged_thread);
> : khugepaged_thread = NULL;
> : }
> :
> : return err;
> : }
Looking more closely... This code seems a bit screwy.
- why is set_recommended_min_free_kbytes() a late_initcall? We've
already done that within
subsys_initcall->hugepage_init->set_recommended_min_free_kbytes()
- there isn't much point in running start_khugepaged() if we've just
set transparent_hugepage_flags to zero.
- start_khugepaged() is misnamed.
So something like this?
--- a/mm/huge_memory.c~a
+++ a/mm/huge_memory.c
@@ -110,10 +110,6 @@ static int set_recommended_min_free_kbyt
int nr_zones = 0;
unsigned long recommended_min;
- /* khugepaged thread has stopped to failed to start */
- if (!khugepaged_thread)
- return 0;
-
for_each_populated_zone(zone)
nr_zones++;
@@ -145,9 +141,8 @@ static int set_recommended_min_free_kbyt
setup_per_zone_wmarks();
return 0;
}
-late_initcall(set_recommended_min_free_kbytes);
-static int start_khugepaged(void)
+static int start_stop_khugepaged(void)
{
int err = 0;
if (khugepaged_enabled()) {
@@ -158,6 +153,7 @@ static int start_khugepaged(void)
pr_err("khugepaged: kthread_run(khugepaged) failed\n");
err = PTR_ERR(khugepaged_thread);
khugepaged_thread = NULL;
+ goto fail;
}
if (!list_empty(&khugepaged_scan.mm_head))
@@ -168,7 +164,7 @@ static int start_khugepaged(void)
kthread_stop(khugepaged_thread);
khugepaged_thread = NULL;
}
-
+fail:
return err;
}
@@ -302,7 +298,7 @@ static ssize_t enabled_store(struct kobj
int err;
mutex_lock(&khugepaged_mutex);
- err = start_khugepaged();
+ err = start_stop_khugepaged();
mutex_unlock(&khugepaged_mutex);
if (err)
@@ -651,12 +647,13 @@ static int __init hugepage_init(void)
* where the extra memory used could hurt more than TLB overhead
* is likely to save. The admin can still enable it through /sys.
*/
- if (totalram_pages < (512 << (20 - PAGE_SHIFT)))
+ if (totalram_pages < (512 << (20 - PAGE_SHIFT))) {
transparent_hugepage_flags = 0;
-
- err = start_khugepaged();
- if (err)
- goto err_khugepaged;
+ } else {
+ err = start_stop_khugepaged();
+ if (err)
+ goto err_khugepaged;
+ }
return 0;
err_khugepaged:
_
--
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] 6+ messages in thread
* Re: [PATCH] thp: do not adjust zone water marks if khugepaged is not started
2015-03-27 21:47 ` Andrew Morton
2015-03-27 22:00 ` Andrew Morton
@ 2015-03-27 22:00 ` Kirill A. Shutemov
1 sibling, 0 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2015-03-27 22:00 UTC (permalink / raw)
To: Andrew Morton, Andrea Arcangeli
Cc: Kirill A. Shutemov, David Rientjes, linux-mm
On Fri, Mar 27, 2015 at 02:47:08PM -0700, Andrew Morton wrote:
> On Fri, 27 Mar 2015 13:39:38 +0200 "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote:
>
> > set_recommended_min_free_kbytes() adjusts zone water marks to be suitable
> > for khugepaged. We avoid doing this if khugepaged is disabled, but don't
> > catch the case when khugepaged is failed to start.
> >
> > Let's address this by checking khugepaged_thread instead of
> > khugepaged_enabled() in set_recommended_min_free_kbytes().
> > It's NULL if the kernel thread is stopped or failed to start.
> >
>
> hm, why didn't khugepaged start up? Is this a theoretical
> by-code-inspection thing or has the problem been observed in real life?
David mentioned this scenario in comment to my previous patch.
>
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -110,7 +110,8 @@ static int set_recommended_min_free_kbytes(void)
> > int nr_zones = 0;
> > unsigned long recommended_min;
> >
> > - if (!khugepaged_enabled())
> > + /* khugepaged thread has stopped to failed to start */
> > + if (!khugepaged_thread)
> > return 0;
> >
> > for_each_populated_zone(zone)
>
> Fair enough, but take a look at start_khugepaged():
>
> : static int start_khugepaged(void)
> : {
> : int err = 0;
> : if (khugepaged_enabled()) {
> : if (!khugepaged_thread)
> : khugepaged_thread = kthread_run(khugepaged, NULL,
> : "khugepaged");
> : if (unlikely(IS_ERR(khugepaged_thread))) {
> : pr_err("khugepaged: kthread_run(khugepaged) failed\n");
> : err = PTR_ERR(khugepaged_thread);
> : khugepaged_thread = NULL;
>
> -->> stop here
Right, but set_recommended_min_free_kbytes() is also registered to
late_initcall() and will get called anyway.
It's not obvious why would we need it registered there. Call from
start_khugepaged() should be enough.
Andrea?
> : }
> :
> : if (!list_empty(&khugepaged_scan.mm_head))
> : wake_up_interruptible(&khugepaged_wait);
> :
> : set_recommended_min_free_kbytes();
> : } else if (khugepaged_thread) {
> : kthread_stop(khugepaged_thread);
> : khugepaged_thread = NULL;
> : }
> :
> : return err;
> : }
>
> --
> 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>
--
Kirill A. Shutemov
--
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] 6+ messages in thread
* Re: [PATCH] thp: do not adjust zone water marks if khugepaged is not started
2015-03-27 22:00 ` Andrew Morton
@ 2015-03-27 22:08 ` Kirill A. Shutemov
2015-04-02 12:08 ` Kirill A. Shutemov
1 sibling, 0 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2015-03-27 22:08 UTC (permalink / raw)
To: Andrew Morton
Cc: Kirill A. Shutemov, David Rientjes, Andrea Arcangeli, linux-mm
On Fri, Mar 27, 2015 at 03:00:26PM -0700, Andrew Morton wrote:
> On Fri, 27 Mar 2015 14:47:08 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> > Fair enough, but take a look at start_khugepaged():
> >
> > : static int start_khugepaged(void)
> > : {
> > : int err = 0;
> > : if (khugepaged_enabled()) {
> > : if (!khugepaged_thread)
> > : khugepaged_thread = kthread_run(khugepaged, NULL,
> > : "khugepaged");
> > : if (unlikely(IS_ERR(khugepaged_thread))) {
> > : pr_err("khugepaged: kthread_run(khugepaged) failed\n");
> > : err = PTR_ERR(khugepaged_thread);
> > : khugepaged_thread = NULL;
> >
> > -->> stop here
> >
> > : }
> > :
> > : if (!list_empty(&khugepaged_scan.mm_head))
> > : wake_up_interruptible(&khugepaged_wait);
> > :
> > : set_recommended_min_free_kbytes();
> > : } else if (khugepaged_thread) {
> > : kthread_stop(khugepaged_thread);
> > : khugepaged_thread = NULL;
> > : }
> > :
> > : return err;
> > : }
>
> Looking more closely... This code seems a bit screwy.
>
> - why is set_recommended_min_free_kbytes() a late_initcall? We've
> already done that within
> subsys_initcall->hugepage_init->set_recommended_min_free_kbytes()
>
> - there isn't much point in running start_khugepaged() if we've just
> set transparent_hugepage_flags to zero.
>
> - start_khugepaged() is misnamed.
>
> So something like this?
Yeah, looks good to me.
> --- a/mm/huge_memory.c~a
> +++ a/mm/huge_memory.c
> @@ -110,10 +110,6 @@ static int set_recommended_min_free_kbyt
> int nr_zones = 0;
> unsigned long recommended_min;
>
> - /* khugepaged thread has stopped to failed to start */
> - if (!khugepaged_thread)
> - return 0;
> -
> for_each_populated_zone(zone)
> nr_zones++;
>
> @@ -145,9 +141,8 @@ static int set_recommended_min_free_kbyt
> setup_per_zone_wmarks();
> return 0;
> }
> -late_initcall(set_recommended_min_free_kbytes);
>
> -static int start_khugepaged(void)
> +static int start_stop_khugepaged(void)
> {
> int err = 0;
> if (khugepaged_enabled()) {
> @@ -158,6 +153,7 @@ static int start_khugepaged(void)
> pr_err("khugepaged: kthread_run(khugepaged) failed\n");
> err = PTR_ERR(khugepaged_thread);
> khugepaged_thread = NULL;
> + goto fail;
> }
>
> if (!list_empty(&khugepaged_scan.mm_head))
> @@ -168,7 +164,7 @@ static int start_khugepaged(void)
> kthread_stop(khugepaged_thread);
> khugepaged_thread = NULL;
> }
> -
> +fail:
> return err;
> }
>
> @@ -302,7 +298,7 @@ static ssize_t enabled_store(struct kobj
> int err;
>
> mutex_lock(&khugepaged_mutex);
> - err = start_khugepaged();
> + err = start_stop_khugepaged();
> mutex_unlock(&khugepaged_mutex);
>
> if (err)
> @@ -651,12 +647,13 @@ static int __init hugepage_init(void)
> * where the extra memory used could hurt more than TLB overhead
> * is likely to save. The admin can still enable it through /sys.
> */
> - if (totalram_pages < (512 << (20 - PAGE_SHIFT)))
> + if (totalram_pages < (512 << (20 - PAGE_SHIFT))) {
> transparent_hugepage_flags = 0;
> -
> - err = start_khugepaged();
> - if (err)
> - goto err_khugepaged;
> + } else {
> + err = start_stop_khugepaged();
> + if (err)
> + goto err_khugepaged;
> + }
>
> return 0;
> err_khugepaged:
> _
>
> --
> 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>
--
Kirill A. Shutemov
--
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] 6+ messages in thread
* Re: [PATCH] thp: do not adjust zone water marks if khugepaged is not started
2015-03-27 22:00 ` Andrew Morton
2015-03-27 22:08 ` Kirill A. Shutemov
@ 2015-04-02 12:08 ` Kirill A. Shutemov
1 sibling, 0 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2015-04-02 12:08 UTC (permalink / raw)
To: Andrew Morton
Cc: Kirill A. Shutemov, David Rientjes, Andrea Arcangeli, linux-mm
On Fri, Mar 27, 2015 at 03:00:26PM -0700, Andrew Morton wrote:
> On Fri, 27 Mar 2015 14:47:08 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> > Fair enough, but take a look at start_khugepaged():
> >
> > : static int start_khugepaged(void)
> > : {
> > : int err = 0;
> > : if (khugepaged_enabled()) {
> > : if (!khugepaged_thread)
> > : khugepaged_thread = kthread_run(khugepaged, NULL,
> > : "khugepaged");
> > : if (unlikely(IS_ERR(khugepaged_thread))) {
> > : pr_err("khugepaged: kthread_run(khugepaged) failed\n");
> > : err = PTR_ERR(khugepaged_thread);
> > : khugepaged_thread = NULL;
> >
> > -->> stop here
> >
> > : }
> > :
> > : if (!list_empty(&khugepaged_scan.mm_head))
> > : wake_up_interruptible(&khugepaged_wait);
> > :
> > : set_recommended_min_free_kbytes();
> > : } else if (khugepaged_thread) {
> > : kthread_stop(khugepaged_thread);
> > : khugepaged_thread = NULL;
> > : }
> > :
> > : return err;
> > : }
>
> Looking more closely... This code seems a bit screwy.
>
> - why is set_recommended_min_free_kbytes() a late_initcall? We've
> already done that within
> subsys_initcall->hugepage_init->set_recommended_min_free_kbytes()
>
> - there isn't much point in running start_khugepaged() if we've just
> set transparent_hugepage_flags to zero.
>
> - start_khugepaged() is misnamed.
>
> So something like this?
Looks like you didn't apply this. Here's proper patch:
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-02 12:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-27 11:39 [PATCH] thp: do not adjust zone water marks if khugepaged is not started Kirill A. Shutemov
2015-03-27 21:47 ` Andrew Morton
2015-03-27 22:00 ` Andrew Morton
2015-03-27 22:08 ` Kirill A. Shutemov
2015-04-02 12:08 ` Kirill A. Shutemov
2015-03-27 22:00 ` Kirill A. Shutemov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.