* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-09 7:32 ` Han Pingtian
0 siblings, 0 replies; 52+ messages in thread
From: Han Pingtian @ 2014-01-09 7:32 UTC (permalink / raw)
To: linux-kernel
Cc: Mel Gorman, Andrew Morton, linux-mm, Michal Hocko, Dave Hansen,
David Rientjes
On Wed, Jan 08, 2014 at 11:16:11AM +0100, Michal Hocko wrote:
> On Wed 08-01-14 16:20:01, Han Pingtian wrote:
> > On Mon, Jan 06, 2014 at 05:46:04PM +0100, Michal Hocko wrote:
> > > On Sun 05-01-14 08:35:01, Han Pingtian wrote:
> > > [...]
> > > > From f4d085a880dfae7638b33c242554efb0afc0852b Mon Sep 17 00:00:00 2001
> > > > From: Han Pingtian <hanpt@linux.vnet.ibm.com>
> > > > Date: Fri, 3 Jan 2014 11:10:49 +0800
> > > > Subject: [PATCH] mm: show message when raising min_free_kbytes in THP
> > > >
> > > > min_free_kbytes may be raised during THP's initialization. Sometimes,
> > > > this will change the value being set by user. Showing message will
> > > > clarify this confusion.
> > >
> > > I do not have anything against informing about changing value
> > > set by user but this will inform also when the default value is
> > > updated. Is this what you want? Don't you want to check against
> > > user_min_free_kbytes? (0 if not set by user)
> > >
> >
> > To use user_min_free_kbytes in mm/huge_memory.c, we need a
> >
> > extern int user_min_free_kbytes;
>
> The variable is not defined as static so you can use it outside of
> mm/page_alloc.c.
>
> > in somewhere? Where should we put it? I guess it is mm/internal.h,
> > right?
>
> I do not think this has to be globaly visible though. Why not just
> extern declaration in mm/huge_memory.c?
>
This is the new patch, please review. Thanks.
>From b8db4f157a17d6d8652cc9cff024a192c3ee0779 Mon Sep 17 00:00:00 2001
From: Han Pingtian <hanpt@linux.vnet.ibm.com>
Date: Thu, 9 Jan 2014 15:24:26 +0800
Subject: [PATCH] mm: show message when raising min_free_kbytes in THP
min_free_kbytes may be raised during THP's initialization. Sometimes,
this will change the value being set by user. Showing message will
clarify this confusion.
Only show this message when changing the value set by user according to
Michal Hocko's suggestion.
Showing the old value of min_free_kbytes according to Dave Hansen's
suggestion. This will give user the chance to restore old value of
min_free_kbytes.
Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
---
mm/huge_memory.c | 9 ++++++++-
mm/page_alloc.c | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 7de1bf8..e0e4e29 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
.mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
};
+extern int user_min_free_kbytes;
static int set_recommended_min_free_kbytes(void)
{
@@ -130,8 +131,14 @@ static int set_recommended_min_free_kbytes(void)
(unsigned long) nr_free_buffer_pages() / 20);
recommended_min <<= (PAGE_SHIFT-10);
- if (recommended_min > min_free_kbytes)
+ if (recommended_min > min_free_kbytes) {
+ if (user_min_free_kbytes >= 0)
+ pr_info("raising min_free_kbytes from %d to %lu "
+ "to help transparent hugepage allocations\n",
+ min_free_kbytes, recommended_min);
+
min_free_kbytes = recommended_min;
+ }
setup_per_zone_wmarks();
return 0;
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9ea62b2..a9dcfd8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -205,7 +205,7 @@ static char * const zone_names[MAX_NR_ZONES] = {
};
int min_free_kbytes = 1024;
-int user_min_free_kbytes;
+int user_min_free_kbytes = -1;
static unsigned long __meminitdata nr_kernel_pages;
static unsigned long __meminitdata nr_all_pages;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-09 7:32 ` Han Pingtian
@ 2014-01-09 9:02 ` Michal Hocko
-1 siblings, 0 replies; 52+ messages in thread
From: Michal Hocko @ 2014-01-09 9:02 UTC (permalink / raw)
To: linux-kernel, Mel Gorman, Andrew Morton, linux-mm, Dave Hansen,
David Rientjes
On Thu 09-01-14 15:32:59, Han Pingtian wrote:
[...]
> From b8db4f157a17d6d8652cc9cff024a192c3ee0779 Mon Sep 17 00:00:00 2001
> From: Han Pingtian <hanpt@linux.vnet.ibm.com>
> Date: Thu, 9 Jan 2014 15:24:26 +0800
> Subject: [PATCH] mm: show message when raising min_free_kbytes in THP
>
> min_free_kbytes may be raised during THP's initialization. Sometimes,
> this will change the value being set by user. Showing message will
> clarify this confusion.
>
> Only show this message when changing the value set by user according to
> Michal Hocko's suggestion.
>
> Showing the old value of min_free_kbytes according to Dave Hansen's
> suggestion. This will give user the chance to restore old value of
> min_free_kbytes.
>
> Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
Looks good to me
Reviewed-by: Michal Hocko <mhocko@suse.cz>
Thanks!
> ---
> mm/huge_memory.c | 9 ++++++++-
> mm/page_alloc.c | 2 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 7de1bf8..e0e4e29 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> };
>
> +extern int user_min_free_kbytes;
>
> static int set_recommended_min_free_kbytes(void)
> {
> @@ -130,8 +131,14 @@ static int set_recommended_min_free_kbytes(void)
> (unsigned long) nr_free_buffer_pages() / 20);
> recommended_min <<= (PAGE_SHIFT-10);
>
> - if (recommended_min > min_free_kbytes)
> + if (recommended_min > min_free_kbytes) {
> + if (user_min_free_kbytes >= 0)
> + pr_info("raising min_free_kbytes from %d to %lu "
> + "to help transparent hugepage allocations\n",
> + min_free_kbytes, recommended_min);
> +
> min_free_kbytes = recommended_min;
> + }
> setup_per_zone_wmarks();
> return 0;
> }
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 9ea62b2..a9dcfd8 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -205,7 +205,7 @@ static char * const zone_names[MAX_NR_ZONES] = {
> };
>
> int min_free_kbytes = 1024;
> -int user_min_free_kbytes;
> +int user_min_free_kbytes = -1;
>
> static unsigned long __meminitdata nr_kernel_pages;
> static unsigned long __meminitdata nr_all_pages;
> --
> 1.7.7.6
>
--
Michal Hocko
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] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-09 9:02 ` Michal Hocko
0 siblings, 0 replies; 52+ messages in thread
From: Michal Hocko @ 2014-01-09 9:02 UTC (permalink / raw)
To: linux-kernel, Mel Gorman, Andrew Morton, linux-mm, Dave Hansen,
David Rientjes
On Thu 09-01-14 15:32:59, Han Pingtian wrote:
[...]
> From b8db4f157a17d6d8652cc9cff024a192c3ee0779 Mon Sep 17 00:00:00 2001
> From: Han Pingtian <hanpt@linux.vnet.ibm.com>
> Date: Thu, 9 Jan 2014 15:24:26 +0800
> Subject: [PATCH] mm: show message when raising min_free_kbytes in THP
>
> min_free_kbytes may be raised during THP's initialization. Sometimes,
> this will change the value being set by user. Showing message will
> clarify this confusion.
>
> Only show this message when changing the value set by user according to
> Michal Hocko's suggestion.
>
> Showing the old value of min_free_kbytes according to Dave Hansen's
> suggestion. This will give user the chance to restore old value of
> min_free_kbytes.
>
> Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
Looks good to me
Reviewed-by: Michal Hocko <mhocko@suse.cz>
Thanks!
> ---
> mm/huge_memory.c | 9 ++++++++-
> mm/page_alloc.c | 2 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 7de1bf8..e0e4e29 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> };
>
> +extern int user_min_free_kbytes;
>
> static int set_recommended_min_free_kbytes(void)
> {
> @@ -130,8 +131,14 @@ static int set_recommended_min_free_kbytes(void)
> (unsigned long) nr_free_buffer_pages() / 20);
> recommended_min <<= (PAGE_SHIFT-10);
>
> - if (recommended_min > min_free_kbytes)
> + if (recommended_min > min_free_kbytes) {
> + if (user_min_free_kbytes >= 0)
> + pr_info("raising min_free_kbytes from %d to %lu "
> + "to help transparent hugepage allocations\n",
> + min_free_kbytes, recommended_min);
> +
> min_free_kbytes = recommended_min;
> + }
> setup_per_zone_wmarks();
> return 0;
> }
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 9ea62b2..a9dcfd8 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -205,7 +205,7 @@ static char * const zone_names[MAX_NR_ZONES] = {
> };
>
> int min_free_kbytes = 1024;
> -int user_min_free_kbytes;
> +int user_min_free_kbytes = -1;
>
> static unsigned long __meminitdata nr_kernel_pages;
> static unsigned long __meminitdata nr_all_pages;
> --
> 1.7.7.6
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-09 7:32 ` Han Pingtian
@ 2014-01-09 21:15 ` David Rientjes
-1 siblings, 0 replies; 52+ messages in thread
From: David Rientjes @ 2014-01-09 21:15 UTC (permalink / raw)
To: linux-kernel, Mel Gorman, Andrew Morton, linux-mm, Michal Hocko,
Dave Hansen
On Thu, 9 Jan 2014, Han Pingtian wrote:
> min_free_kbytes may be raised during THP's initialization. Sometimes,
> this will change the value being set by user. Showing message will
> clarify this confusion.
>
> Only show this message when changing the value set by user according to
> Michal Hocko's suggestion.
>
> Showing the old value of min_free_kbytes according to Dave Hansen's
> suggestion. This will give user the chance to restore old value of
> min_free_kbytes.
>
> Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
> ---
> mm/huge_memory.c | 9 ++++++++-
> mm/page_alloc.c | 2 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 7de1bf8..e0e4e29 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> };
>
> +extern int user_min_free_kbytes;
>
We don't add extern declarations to .c files. How many other examples of
this can you find in mm/?
> static int set_recommended_min_free_kbytes(void)
> {
> @@ -130,8 +131,14 @@ static int set_recommended_min_free_kbytes(void)
> (unsigned long) nr_free_buffer_pages() / 20);
> recommended_min <<= (PAGE_SHIFT-10);
>
> - if (recommended_min > min_free_kbytes)
> + if (recommended_min > min_free_kbytes) {
> + if (user_min_free_kbytes >= 0)
> + pr_info("raising min_free_kbytes from %d to %lu "
> + "to help transparent hugepage allocations\n",
> + min_free_kbytes, recommended_min);
> +
> min_free_kbytes = recommended_min;
> + }
> setup_per_zone_wmarks();
> return 0;
> }
Does this even ever trigger since set_recommended_min_free_kbytes() is
called via late_initcall()?
--
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] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-09 21:15 ` David Rientjes
0 siblings, 0 replies; 52+ messages in thread
From: David Rientjes @ 2014-01-09 21:15 UTC (permalink / raw)
To: linux-kernel, Mel Gorman, Andrew Morton, linux-mm, Michal Hocko,
Dave Hansen
On Thu, 9 Jan 2014, Han Pingtian wrote:
> min_free_kbytes may be raised during THP's initialization. Sometimes,
> this will change the value being set by user. Showing message will
> clarify this confusion.
>
> Only show this message when changing the value set by user according to
> Michal Hocko's suggestion.
>
> Showing the old value of min_free_kbytes according to Dave Hansen's
> suggestion. This will give user the chance to restore old value of
> min_free_kbytes.
>
> Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
> ---
> mm/huge_memory.c | 9 ++++++++-
> mm/page_alloc.c | 2 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 7de1bf8..e0e4e29 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> };
>
> +extern int user_min_free_kbytes;
>
We don't add extern declarations to .c files. How many other examples of
this can you find in mm/?
> static int set_recommended_min_free_kbytes(void)
> {
> @@ -130,8 +131,14 @@ static int set_recommended_min_free_kbytes(void)
> (unsigned long) nr_free_buffer_pages() / 20);
> recommended_min <<= (PAGE_SHIFT-10);
>
> - if (recommended_min > min_free_kbytes)
> + if (recommended_min > min_free_kbytes) {
> + if (user_min_free_kbytes >= 0)
> + pr_info("raising min_free_kbytes from %d to %lu "
> + "to help transparent hugepage allocations\n",
> + min_free_kbytes, recommended_min);
> +
> min_free_kbytes = recommended_min;
> + }
> setup_per_zone_wmarks();
> return 0;
> }
Does this even ever trigger since set_recommended_min_free_kbytes() is
called via late_initcall()?
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-09 21:15 ` David Rientjes
@ 2014-01-10 8:05 ` Michal Hocko
-1 siblings, 0 replies; 52+ messages in thread
From: Michal Hocko @ 2014-01-10 8:05 UTC (permalink / raw)
To: David Rientjes
Cc: linux-kernel, Mel Gorman, Andrew Morton, linux-mm, Dave Hansen
On Thu 09-01-14 13:15:54, David Rientjes wrote:
> On Thu, 9 Jan 2014, Han Pingtian wrote:
>
> > min_free_kbytes may be raised during THP's initialization. Sometimes,
> > this will change the value being set by user. Showing message will
> > clarify this confusion.
> >
> > Only show this message when changing the value set by user according to
> > Michal Hocko's suggestion.
> >
> > Showing the old value of min_free_kbytes according to Dave Hansen's
> > suggestion. This will give user the chance to restore old value of
> > min_free_kbytes.
> >
> > Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
> > ---
> > mm/huge_memory.c | 9 ++++++++-
> > mm/page_alloc.c | 2 +-
> > 2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 7de1bf8..e0e4e29 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > };
> >
> > +extern int user_min_free_kbytes;
> >
>
> We don't add extern declarations to .c files. How many other examples of
> this can you find in mm/?
I have suggested this because general visibility is not needed. But if
you think that it should then include/linux/mm.h sounds like a proper
place.
> > static int set_recommended_min_free_kbytes(void)
> > {
> > @@ -130,8 +131,14 @@ static int set_recommended_min_free_kbytes(void)
> > (unsigned long) nr_free_buffer_pages() / 20);
> > recommended_min <<= (PAGE_SHIFT-10);
> >
> > - if (recommended_min > min_free_kbytes)
> > + if (recommended_min > min_free_kbytes) {
> > + if (user_min_free_kbytes >= 0)
> > + pr_info("raising min_free_kbytes from %d to %lu "
> > + "to help transparent hugepage allocations\n",
> > + min_free_kbytes, recommended_min);
> > +
> > min_free_kbytes = recommended_min;
> > + }
> > setup_per_zone_wmarks();
> > return 0;
> > }
>
> Does this even ever trigger since set_recommended_min_free_kbytes() is
> called via late_initcall()?
This is called whenever THP is enabled so it might be called later after
boot. The point is AFAIU to warn user that the admin decision about
min_free_kbytes is overridden.
--
Michal Hocko
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] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-10 8:05 ` Michal Hocko
0 siblings, 0 replies; 52+ messages in thread
From: Michal Hocko @ 2014-01-10 8:05 UTC (permalink / raw)
To: David Rientjes
Cc: linux-kernel, Mel Gorman, Andrew Morton, linux-mm, Dave Hansen
On Thu 09-01-14 13:15:54, David Rientjes wrote:
> On Thu, 9 Jan 2014, Han Pingtian wrote:
>
> > min_free_kbytes may be raised during THP's initialization. Sometimes,
> > this will change the value being set by user. Showing message will
> > clarify this confusion.
> >
> > Only show this message when changing the value set by user according to
> > Michal Hocko's suggestion.
> >
> > Showing the old value of min_free_kbytes according to Dave Hansen's
> > suggestion. This will give user the chance to restore old value of
> > min_free_kbytes.
> >
> > Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
> > ---
> > mm/huge_memory.c | 9 ++++++++-
> > mm/page_alloc.c | 2 +-
> > 2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 7de1bf8..e0e4e29 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > };
> >
> > +extern int user_min_free_kbytes;
> >
>
> We don't add extern declarations to .c files. How many other examples of
> this can you find in mm/?
I have suggested this because general visibility is not needed. But if
you think that it should then include/linux/mm.h sounds like a proper
place.
> > static int set_recommended_min_free_kbytes(void)
> > {
> > @@ -130,8 +131,14 @@ static int set_recommended_min_free_kbytes(void)
> > (unsigned long) nr_free_buffer_pages() / 20);
> > recommended_min <<= (PAGE_SHIFT-10);
> >
> > - if (recommended_min > min_free_kbytes)
> > + if (recommended_min > min_free_kbytes) {
> > + if (user_min_free_kbytes >= 0)
> > + pr_info("raising min_free_kbytes from %d to %lu "
> > + "to help transparent hugepage allocations\n",
> > + min_free_kbytes, recommended_min);
> > +
> > min_free_kbytes = recommended_min;
> > + }
> > setup_per_zone_wmarks();
> > return 0;
> > }
>
> Does this even ever trigger since set_recommended_min_free_kbytes() is
> called via late_initcall()?
This is called whenever THP is enabled so it might be called later after
boot. The point is AFAIU to warn user that the admin decision about
min_free_kbytes is overridden.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-10 8:05 ` Michal Hocko
@ 2014-01-10 8:13 ` Andrew Morton
-1 siblings, 0 replies; 52+ messages in thread
From: Andrew Morton @ 2014-01-10 8:13 UTC (permalink / raw)
To: Michal Hocko
Cc: David Rientjes, linux-kernel, Mel Gorman, linux-mm, Dave Hansen
On Fri, 10 Jan 2014 09:05:04 +0100 Michal Hocko <mhocko@suse.cz> wrote:
> > > --- a/mm/huge_memory.c
> > > +++ b/mm/huge_memory.c
> > > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > > };
> > >
> > > +extern int user_min_free_kbytes;
> > >
> >
> > We don't add extern declarations to .c files. How many other examples of
> > this can you find in mm/?
>
> I have suggested this because general visibility is not needed.
It's best to use a common declaration which is seen by the definition
site and all references, so everyone agrees on the variable's type.
Otherwise we could have "long foo;" in one file and "extern char foo;"
in another and the compiler won't tell us. I think the linker could
tell us, but it doesn't, afaik. Perhaps there's an option...
> But if
> you think that it should then include/linux/mm.h sounds like a proper
> place.
mm/internal.h might suit.
--
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] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-10 8:13 ` Andrew Morton
0 siblings, 0 replies; 52+ messages in thread
From: Andrew Morton @ 2014-01-10 8:13 UTC (permalink / raw)
To: Michal Hocko
Cc: David Rientjes, linux-kernel, Mel Gorman, linux-mm, Dave Hansen
On Fri, 10 Jan 2014 09:05:04 +0100 Michal Hocko <mhocko@suse.cz> wrote:
> > > --- a/mm/huge_memory.c
> > > +++ b/mm/huge_memory.c
> > > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > > };
> > >
> > > +extern int user_min_free_kbytes;
> > >
> >
> > We don't add extern declarations to .c files. How many other examples of
> > this can you find in mm/?
>
> I have suggested this because general visibility is not needed.
It's best to use a common declaration which is seen by the definition
site and all references, so everyone agrees on the variable's type.
Otherwise we could have "long foo;" in one file and "extern char foo;"
in another and the compiler won't tell us. I think the linker could
tell us, but it doesn't, afaik. Perhaps there's an option...
> But if
> you think that it should then include/linux/mm.h sounds like a proper
> place.
mm/internal.h might suit.
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-10 8:13 ` Andrew Morton
@ 2014-01-10 8:17 ` Michal Hocko
-1 siblings, 0 replies; 52+ messages in thread
From: Michal Hocko @ 2014-01-10 8:17 UTC (permalink / raw)
To: Andrew Morton
Cc: David Rientjes, linux-kernel, Mel Gorman, linux-mm, Dave Hansen
On Fri 10-01-14 00:13:44, Andrew Morton wrote:
> On Fri, 10 Jan 2014 09:05:04 +0100 Michal Hocko <mhocko@suse.cz> wrote:
>
> > > > --- a/mm/huge_memory.c
> > > > +++ b/mm/huge_memory.c
> > > > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > > > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > > > };
> > > >
> > > > +extern int user_min_free_kbytes;
> > > >
> > >
> > > We don't add extern declarations to .c files. How many other examples of
> > > this can you find in mm/?
> >
> > I have suggested this because general visibility is not needed.
>
> It's best to use a common declaration which is seen by the definition
> site and all references, so everyone agrees on the variable's type.
> Otherwise we could have "long foo;" in one file and "extern char foo;"
> in another and the compiler won't tell us. I think the linker could
> tell us, but it doesn't, afaik. Perhaps there's an option...
>
> > But if
> > you think that it should then include/linux/mm.h sounds like a proper
> > place.
>
> mm/internal.h might suit.
min_free_kbytes is in mm.h so I thought having them together would be
appropriate.
--
Michal Hocko
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] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-10 8:17 ` Michal Hocko
0 siblings, 0 replies; 52+ messages in thread
From: Michal Hocko @ 2014-01-10 8:17 UTC (permalink / raw)
To: Andrew Morton
Cc: David Rientjes, linux-kernel, Mel Gorman, linux-mm, Dave Hansen
On Fri 10-01-14 00:13:44, Andrew Morton wrote:
> On Fri, 10 Jan 2014 09:05:04 +0100 Michal Hocko <mhocko@suse.cz> wrote:
>
> > > > --- a/mm/huge_memory.c
> > > > +++ b/mm/huge_memory.c
> > > > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > > > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > > > };
> > > >
> > > > +extern int user_min_free_kbytes;
> > > >
> > >
> > > We don't add extern declarations to .c files. How many other examples of
> > > this can you find in mm/?
> >
> > I have suggested this because general visibility is not needed.
>
> It's best to use a common declaration which is seen by the definition
> site and all references, so everyone agrees on the variable's type.
> Otherwise we could have "long foo;" in one file and "extern char foo;"
> in another and the compiler won't tell us. I think the linker could
> tell us, but it doesn't, afaik. Perhaps there's an option...
>
> > But if
> > you think that it should then include/linux/mm.h sounds like a proper
> > place.
>
> mm/internal.h might suit.
min_free_kbytes is in mm.h so I thought having them together would be
appropriate.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-10 8:17 ` Michal Hocko
@ 2014-01-11 3:27 ` Han Pingtian
-1 siblings, 0 replies; 52+ messages in thread
From: Han Pingtian @ 2014-01-11 3:27 UTC (permalink / raw)
To: linux-kernel
Cc: David Rientjes, Mel Gorman, linux-mm, Dave Hansen, Michal Hocko,
Andrew Morton
On Fri, Jan 10, 2014 at 09:17:44AM +0100, Michal Hocko wrote:
> On Fri 10-01-14 00:13:44, Andrew Morton wrote:
> > On Fri, 10 Jan 2014 09:05:04 +0100 Michal Hocko <mhocko@suse.cz> wrote:
> >
> > > > > --- a/mm/huge_memory.c
> > > > > +++ b/mm/huge_memory.c
> > > > > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > > > > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > > > > };
> > > > >
> > > > > +extern int user_min_free_kbytes;
> > > > >
> > > >
> > > > We don't add extern declarations to .c files. How many other examples of
> > > > this can you find in mm/?
> > >
> > > I have suggested this because general visibility is not needed.
> >
> > It's best to use a common declaration which is seen by the definition
> > site and all references, so everyone agrees on the variable's type.
> > Otherwise we could have "long foo;" in one file and "extern char foo;"
> > in another and the compiler won't tell us. I think the linker could
> > tell us, but it doesn't, afaik. Perhaps there's an option...
> >
> > > But if
> > > you think that it should then include/linux/mm.h sounds like a proper
> > > place.
> >
> > mm/internal.h might suit.
>
> min_free_kbytes is in mm.h so I thought having them together would be
> appropriate.
>
At present, we only use user_min_free_kbytes in memory subsystem. So I
think mm/internal.h is suit.
Thanks.
--
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] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-11 3:27 ` Han Pingtian
0 siblings, 0 replies; 52+ messages in thread
From: Han Pingtian @ 2014-01-11 3:27 UTC (permalink / raw)
To: linux-kernel
Cc: David Rientjes, Mel Gorman, linux-mm, Dave Hansen, Michal Hocko,
Andrew Morton
On Fri, Jan 10, 2014 at 09:17:44AM +0100, Michal Hocko wrote:
> On Fri 10-01-14 00:13:44, Andrew Morton wrote:
> > On Fri, 10 Jan 2014 09:05:04 +0100 Michal Hocko <mhocko@suse.cz> wrote:
> >
> > > > > --- a/mm/huge_memory.c
> > > > > +++ b/mm/huge_memory.c
> > > > > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > > > > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > > > > };
> > > > >
> > > > > +extern int user_min_free_kbytes;
> > > > >
> > > >
> > > > We don't add extern declarations to .c files. How many other examples of
> > > > this can you find in mm/?
> > >
> > > I have suggested this because general visibility is not needed.
> >
> > It's best to use a common declaration which is seen by the definition
> > site and all references, so everyone agrees on the variable's type.
> > Otherwise we could have "long foo;" in one file and "extern char foo;"
> > in another and the compiler won't tell us. I think the linker could
> > tell us, but it doesn't, afaik. Perhaps there's an option...
> >
> > > But if
> > > you think that it should then include/linux/mm.h sounds like a proper
> > > place.
> >
> > mm/internal.h might suit.
>
> min_free_kbytes is in mm.h so I thought having them together would be
> appropriate.
>
At present, we only use user_min_free_kbytes in memory subsystem. So I
think mm/internal.h is suit.
Thanks.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-10 8:17 ` Michal Hocko
@ 2014-01-14 20:07 ` Han Pingtian
-1 siblings, 0 replies; 52+ messages in thread
From: Han Pingtian @ 2014-01-14 20:07 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Hocko, Andrew Morton, David Rientjes, Mel Gorman, linux-mm,
Dave Hansen
On Fri, Jan 10, 2014 at 09:17:44AM +0100, Michal Hocko wrote:
> On Fri 10-01-14 00:13:44, Andrew Morton wrote:
> > On Fri, 10 Jan 2014 09:05:04 +0100 Michal Hocko <mhocko@suse.cz> wrote:
> >
> > > > > --- a/mm/huge_memory.c
> > > > > +++ b/mm/huge_memory.c
> > > > > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > > > > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > > > > };
> > > > >
> > > > > +extern int user_min_free_kbytes;
> > > > >
> > > >
> > > > We don't add extern declarations to .c files. How many other examples of
> > > > this can you find in mm/?
> > >
> > > I have suggested this because general visibility is not needed.
> >
> > It's best to use a common declaration which is seen by the definition
> > site and all references, so everyone agrees on the variable's type.
> > Otherwise we could have "long foo;" in one file and "extern char foo;"
> > in another and the compiler won't tell us. I think the linker could
> > tell us, but it doesn't, afaik. Perhaps there's an option...
> >
> > > But if
> > > you think that it should then include/linux/mm.h sounds like a proper
> > > place.
> >
> > mm/internal.h might suit.
>
> min_free_kbytes is in mm.h so I thought having them together would be
> appropriate.
>
This is the latest version which put user_min_free_kbytes in
mm/internal.h. Please have a look. Thanks.
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-14 20:07 ` Han Pingtian
0 siblings, 0 replies; 52+ messages in thread
From: Han Pingtian @ 2014-01-14 20:07 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Hocko, Andrew Morton, David Rientjes, Mel Gorman, linux-mm,
Dave Hansen
On Fri, Jan 10, 2014 at 09:17:44AM +0100, Michal Hocko wrote:
> On Fri 10-01-14 00:13:44, Andrew Morton wrote:
> > On Fri, 10 Jan 2014 09:05:04 +0100 Michal Hocko <mhocko@suse.cz> wrote:
> >
> > > > > --- a/mm/huge_memory.c
> > > > > +++ b/mm/huge_memory.c
> > > > > @@ -100,6 +100,7 @@ static struct khugepaged_scan khugepaged_scan = {
> > > > > .mm_head = LIST_HEAD_INIT(khugepaged_scan.mm_head),
> > > > > };
> > > > >
> > > > > +extern int user_min_free_kbytes;
> > > > >
> > > >
> > > > We don't add extern declarations to .c files. How many other examples of
> > > > this can you find in mm/?
> > >
> > > I have suggested this because general visibility is not needed.
> >
> > It's best to use a common declaration which is seen by the definition
> > site and all references, so everyone agrees on the variable's type.
> > Otherwise we could have "long foo;" in one file and "extern char foo;"
> > in another and the compiler won't tell us. I think the linker could
> > tell us, but it doesn't, afaik. Perhaps there's an option...
> >
> > > But if
> > > you think that it should then include/linux/mm.h sounds like a proper
> > > place.
> >
> > mm/internal.h might suit.
>
> min_free_kbytes is in mm.h so I thought having them together would be
> appropriate.
>
This is the latest version which put user_min_free_kbytes in
mm/internal.h. Please have a look. Thanks.
>From 0d2583bea1f8ffa919e2cee3ee8ed08ec547284a Mon Sep 17 00:00:00 2001
From: Han Pingtian <hanpt@linux.vnet.ibm.com>
Date: Thu, 9 Jan 2014 15:24:26 +0800
Subject: [PATCH] mm: show message when raising min_free_kbytes in THP
min_free_kbytes may be raised during THP's initialization. Sometimes,
this will change the value being set by user. Showing message will
clarify this confusion.
Only show this message when changing the value set by user according to
Michal Hocko's suggestion.
Showing the old value of min_free_kbytes according to Dave Hansen's
suggestion. This will give user the chance to restore old value of
min_free_kbytes.
Signed-off-by: Han Pingtian <hanpt@linux.vnet.ibm.com>
---
mm/huge_memory.c | 8 +++++++-
mm/internal.h | 1 +
mm/page_alloc.c | 2 +-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 7de1bf8..2ca526b8 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -130,8 +130,14 @@ static int set_recommended_min_free_kbytes(void)
(unsigned long) nr_free_buffer_pages() / 20);
recommended_min <<= (PAGE_SHIFT-10);
- if (recommended_min > min_free_kbytes)
+ if (recommended_min > min_free_kbytes) {
+ if (user_min_free_kbytes >= 0)
+ pr_info("raising min_free_kbytes from %d to %lu "
+ "to help transparent hugepage allocations\n",
+ min_free_kbytes, recommended_min);
+
min_free_kbytes = recommended_min;
+ }
setup_per_zone_wmarks();
return 0;
}
diff --git a/mm/internal.h b/mm/internal.h
index 684f7aa..110d8da 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -101,6 +101,7 @@ extern void prep_compound_page(struct page *page, unsigned long order);
#ifdef CONFIG_MEMORY_FAILURE
extern bool is_free_buddy_page(struct page *page);
#endif
+extern int user_min_free_kbytes;
#if defined CONFIG_COMPACTION || defined CONFIG_CMA
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9ea62b2..a9dcfd8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -205,7 +205,7 @@ static char * const zone_names[MAX_NR_ZONES] = {
};
int min_free_kbytes = 1024;
-int user_min_free_kbytes;
+int user_min_free_kbytes = -1;
static unsigned long __meminitdata nr_kernel_pages;
static unsigned long __meminitdata nr_all_pages;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-14 20:07 ` Han Pingtian
@ 2014-01-14 23:52 ` Andrew Morton
-1 siblings, 0 replies; 52+ messages in thread
From: Andrew Morton @ 2014-01-14 23:52 UTC (permalink / raw)
To: Han Pingtian
Cc: linux-kernel, Michal Hocko, David Rientjes, Mel Gorman, linux-mm,
Dave Hansen
On Wed, 15 Jan 2014 04:07:20 +0800 Han Pingtian <hanpt@linux.vnet.ibm.com> wrote:
> min_free_kbytes may be raised during THP's initialization. Sometimes,
> this will change the value being set by user. Showing message will
> clarify this confusion.
>
> Only show this message when changing the value set by user according to
> Michal Hocko's suggestion.
>
> Showing the old value of min_free_kbytes according to Dave Hansen's
> suggestion. This will give user the chance to restore old value of
> min_free_kbytes.
>
This is all a bit nasty, isn't it? THP goes and alters min_free_kbytes
to improve its own reliability, but min_free_kbytes is also
user-modifiable. And over many years we have trained a *lot* of users
to alter min_free_kbytes. Often to prevent nasty page allocation
failure warnings from net drivers.
So there are probably quite a lot of people out there who are manually
rubbing out THP's efforts. And there may also be people who are
setting min_free_kbytes to a value which is unnecessarily high for more
recent kernels.
I don't know what to do about this mess though :(
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -130,8 +130,14 @@ static int set_recommended_min_free_kbytes(void)
> (unsigned long) nr_free_buffer_pages() / 20);
> recommended_min <<= (PAGE_SHIFT-10);
>
> - if (recommended_min > min_free_kbytes)
> + if (recommended_min > min_free_kbytes) {
> + if (user_min_free_kbytes >= 0)
> + pr_info("raising min_free_kbytes from %d to %lu "
> + "to help transparent hugepage allocations\n",
> + min_free_kbytes, recommended_min);
hm, recommended_min shouldn't have had long type. Oh well, we've done
worse things.
> min_free_kbytes = recommended_min;
> + }
> setup_per_zone_wmarks();
> return 0;
> }
--
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] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-14 23:52 ` Andrew Morton
0 siblings, 0 replies; 52+ messages in thread
From: Andrew Morton @ 2014-01-14 23:52 UTC (permalink / raw)
To: Han Pingtian
Cc: linux-kernel, Michal Hocko, David Rientjes, Mel Gorman, linux-mm,
Dave Hansen
On Wed, 15 Jan 2014 04:07:20 +0800 Han Pingtian <hanpt@linux.vnet.ibm.com> wrote:
> min_free_kbytes may be raised during THP's initialization. Sometimes,
> this will change the value being set by user. Showing message will
> clarify this confusion.
>
> Only show this message when changing the value set by user according to
> Michal Hocko's suggestion.
>
> Showing the old value of min_free_kbytes according to Dave Hansen's
> suggestion. This will give user the chance to restore old value of
> min_free_kbytes.
>
This is all a bit nasty, isn't it? THP goes and alters min_free_kbytes
to improve its own reliability, but min_free_kbytes is also
user-modifiable. And over many years we have trained a *lot* of users
to alter min_free_kbytes. Often to prevent nasty page allocation
failure warnings from net drivers.
So there are probably quite a lot of people out there who are manually
rubbing out THP's efforts. And there may also be people who are
setting min_free_kbytes to a value which is unnecessarily high for more
recent kernels.
I don't know what to do about this mess though :(
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -130,8 +130,14 @@ static int set_recommended_min_free_kbytes(void)
> (unsigned long) nr_free_buffer_pages() / 20);
> recommended_min <<= (PAGE_SHIFT-10);
>
> - if (recommended_min > min_free_kbytes)
> + if (recommended_min > min_free_kbytes) {
> + if (user_min_free_kbytes >= 0)
> + pr_info("raising min_free_kbytes from %d to %lu "
> + "to help transparent hugepage allocations\n",
> + min_free_kbytes, recommended_min);
hm, recommended_min shouldn't have had long type. Oh well, we've done
worse things.
> min_free_kbytes = recommended_min;
> + }
> setup_per_zone_wmarks();
> return 0;
> }
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-14 23:52 ` Andrew Morton
@ 2014-01-15 0:25 ` David Rientjes
-1 siblings, 0 replies; 52+ messages in thread
From: David Rientjes @ 2014-01-15 0:25 UTC (permalink / raw)
To: Andrew Morton
Cc: Han Pingtian, linux-kernel, Michal Hocko, Mel Gorman, linux-mm,
Dave Hansen
On Tue, 14 Jan 2014, Andrew Morton wrote:
> This is all a bit nasty, isn't it? THP goes and alters min_free_kbytes
> to improve its own reliability, but min_free_kbytes is also
> user-modifiable. And over many years we have trained a *lot* of users
> to alter min_free_kbytes. Often to prevent nasty page allocation
> failure warnings from net drivers.
>
I can vouch for kernel logs that are spammed with tons of net page
allocation failure warnings, in fact we're going through and adding
__GFP_NOWARN to most of these.
> So there are probably quite a lot of people out there who are manually
> rubbing out THP's efforts. And there may also be people who are
> setting min_free_kbytes to a value which is unnecessarily high for more
> recent kernels.
>
Indeed, we have initscripts that modified min_free_kbytes before thp was
introduced but luckily they were comparing their newly computed value to
the existing value and only writing if the new value is greater.
Hopefully most users are doing the same thing.
Would it be overkill to save the kernel default both with and without thp
and then doing a WARN_ON_ONCE() if a user-written value is ever less?
--
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] 52+ messages in thread
* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-15 0:25 ` David Rientjes
0 siblings, 0 replies; 52+ messages in thread
From: David Rientjes @ 2014-01-15 0:25 UTC (permalink / raw)
To: Andrew Morton
Cc: Han Pingtian, linux-kernel, Michal Hocko, Mel Gorman, linux-mm,
Dave Hansen
On Tue, 14 Jan 2014, Andrew Morton wrote:
> This is all a bit nasty, isn't it? THP goes and alters min_free_kbytes
> to improve its own reliability, but min_free_kbytes is also
> user-modifiable. And over many years we have trained a *lot* of users
> to alter min_free_kbytes. Often to prevent nasty page allocation
> failure warnings from net drivers.
>
I can vouch for kernel logs that are spammed with tons of net page
allocation failure warnings, in fact we're going through and adding
__GFP_NOWARN to most of these.
> So there are probably quite a lot of people out there who are manually
> rubbing out THP's efforts. And there may also be people who are
> setting min_free_kbytes to a value which is unnecessarily high for more
> recent kernels.
>
Indeed, we have initscripts that modified min_free_kbytes before thp was
introduced but luckily they were comparing their newly computed value to
the existing value and only writing if the new value is greater.
Hopefully most users are doing the same thing.
Would it be overkill to save the kernel default both with and without thp
and then doing a WARN_ON_ONCE() if a user-written value is ever less?
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-15 0:25 ` David Rientjes
@ 2014-01-15 0:35 ` Andrew Morton
-1 siblings, 0 replies; 52+ messages in thread
From: Andrew Morton @ 2014-01-15 0:35 UTC (permalink / raw)
To: David Rientjes
Cc: Han Pingtian, linux-kernel, Michal Hocko, Mel Gorman, linux-mm,
Dave Hansen
On Tue, 14 Jan 2014 16:25:10 -0800 (PST) David Rientjes <rientjes@google.com> wrote:
> On Tue, 14 Jan 2014, Andrew Morton wrote:
>
> > This is all a bit nasty, isn't it? THP goes and alters min_free_kbytes
> > to improve its own reliability, but min_free_kbytes is also
> > user-modifiable. And over many years we have trained a *lot* of users
> > to alter min_free_kbytes. Often to prevent nasty page allocation
> > failure warnings from net drivers.
> >
>
> I can vouch for kernel logs that are spammed with tons of net page
> allocation failure warnings, in fact we're going through and adding
> __GFP_NOWARN to most of these.
>
> > So there are probably quite a lot of people out there who are manually
> > rubbing out THP's efforts. And there may also be people who are
> > setting min_free_kbytes to a value which is unnecessarily high for more
> > recent kernels.
> >
>
> Indeed, we have initscripts that modified min_free_kbytes before thp was
> introduced but luckily they were comparing their newly computed value to
> the existing value and only writing if the new value is greater.
> Hopefully most users are doing the same thing.
I've been waiting 10+ years for us to decide to delete that warning due
to the false positives. Hasn't happened yet, and the warning does
find bugs/issues/misconfigurations/etc.
But I do worry this has led to users unnecessarily increasing
min_free_kbytes just to shut the warnings up. Net result: they have
less memory available for cache, etc.
> Would it be overkill to save the kernel default both with and without thp
> and then doing a WARN_ON_ONCE() if a user-written value is ever less?
Well, min_free_kbytes is a userspace thing, not a kernel thing - maybe
THP shouldn't be dinking with it. What effect is THP trying to achieve
and can we achieve it by other/better means?
--
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] 52+ messages in thread
* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-15 0:35 ` Andrew Morton
0 siblings, 0 replies; 52+ messages in thread
From: Andrew Morton @ 2014-01-15 0:35 UTC (permalink / raw)
To: David Rientjes
Cc: Han Pingtian, linux-kernel, Michal Hocko, Mel Gorman, linux-mm,
Dave Hansen
On Tue, 14 Jan 2014 16:25:10 -0800 (PST) David Rientjes <rientjes@google.com> wrote:
> On Tue, 14 Jan 2014, Andrew Morton wrote:
>
> > This is all a bit nasty, isn't it? THP goes and alters min_free_kbytes
> > to improve its own reliability, but min_free_kbytes is also
> > user-modifiable. And over many years we have trained a *lot* of users
> > to alter min_free_kbytes. Often to prevent nasty page allocation
> > failure warnings from net drivers.
> >
>
> I can vouch for kernel logs that are spammed with tons of net page
> allocation failure warnings, in fact we're going through and adding
> __GFP_NOWARN to most of these.
>
> > So there are probably quite a lot of people out there who are manually
> > rubbing out THP's efforts. And there may also be people who are
> > setting min_free_kbytes to a value which is unnecessarily high for more
> > recent kernels.
> >
>
> Indeed, we have initscripts that modified min_free_kbytes before thp was
> introduced but luckily they were comparing their newly computed value to
> the existing value and only writing if the new value is greater.
> Hopefully most users are doing the same thing.
I've been waiting 10+ years for us to decide to delete that warning due
to the false positives. Hasn't happened yet, and the warning does
find bugs/issues/misconfigurations/etc.
But I do worry this has led to users unnecessarily increasing
min_free_kbytes just to shut the warnings up. Net result: they have
less memory available for cache, etc.
> Would it be overkill to save the kernel default both with and without thp
> and then doing a WARN_ON_ONCE() if a user-written value is ever less?
Well, min_free_kbytes is a userspace thing, not a kernel thing - maybe
THP shouldn't be dinking with it. What effect is THP trying to achieve
and can we achieve it by other/better means?
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-15 0:35 ` Andrew Morton
@ 2014-01-15 0:52 ` David Rientjes
-1 siblings, 0 replies; 52+ messages in thread
From: David Rientjes @ 2014-01-15 0:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Han Pingtian, linux-kernel, Michal Hocko, Mel Gorman, linux-mm,
Dave Hansen
On Tue, 14 Jan 2014, Andrew Morton wrote:
> I've been waiting 10+ years for us to decide to delete that warning due
> to the false positives. Hasn't happened yet, and the warning does
> find bugs/issues/misconfigurations/etc.
>
I've found memory leaks from the meminfo that is emitted as part of page
allocation failure warnings, that seems to be the only helpful part.
Unfortunately, they typically emit ~80 lines to the kernel log and become
quite verbose in succession. If you have a lot of nodes, it just becomes
longer.
I think we want to consider alternative values for the ratelimiter, in
this case nopage_rs that Dave added. Dave?
> > Would it be overkill to save the kernel default both with and without thp
> > and then doing a WARN_ON_ONCE() if a user-written value is ever less?
>
> Well, min_free_kbytes is a userspace thing, not a kernel thing - maybe
> THP shouldn't be dinking with it. What effect is THP trying to achieve
> and can we achieve it by other/better means?
>
It moved the preferred "hugeadm --set-recommended-min_free_kbytes"
behavior into the kernel that gave better results (due to lower occurrence
of fragmentation) for thp hosts. Previously, people were using hugeadm
in initscripts and then it became the default kernel logic when thp was
originally merged. I think it's primarily targeted to adjust the high
watermark so we could probably get the same behavior by special casing thp
with some scalar to the watermarks, but changing min_free_kbytes was
probably the easiest way to do it.
--
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] 52+ messages in thread
* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-15 0:52 ` David Rientjes
0 siblings, 0 replies; 52+ messages in thread
From: David Rientjes @ 2014-01-15 0:52 UTC (permalink / raw)
To: Andrew Morton
Cc: Han Pingtian, linux-kernel, Michal Hocko, Mel Gorman, linux-mm,
Dave Hansen
On Tue, 14 Jan 2014, Andrew Morton wrote:
> I've been waiting 10+ years for us to decide to delete that warning due
> to the false positives. Hasn't happened yet, and the warning does
> find bugs/issues/misconfigurations/etc.
>
I've found memory leaks from the meminfo that is emitted as part of page
allocation failure warnings, that seems to be the only helpful part.
Unfortunately, they typically emit ~80 lines to the kernel log and become
quite verbose in succession. If you have a lot of nodes, it just becomes
longer.
I think we want to consider alternative values for the ratelimiter, in
this case nopage_rs that Dave added. Dave?
> > Would it be overkill to save the kernel default both with and without thp
> > and then doing a WARN_ON_ONCE() if a user-written value is ever less?
>
> Well, min_free_kbytes is a userspace thing, not a kernel thing - maybe
> THP shouldn't be dinking with it. What effect is THP trying to achieve
> and can we achieve it by other/better means?
>
It moved the preferred "hugeadm --set-recommended-min_free_kbytes"
behavior into the kernel that gave better results (due to lower occurrence
of fragmentation) for thp hosts. Previously, people were using hugeadm
in initscripts and then it became the default kernel logic when thp was
originally merged. I think it's primarily targeted to adjust the high
watermark so we could probably get the same behavior by special casing thp
with some scalar to the watermarks, but changing min_free_kbytes was
probably the easiest way to do it.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [RFC] mm: show message when updating min_free_kbytes in thp
2014-01-15 0:35 ` Andrew Morton
@ 2014-01-15 15:22 ` Mel Gorman
-1 siblings, 0 replies; 52+ messages in thread
From: Mel Gorman @ 2014-01-15 15:22 UTC (permalink / raw)
To: Andrew Morton
Cc: David Rientjes, Han Pingtian, linux-kernel, Michal Hocko,
linux-mm, Dave Hansen
On Tue, Jan 14, 2014 at 04:35:33PM -0800, Andrew Morton wrote:
> > Would it be overkill to save the kernel default both with and without thp
> > and then doing a WARN_ON_ONCE() if a user-written value is ever less?
>
> Well, min_free_kbytes is a userspace thing, not a kernel thing - maybe
> THP shouldn't be dinking with it. What effect is THP trying to achieve
> and can we achieve it by other/better means?
It moved logic from hugeadm where few people knew about it to the
kernel. The value is related to anti-fragmentation. With the recommended
setting the probability of mixing pages of different mobility within a
single pageblock is reduced. Very very superficially, it reduces the
number of instances the mm_page_alloc_extfrag tracepoint is triggered
with parameters that are considered to be severely fragmenting.
--
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] 52+ messages in thread
* Re: [RFC] mm: show message when updating min_free_kbytes in thp
@ 2014-01-15 15:22 ` Mel Gorman
0 siblings, 0 replies; 52+ messages in thread
From: Mel Gorman @ 2014-01-15 15:22 UTC (permalink / raw)
To: Andrew Morton
Cc: David Rientjes, Han Pingtian, linux-kernel, Michal Hocko,
linux-mm, Dave Hansen
On Tue, Jan 14, 2014 at 04:35:33PM -0800, Andrew Morton wrote:
> > Would it be overkill to save the kernel default both with and without thp
> > and then doing a WARN_ON_ONCE() if a user-written value is ever less?
>
> Well, min_free_kbytes is a userspace thing, not a kernel thing - maybe
> THP shouldn't be dinking with it. What effect is THP trying to achieve
> and can we achieve it by other/better means?
It moved logic from hugeadm where few people knew about it to the
kernel. The value is related to anti-fragmentation. With the recommended
setting the probability of mixing pages of different mobility within a
single pageblock is reduced. Very very superficially, it reduces the
number of instances the mm_page_alloc_extfrag tracepoint is triggered
with parameters that are considered to be severely fragmenting.
--
Mel Gorman
SUSE Labs
^ permalink raw reply [flat|nested] 52+ messages in thread