Patch "mm: fix remote numa hits statistics" has been added to the 4.9-stable tree

Patch "mm: fix remote numa hits statistics" has been added to the 4.9-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    mm: fix remote numa hits statistics

to the 4.9-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     mm-fix-remote-numa-hits-statistics.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From 2df26639e708a88dcc22171949da638a9998f3bc Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Tue, 10 Jan 2017 16:57:39 -0800
Subject: mm: fix remote numa hits statistics

From: Michal Hocko <mhocko@suse.com>

commit 2df26639e708a88dcc22171949da638a9998f3bc upstream.

Jia He has noticed that commit b9f00e147f27 ("mm, page_alloc: reduce
branches in zone_statistics") has an unintentional side effect that
remote node allocation requests are accounted as NUMA_MISS rathat than
NUMA_HIT and NUMA_OTHER if such a request doesn't use __GFP_OTHER_NODE.

There are many of these potentially because the flag is used very rarely
while we have many users of __alloc_pages_node.

Fix this by simply ignoring __GFP_OTHER_NODE (it can be removed in a
follow up patch) and treat all allocations that were satisfied from the
preferred zone's node as NUMA_HITS because this is the same node we
requested the allocation from in most cases.  If this is not the local
node then we just account it as NUMA_OTHER rather than NUMA_LOCAL.

One downsize would be that an allocation request for a node which is
outside of the mempolicy nodemask would be reported as a hit which is a
bit weird but that was the case before b9f00e147f27 already.

Fixes: b9f00e147f27 ("mm, page_alloc: reduce branches in zone_statistics")
Link: http://lkml.kernel.org/r/20170102153057.9451-2-mhocko@kernel.org
Signed-off-by: Michal Hocko <mhocko@suse.com>
Reported-by: Jia He <hejianet@gmail.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz> # with cbmc[1] superpowers
Acked-by: Mel Gorman <mgorman@suse.de>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Taku Izumi <izumi.taku@jp.fujitsu.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 mm/page_alloc.c |   15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2556,30 +2556,23 @@ int __isolate_free_page(struct page *pag
  * Update NUMA hit/miss statistics
  *
  * Must be called with interrupts disabled.
- *
- * When __GFP_OTHER_NODE is set assume the node of the preferred
- * zone is the local node. This is useful for daemons who allocate
- * memory on behalf of other processes.
  */
 static inline void zone_statistics(struct zone *preferred_zone, struct zone *z,
 								gfp_t flags)
 {
 #ifdef CONFIG_NUMA
-	int local_nid = numa_node_id();
 	enum zone_stat_item local_stat = NUMA_LOCAL;
 
-	if (unlikely(flags & __GFP_OTHER_NODE)) {
+	if (z->node != numa_node_id())
 		local_stat = NUMA_OTHER;
-		local_nid = preferred_zone->node;
-	}
 
-	if (z->node == local_nid) {
+	if (z->node == preferred_zone->node)
 		__inc_zone_state(z, NUMA_HIT);
-		__inc_zone_state(z, local_stat);
-	} else {
+	else {
 		__inc_zone_state(z, NUMA_MISS);
 		__inc_zone_state(preferred_zone, NUMA_FOREIGN);
 	}
+	__inc_zone_state(z, local_stat);
 #endif
 }
 


Patches currently in stable-queue which might be from mhocko@suse.com are

queue-4.9/mm-memcg-fix-the-active-list-aging-for-lowmem-requests-when-memcg-is-enabled.patch
queue-4.9/mm-fix-remote-numa-hits-statistics.patch
queue-4.9/mm-pmd-dirty-emulation-in-page-fault-handler.patch

Re: Patch "mm: fix remote numa hits statistics" has been added to the 4.9-stable tree

[Michal Hocko]

On Tue 17-01-17 17:13:03, Greg KH wrote:
> 
> This is a note to let you know that I've just added the patch titled
> 
>     mm: fix remote numa hits statistics
> 
> to the 4.9-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      mm-fix-remote-numa-hits-statistics.patch
> and it can be found in the queue-4.9 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.

It is not marked for stable so why it has been chosen? Me or Andrew
would have marked it for stable if it was needed. In fact I do not
really thing it is necessary. It fixes counters which haven't been
accurate for quite some time and nobody has noticed.
 
> >From 2df26639e708a88dcc22171949da638a9998f3bc Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Tue, 10 Jan 2017 16:57:39 -0800
> Subject: mm: fix remote numa hits statistics
> 
> From: Michal Hocko <mhocko@suse.com>
> 
> commit 2df26639e708a88dcc22171949da638a9998f3bc upstream.
> 
> Jia He has noticed that commit b9f00e147f27 ("mm, page_alloc: reduce
> branches in zone_statistics") has an unintentional side effect that
> remote node allocation requests are accounted as NUMA_MISS rathat than
> NUMA_HIT and NUMA_OTHER if such a request doesn't use __GFP_OTHER_NODE.
> 
> There are many of these potentially because the flag is used very rarely
> while we have many users of __alloc_pages_node.
> 
> Fix this by simply ignoring __GFP_OTHER_NODE (it can be removed in a
> follow up patch) and treat all allocations that were satisfied from the
> preferred zone's node as NUMA_HITS because this is the same node we
> requested the allocation from in most cases.  If this is not the local
> node then we just account it as NUMA_OTHER rather than NUMA_LOCAL.
> 
> One downsize would be that an allocation request for a node which is
> outside of the mempolicy nodemask would be reported as a hit which is a
> bit weird but that was the case before b9f00e147f27 already.
> 
> Fixes: b9f00e147f27 ("mm, page_alloc: reduce branches in zone_statistics")
> Link: http://lkml.kernel.org/r/20170102153057.9451-2-mhocko@kernel.org
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> Reported-by: Jia He <hejianet@gmail.com>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz> # with cbmc[1] superpowers
> Acked-by: Mel Gorman <mgorman@suse.de>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Cc: Taku Izumi <izumi.taku@jp.fujitsu.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
>  mm/page_alloc.c |   15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2556,30 +2556,23 @@ int __isolate_free_page(struct page *pag
>   * Update NUMA hit/miss statistics
>   *
>   * Must be called with interrupts disabled.
> - *
> - * When __GFP_OTHER_NODE is set assume the node of the preferred
> - * zone is the local node. This is useful for daemons who allocate
> - * memory on behalf of other processes.
>   */
>  static inline void zone_statistics(struct zone *preferred_zone, struct zone *z,
>  								gfp_t flags)
>  {
>  #ifdef CONFIG_NUMA
> -	int local_nid = numa_node_id();
>  	enum zone_stat_item local_stat = NUMA_LOCAL;
>  
> -	if (unlikely(flags & __GFP_OTHER_NODE)) {
> +	if (z->node != numa_node_id())
>  		local_stat = NUMA_OTHER;
> -		local_nid = preferred_zone->node;
> -	}
>  
> -	if (z->node == local_nid) {
> +	if (z->node == preferred_zone->node)
>  		__inc_zone_state(z, NUMA_HIT);
> -		__inc_zone_state(z, local_stat);
> -	} else {
> +	else {
>  		__inc_zone_state(z, NUMA_MISS);
>  		__inc_zone_state(preferred_zone, NUMA_FOREIGN);
>  	}
> +	__inc_zone_state(z, local_stat);
>  #endif
>  }
>  
> 
> 
> Patches currently in stable-queue which might be from mhocko@suse.com are
> 
> queue-4.9/mm-memcg-fix-the-active-list-aging-for-lowmem-requests-when-memcg-is-enabled.patch
> queue-4.9/mm-fix-remote-numa-hits-statistics.patch
> queue-4.9/mm-pmd-dirty-emulation-in-page-fault-handler.patch

-- 
Michal Hocko
SUSE Labs

Re: Patch "mm: fix remote numa hits statistics" has been added to the 4.9-stable tree

[Greg KH]

On Tue, Jan 17, 2017 at 05:21:44PM +0100, Michal Hocko wrote:
> On Tue 17-01-17 17:13:03, Greg KH wrote:
> > 
> > This is a note to let you know that I've just added the patch titled
> > 
> >     mm: fix remote numa hits statistics
> > 
> > to the 4.9-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > 
> > The filename of the patch is:
> >      mm-fix-remote-numa-hits-statistics.patch
> > and it can be found in the queue-4.9 subdirectory.
> > 
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> 
> It is not marked for stable so why it has been chosen?

I found it by digging through the git commit logs.

> Me or Andrew would have marked it for stable if it was needed. In fact
> I do not really thing it is necessary. It fixes counters which haven't
> been accurate for quite some time and nobody has noticed.

Ok, that's fair enough, I'll go drop it from the tree.

thanks,

greg k-h

Re: Patch "mm: fix remote numa hits statistics" has been added to the 4.9-stable tree

[Michal Hocko]

On Tue 17-01-17 17:28:25, Greg KH wrote:
> On Tue, Jan 17, 2017 at 05:21:44PM +0100, Michal Hocko wrote:
> > On Tue 17-01-17 17:13:03, Greg KH wrote:
> > > 
> > > This is a note to let you know that I've just added the patch titled
> > > 
> > >     mm: fix remote numa hits statistics
> > > 
> > > to the 4.9-stable tree which can be found at:
> > >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > > 
> > > The filename of the patch is:
> > >      mm-fix-remote-numa-hits-statistics.patch
> > > and it can be found in the queue-4.9 subdirectory.
> > > 
> > > If you, or anyone else, feels it should not be added to the stable tree,
> > > please let <stable@vger.kernel.org> know about it.
> > 
> > It is not marked for stable so why it has been chosen?
> 
> I found it by digging through the git commit logs.

Was the Fixes tag the reason to pick it up?

-- 
Michal Hocko
SUSE Labs

Re: Patch "mm: fix remote numa hits statistics" has been added to the 4.9-stable tree

[Greg KH]

On Wed, Jan 18, 2017 at 11:01:22AM +0100, Michal Hocko wrote:
> On Tue 17-01-17 17:28:25, Greg KH wrote:
> > On Tue, Jan 17, 2017 at 05:21:44PM +0100, Michal Hocko wrote:
> > > On Tue 17-01-17 17:13:03, Greg KH wrote:
> > > > 
> > > > This is a note to let you know that I've just added the patch titled
> > > > 
> > > >     mm: fix remote numa hits statistics
> > > > 
> > > > to the 4.9-stable tree which can be found at:
> > > >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > > > 
> > > > The filename of the patch is:
> > > >      mm-fix-remote-numa-hits-statistics.patch
> > > > and it can be found in the queue-4.9 subdirectory.
> > > > 
> > > > If you, or anyone else, feels it should not be added to the stable tree,
> > > > please let <stable@vger.kernel.org> know about it.
> > > 
> > > It is not marked for stable so why it has been chosen?
> > 
> > I found it by digging through the git commit logs.
> 
> Was the Fixes tag the reason to pick it up?

It made me pay a bit more attention to it, yes.  I search for that and
other keywords, in the git log to see if there are patches that need to
be backported where people forget to add the stable tags.  I have to do
this because some subsystems still never set them :(

thanks,

greg k-h

Re: Patch "mm: fix remote numa hits statistics" has been added to the 4.9-stable tree

[Michal Hocko]

On Wed 18-01-17 11:23:38, Greg KH wrote:
> On Wed, Jan 18, 2017 at 11:01:22AM +0100, Michal Hocko wrote:
> > On Tue 17-01-17 17:28:25, Greg KH wrote:
> > > On Tue, Jan 17, 2017 at 05:21:44PM +0100, Michal Hocko wrote:
> > > > On Tue 17-01-17 17:13:03, Greg KH wrote:
> > > > > 
> > > > > This is a note to let you know that I've just added the patch titled
> > > > > 
> > > > >     mm: fix remote numa hits statistics
> > > > > 
> > > > > to the 4.9-stable tree which can be found at:
> > > > >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > > > > 
> > > > > The filename of the patch is:
> > > > >      mm-fix-remote-numa-hits-statistics.patch
> > > > > and it can be found in the queue-4.9 subdirectory.
> > > > > 
> > > > > If you, or anyone else, feels it should not be added to the stable tree,
> > > > > please let <stable@vger.kernel.org> know about it.
> > > > 
> > > > It is not marked for stable so why it has been chosen?
> > > 
> > > I found it by digging through the git commit logs.
> > 
> > Was the Fixes tag the reason to pick it up?
> 
> It made me pay a bit more attention to it, yes.  I search for that and
> other keywords, in the git log to see if there are patches that need to
> be backported where people forget to add the stable tags.  I have to do
> this because some subsystems still never set them :(

I am pretty sure that Andrew is really trying hard to mark all the core
MM changes for stable. Picking up changes just because they have Fixes:
tag in them is imho a bad idea. If somebody made the effort to add this
tag I am pretty sure Cc: stable was considered as well. If that is not
the case let's just push back and ask directly when a patch is submitted
rather than picking up the patch and hope that somebody will object...

Don't get me wrong, I really do appreciate your effort here I am just
worried that conflating Fixes: and Cc: stable is a wrong thing. I
absolutely see a reason CC: stable should imply Fixes: but not other way
around.
-- 
Michal Hocko
SUSE Labs

Re: Patch "mm: fix remote numa hits statistics" has been added to the 4.9-stable tree

[Greg KH]

On Wed, Jan 18, 2017 at 11:33:39AM +0100, Michal Hocko wrote:
> On Wed 18-01-17 11:23:38, Greg KH wrote:
> > On Wed, Jan 18, 2017 at 11:01:22AM +0100, Michal Hocko wrote:
> > > On Tue 17-01-17 17:28:25, Greg KH wrote:
> > > > On Tue, Jan 17, 2017 at 05:21:44PM +0100, Michal Hocko wrote:
> > > > > On Tue 17-01-17 17:13:03, Greg KH wrote:
> > > > > > 
> > > > > > This is a note to let you know that I've just added the patch titled
> > > > > > 
> > > > > >     mm: fix remote numa hits statistics
> > > > > > 
> > > > > > to the 4.9-stable tree which can be found at:
> > > > > >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > > > > > 
> > > > > > The filename of the patch is:
> > > > > >      mm-fix-remote-numa-hits-statistics.patch
> > > > > > and it can be found in the queue-4.9 subdirectory.
> > > > > > 
> > > > > > If you, or anyone else, feels it should not be added to the stable tree,
> > > > > > please let <stable@vger.kernel.org> know about it.
> > > > > 
> > > > > It is not marked for stable so why it has been chosen?
> > > > 
> > > > I found it by digging through the git commit logs.
> > > 
> > > Was the Fixes tag the reason to pick it up?
> > 
> > It made me pay a bit more attention to it, yes.  I search for that and
> > other keywords, in the git log to see if there are patches that need to
> > be backported where people forget to add the stable tags.  I have to do
> > this because some subsystems still never set them :(
> 
> I am pretty sure that Andrew is really trying hard to mark all the core
> MM changes for stable. Picking up changes just because they have Fixes:
> tag in them is imho a bad idea. If somebody made the effort to add this
> tag I am pretty sure Cc: stable was considered as well. If that is not
> the case let's just push back and ask directly when a patch is submitted
> rather than picking up the patch and hope that somebody will object...
> 
> Don't get me wrong, I really do appreciate your effort here I am just
> worried that conflating Fixes: and Cc: stable is a wrong thing. I
> absolutely see a reason CC: stable should imply Fixes: but not other way
> around.

Yes, for lots of subsystems fixes: does not mean stable, but for a
non-small number of other ones, it is needed still :(

Andrew does a great job marking stable fixes, I'll try to remember that
mm: stuff should not be backported if it only has a fixes: tag in the
future.

thanks,

greg k-h

Patch "mm: fix remote numa hits statistics" has been added to the 4.9-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    mm: fix remote numa hits statistics

to the 4.9-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     mm-fix-remote-numa-hits-statistics.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From foo@baz Wed Dec  6 17:39:55 CET 2017
From: Michal Hocko <mhocko@suse.com>
Date: Tue, 10 Jan 2017 16:57:39 -0800
Subject: mm: fix remote numa hits statistics

From: Michal Hocko <mhocko@suse.com>


[ Upstream commit 2df26639e708a88dcc22171949da638a9998f3bc ]

Jia He has noticed that commit b9f00e147f27 ("mm, page_alloc: reduce
branches in zone_statistics") has an unintentional side effect that
remote node allocation requests are accounted as NUMA_MISS rathat than
NUMA_HIT and NUMA_OTHER if such a request doesn't use __GFP_OTHER_NODE.

There are many of these potentially because the flag is used very rarely
while we have many users of __alloc_pages_node.

Fix this by simply ignoring __GFP_OTHER_NODE (it can be removed in a
follow up patch) and treat all allocations that were satisfied from the
preferred zone's node as NUMA_HITS because this is the same node we
requested the allocation from in most cases.  If this is not the local
node then we just account it as NUMA_OTHER rather than NUMA_LOCAL.

One downsize would be that an allocation request for a node which is
outside of the mempolicy nodemask would be reported as a hit which is a
bit weird but that was the case before b9f00e147f27 already.

Fixes: b9f00e147f27 ("mm, page_alloc: reduce branches in zone_statistics")
Link: http://lkml.kernel.org/r/20170102153057.9451-2-mhocko@kernel.org
Signed-off-by: Michal Hocko <mhocko@suse.com>
Reported-by: Jia He <hejianet@gmail.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz> # with cbmc[1] superpowers
Acked-by: Mel Gorman <mgorman@suse.de>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Taku Izumi <izumi.taku@jp.fujitsu.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/page_alloc.c |   15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2592,30 +2592,23 @@ int __isolate_free_page(struct page *pag
  * Update NUMA hit/miss statistics
  *
  * Must be called with interrupts disabled.
- *
- * When __GFP_OTHER_NODE is set assume the node of the preferred
- * zone is the local node. This is useful for daemons who allocate
- * memory on behalf of other processes.
  */
 static inline void zone_statistics(struct zone *preferred_zone, struct zone *z,
 								gfp_t flags)
 {
 #ifdef CONFIG_NUMA
-	int local_nid = numa_node_id();
 	enum zone_stat_item local_stat = NUMA_LOCAL;
 
-	if (unlikely(flags & __GFP_OTHER_NODE)) {
+	if (z->node != numa_node_id())
 		local_stat = NUMA_OTHER;
-		local_nid = preferred_zone->node;
-	}
 
-	if (z->node == local_nid) {
+	if (z->node == preferred_zone->node)
 		__inc_zone_state(z, NUMA_HIT);
-		__inc_zone_state(z, local_stat);
-	} else {
+	else {
 		__inc_zone_state(z, NUMA_MISS);
 		__inc_zone_state(preferred_zone, NUMA_FOREIGN);
 	}
+	__inc_zone_state(z, local_stat);
 #endif
 }
 


Patches currently in stable-queue which might be from mhocko@suse.com are

queue-4.9/mm-fix-remote-numa-hits-statistics.patch
queue-4.9/mm-oom_reaper-gather-each-vma-to-prevent-leaking-tlb-entry.patch