All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Dave Hansen <dave.hansen@intel.com>,
	Rik van Riel <riel@redhat.com>, Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/7] mm: page_alloc: Use zone node IDs to approximate locality
Date: Tue, 17 Dec 2013 16:08:08 +0000	[thread overview]
Message-ID: <20131217160808.GF11295@suse.de> (raw)
In-Reply-To: <20131217153829.GC21724@cmpxchg.org>

On Tue, Dec 17, 2013 at 10:38:29AM -0500, Johannes Weiner wrote:
> On Tue, Dec 17, 2013 at 11:13:52AM +0000, Mel Gorman wrote:
> > On Mon, Dec 16, 2013 at 03:25:07PM -0500, Johannes Weiner wrote:
> > > On Fri, Dec 13, 2013 at 02:10:03PM +0000, Mel Gorman wrote:
> > > > zone_local is using node_distance which is a more expensive call than
> > > > necessary. On x86, it's another function call in the allocator fast path
> > > > and increases cache footprint. This patch makes the assumption zones on a
> > > > local node will share the same node ID. The necessary information should
> > > > already be cache hot.
> > > > 
> > > > Signed-off-by: Mel Gorman <mgorman@suse.de>
> > > > ---
> > > >  mm/page_alloc.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > > index 64020eb..fd9677e 100644
> > > > --- a/mm/page_alloc.c
> > > > +++ b/mm/page_alloc.c
> > > > @@ -1816,7 +1816,7 @@ static void zlc_clear_zones_full(struct zonelist *zonelist)
> > > >  
> > > >  static bool zone_local(struct zone *local_zone, struct zone *zone)
> > > >  {
> > > > -	return node_distance(local_zone->node, zone->node) == LOCAL_DISTANCE;
> > > > +	return zone_to_nid(zone) == numa_node_id();
> > > 
> > > Why numa_node_id()?  We pass in the preferred zone as @local_zone:
> > > 
> > 
> > Initially because I was thinking "local node" and numa_node_id() is a
> > per-cpu variable that should be cheap to access and in some cases
> > cache-hot as the top-level gfp API calls numa_node_id().
> > 
> > Thinking about it more though it still makes sense because the preferred
> > zone is not necessarily local. If the allocation request requires ZONE_DMA32
> > and the local node does not have that zone then preferred zone is on a
> > remote node.
> 
> Don't we treat everything in relation to the preferred zone?

Usually yes, but this time we really care about whether the memory is
local or remote. It makes sense to me as it is and struggle to see an
advantage of expressing it in terms of the preferred zone. Minimally
zone_local would need to be renamed if it could return true for a remote
zone and I see no advantage in doing that.

I might be stuck in a "la la la, everything is fine" rut.

-- 
Mel Gorman
SUSE Labs

WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@suse.de>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Dave Hansen <dave.hansen@intel.com>,
	Rik van Riel <riel@redhat.com>, Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/7] mm: page_alloc: Use zone node IDs to approximate locality
Date: Tue, 17 Dec 2013 16:08:08 +0000	[thread overview]
Message-ID: <20131217160808.GF11295@suse.de> (raw)
In-Reply-To: <20131217153829.GC21724@cmpxchg.org>

On Tue, Dec 17, 2013 at 10:38:29AM -0500, Johannes Weiner wrote:
> On Tue, Dec 17, 2013 at 11:13:52AM +0000, Mel Gorman wrote:
> > On Mon, Dec 16, 2013 at 03:25:07PM -0500, Johannes Weiner wrote:
> > > On Fri, Dec 13, 2013 at 02:10:03PM +0000, Mel Gorman wrote:
> > > > zone_local is using node_distance which is a more expensive call than
> > > > necessary. On x86, it's another function call in the allocator fast path
> > > > and increases cache footprint. This patch makes the assumption zones on a
> > > > local node will share the same node ID. The necessary information should
> > > > already be cache hot.
> > > > 
> > > > Signed-off-by: Mel Gorman <mgorman@suse.de>
> > > > ---
> > > >  mm/page_alloc.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > > index 64020eb..fd9677e 100644
> > > > --- a/mm/page_alloc.c
> > > > +++ b/mm/page_alloc.c
> > > > @@ -1816,7 +1816,7 @@ static void zlc_clear_zones_full(struct zonelist *zonelist)
> > > >  
> > > >  static bool zone_local(struct zone *local_zone, struct zone *zone)
> > > >  {
> > > > -	return node_distance(local_zone->node, zone->node) == LOCAL_DISTANCE;
> > > > +	return zone_to_nid(zone) == numa_node_id();
> > > 
> > > Why numa_node_id()?  We pass in the preferred zone as @local_zone:
> > > 
> > 
> > Initially because I was thinking "local node" and numa_node_id() is a
> > per-cpu variable that should be cheap to access and in some cases
> > cache-hot as the top-level gfp API calls numa_node_id().
> > 
> > Thinking about it more though it still makes sense because the preferred
> > zone is not necessarily local. If the allocation request requires ZONE_DMA32
> > and the local node does not have that zone then preferred zone is on a
> > remote node.
> 
> Don't we treat everything in relation to the preferred zone?

Usually yes, but this time we really care about whether the memory is
local or remote. It makes sense to me as it is and struggle to see an
advantage of expressing it in terms of the preferred zone. Minimally
zone_local would need to be renamed if it could return true for a remote
zone and I see no advantage in doing that.

I might be stuck in a "la la la, everything is fine" rut.

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

  reply	other threads:[~2013-12-17 16:08 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-13 14:10 [RFC PATCH 0/7] Configurable fair allocation zone policy v2r6 Mel Gorman
2013-12-13 14:10 ` Mel Gorman
2013-12-13 14:10 ` [PATCH 1/7] mm: page_alloc: exclude unreclaimable allocations from zone fairness policy Mel Gorman
2013-12-13 14:10   ` Mel Gorman
2013-12-13 15:45   ` Rik van Riel
2013-12-13 15:45     ` Rik van Riel
2013-12-13 14:10 ` [PATCH 2/7] mm: page_alloc: Break out zone page aging distribution into its own helper Mel Gorman
2013-12-13 14:10   ` Mel Gorman
2013-12-13 15:46   ` Rik van Riel
2013-12-13 15:46     ` Rik van Riel
2013-12-16 20:16   ` Johannes Weiner
2013-12-16 20:16     ` Johannes Weiner
2013-12-13 14:10 ` [PATCH 3/7] mm: page_alloc: Use zone node IDs to approximate locality Mel Gorman
2013-12-13 14:10   ` Mel Gorman
2013-12-16 13:20   ` Rik van Riel
2013-12-16 13:20     ` Rik van Riel
2013-12-16 20:25   ` Johannes Weiner
2013-12-16 20:25     ` Johannes Weiner
2013-12-17 11:13     ` Mel Gorman
2013-12-17 11:13       ` Mel Gorman
2013-12-17 15:38       ` Johannes Weiner
2013-12-17 15:38         ` Johannes Weiner
2013-12-17 16:08         ` Mel Gorman [this message]
2013-12-17 16:08           ` Mel Gorman
2013-12-17 20:11           ` Johannes Weiner
2013-12-17 20:11             ` Johannes Weiner
2013-12-17 21:03             ` Mel Gorman
2013-12-17 21:03               ` Mel Gorman
2013-12-17 22:31               ` Johannes Weiner
2013-12-17 22:31                 ` Johannes Weiner
2013-12-13 14:10 ` [PATCH 4/7] mm: Annotate page cache allocations Mel Gorman
2013-12-13 14:10   ` Mel Gorman
2013-12-16 15:20   ` Rik van Riel
2013-12-16 15:20     ` Rik van Riel
2013-12-13 14:10 ` [PATCH 5/7] mm: page_alloc: Make zone distribution page aging policy configurable Mel Gorman
2013-12-13 14:10   ` Mel Gorman
2013-12-16 19:25   ` Rik van Riel
2013-12-16 19:25     ` Rik van Riel
2013-12-16 20:42   ` Johannes Weiner
2013-12-16 20:42     ` Johannes Weiner
2013-12-17 15:29     ` Mel Gorman
2013-12-17 15:29       ` Mel Gorman
2013-12-17 15:54       ` Johannes Weiner
2013-12-17 15:54         ` Johannes Weiner
2013-12-17 16:14         ` Mel Gorman
2013-12-17 16:14           ` Mel Gorman
2013-12-17 17:43           ` Johannes Weiner
2013-12-17 17:43             ` Johannes Weiner
2013-12-17 21:22             ` Mel Gorman
2013-12-17 21:22               ` Mel Gorman
2013-12-17 22:57               ` Johannes Weiner
2013-12-17 22:57                 ` Johannes Weiner
2013-12-17 23:24                 ` Mel Gorman
2013-12-17 23:24                   ` Mel Gorman
2013-12-13 14:10 ` [PATCH 6/7] mm: page_alloc: Only account batch allocations requests that are eligible Mel Gorman
2013-12-13 14:10   ` Mel Gorman
2013-12-16 20:52   ` Johannes Weiner
2013-12-16 20:52     ` Johannes Weiner
2013-12-17 11:20     ` Mel Gorman
2013-12-17 11:20       ` Mel Gorman
2013-12-17 15:43       ` Johannes Weiner
2013-12-17 15:43         ` Johannes Weiner
2013-12-17 16:06         ` Mel Gorman
2013-12-17 16:06           ` Mel Gorman
2013-12-13 14:10 ` [PATCH 7/7] mm: page_alloc: Default allow file pages to use remote nodes for fair allocation policy Mel Gorman
2013-12-13 14:10   ` Mel Gorman
2013-12-13 17:04   ` Johannes Weiner
2013-12-13 17:04     ` Johannes Weiner
2013-12-13 19:20     ` Mel Gorman
2013-12-13 19:20       ` Mel Gorman
2013-12-13 22:15       ` Johannes Weiner
2013-12-13 22:15         ` Johannes Weiner
2013-12-17 16:04         ` Mel Gorman
2013-12-17 16:04           ` Mel Gorman
2013-12-16 19:26   ` Rik van Riel
2013-12-16 19:26     ` Rik van Riel
2013-12-17 15:07 ` [RFC PATCH 0/7] Configurable fair allocation zone policy v2r6 Zlatko Calusic
2013-12-17 15:07   ` Zlatko Calusic
2013-12-17 21:23   ` Mel Gorman
2013-12-17 21:23     ` Mel Gorman
2013-12-21 16:03     ` Zlatko Calusic
2013-12-21 16:03       ` Zlatko Calusic
2013-12-23 10:26       ` Mel Gorman
2013-12-23 10:26         ` Mel Gorman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131217160808.GF11295@suse.de \
    --to=mgorman@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@intel.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=riel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.