All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Rik van Riel <riel@redhat.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps
Date: Mon, 27 Jan 2014 11:03:30 +0000	[thread overview]
Message-ID: <20140127110330.GH4963@suse.de> (raw)
In-Reply-To: <alpine.DEB.2.02.1401251902180.3140@chino.kir.corp.google.com>

On Sat, Jan 25, 2014 at 07:12:35PM -0800, David Rientjes wrote:
> As a result of commit 5606e3877ad8 ("mm: numa: Migrate on reference 
> policy"), /proc/<pid>/numa_maps prints the mempolicy for any <pid> as 
> "prefer:N" for the local node, N, of the process reading the file.
> 
> This should only be printed when the mempolicy of <pid> is MPOL_PREFERRED 
> for node N.
> 
> If the process is actually only using the default mempolicy for local node 
> allocation, make sure "default" is printed as expected.
> 
> Reported-by: Robert Lippert <rlippert@google.com>
> Signed-off-by: David Rientjes <rientjes@google.com>

Hmm, it is using a preferred policy but I see your point as expectations
of an application parsing numa_maps have been broken.  The patch makes
non-obvious assumptions about how and when MPOL_F_MORON gets set which
could change in the future and be missed. Use this instead? It might need
to be changed again if there is a need to control whether automatic numa
balancing can be enabled or disabled on a per-process basis.

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index c2ccec0..c1a2573 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -120,6 +120,14 @@ static struct mempolicy default_policy = {
 
 static struct mempolicy preferred_node_policy[MAX_NUMNODES];
 
+/* Returns true if the policy is the default policy */
+static bool mpol_is_default(struct mempolicy *pol)
+{
+	return !pol ||
+		pol == &default_policy ||
+		pol == &preferred_node_policy[numa_node_id()];
+}
+
 static struct mempolicy *get_task_policy(struct task_struct *p)
 {
 	struct mempolicy *pol = p->mempolicy;
@@ -2856,7 +2864,7 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol)
 	unsigned short mode = MPOL_DEFAULT;
 	unsigned short flags = 0;
 
-	if (pol && pol != &default_policy) {
+	if (!mpol_is_default(pol)) {
 		mode = pol->mode;
 		flags = pol->flags;
 	}

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

WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@suse.de>
To: David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Rik van Riel <riel@redhat.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps
Date: Mon, 27 Jan 2014 11:03:30 +0000	[thread overview]
Message-ID: <20140127110330.GH4963@suse.de> (raw)
In-Reply-To: <alpine.DEB.2.02.1401251902180.3140@chino.kir.corp.google.com>

On Sat, Jan 25, 2014 at 07:12:35PM -0800, David Rientjes wrote:
> As a result of commit 5606e3877ad8 ("mm: numa: Migrate on reference 
> policy"), /proc/<pid>/numa_maps prints the mempolicy for any <pid> as 
> "prefer:N" for the local node, N, of the process reading the file.
> 
> This should only be printed when the mempolicy of <pid> is MPOL_PREFERRED 
> for node N.
> 
> If the process is actually only using the default mempolicy for local node 
> allocation, make sure "default" is printed as expected.
> 
> Reported-by: Robert Lippert <rlippert@google.com>
> Signed-off-by: David Rientjes <rientjes@google.com>

Hmm, it is using a preferred policy but I see your point as expectations
of an application parsing numa_maps have been broken.  The patch makes
non-obvious assumptions about how and when MPOL_F_MORON gets set which
could change in the future and be missed. Use this instead? It might need
to be changed again if there is a need to control whether automatic numa
balancing can be enabled or disabled on a per-process basis.

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index c2ccec0..c1a2573 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -120,6 +120,14 @@ static struct mempolicy default_policy = {
 
 static struct mempolicy preferred_node_policy[MAX_NUMNODES];
 
+/* Returns true if the policy is the default policy */
+static bool mpol_is_default(struct mempolicy *pol)
+{
+	return !pol ||
+		pol == &default_policy ||
+		pol == &preferred_node_policy[numa_node_id()];
+}
+
 static struct mempolicy *get_task_policy(struct task_struct *p)
 {
 	struct mempolicy *pol = p->mempolicy;
@@ -2856,7 +2864,7 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol)
 	unsigned short mode = MPOL_DEFAULT;
 	unsigned short flags = 0;
 
-	if (pol && pol != &default_policy) {
+	if (!mpol_is_default(pol)) {
 		mode = pol->mode;
 		flags = pol->flags;
 	}

  parent reply	other threads:[~2014-01-27 11:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-26  3:12 [patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps David Rientjes
2014-01-26  3:12 ` David Rientjes
2014-01-27 10:50 ` Peter Zijlstra
2014-01-27 10:50   ` Peter Zijlstra
2014-01-27 13:09   ` Mel Gorman
2014-01-27 13:09     ` Mel Gorman
2014-01-28  0:13     ` David Rientjes
2014-01-28  0:13       ` David Rientjes
2014-01-27 11:03 ` Mel Gorman [this message]
2014-01-27 11:03   ` Mel Gorman
2014-01-27 23:31   ` David Rientjes
2014-01-27 23:31     ` David Rientjes
2014-01-28  8:57     ` Mel Gorman
2014-01-28  8:57       ` 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=20140127110330.GH4963@suse.de \
    --to=mgorman@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    --cc=torvalds@linux-foundation.org \
    /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.