[patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps

[patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps

[David Rientjes]

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>
---
 This affects all 3.7+ kernels and is intended for stable but will need to
 be rebased after it's merged since mpol_to_str() has subsequently
 changed.  I'll rebase and propose it separately.

 mm/mempolicy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2926,7 +2926,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 (pol && pol != &default_policy && !(pol->flags & MPOL_F_MORON)) {
 		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>

Re: [patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps

[Peter Zijlstra]

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.

Should we also consider printing the MOF and MORON states so we get a
better view of what the actual policy is?


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

Re: [patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps

[Mel Gorman]

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>

Re: [patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps

[Mel Gorman]

On Mon, Jan 27, 2014 at 11:50:11AM +0100, Peter Zijlstra wrote:
> 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.
> 
> Should we also consider printing the MOF and MORON states so we get a
> better view of what the actual policy is?
> 

MOF and MORON are separate issues because MOF is exposed to the userspace
API but not the policies that make up MORON. For MORON, I concluded that
we should not expose that via numa_maps unless it can be controlled from
userspace.

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

Re: [patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps

[David Rientjes]

On Mon, 27 Jan 2014, Mel Gorman wrote:

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

I was trying to avoid doing this because numa_node_id() of process A 
reading numa_maps for process B has nothing to do with the policy of the 
process A and I thought MPOL_F_MORON's purpose was exactly for what it is 
used for today.  It works today since you initialize preferred_node_policy 
for all nodes, but could this ever change to only be valid for N_MEMORY 
node states, for example?

I'm not sure what the harm in updating mpol_to_str() would be if 
MPOL_F_MORON is to change in the future?

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

Re: [patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps

[David Rientjes]

On Mon, 27 Jan 2014, Mel Gorman wrote:

> > Should we also consider printing the MOF and MORON states so we get a
> > better view of what the actual policy is?
> > 
> 
> MOF and MORON are separate issues because MOF is exposed to the userspace
> API but not the policies that make up MORON. For MORON, I concluded that
> we should not expose that via numa_maps unless it can be controlled from
> userspace.
> 

We print the possible flags for set_mempolicy() which is pretty simple 
since there's only one possible flag at any given time, but we don't print 
any possible flags for mbind().  If we are to start printing lazy 
migrate-on-fault then we should probably print the other flags as well, I 
just don't know how long we want lines in numa_maps to be.

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

Re: [patch for-3.14] mm, mempolicy: fix mempolicy printing in numa_maps

[Mel Gorman]

On Mon, Jan 27, 2014 at 03:31:32PM -0800, David Rientjes wrote:
> On Mon, 27 Jan 2014, Mel Gorman wrote:
> 
> > 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;
> 
> I was trying to avoid doing this because numa_node_id() of process A 
> reading numa_maps for process B has nothing to do with the policy of the 
> process A and I thought MPOL_F_MORON's purpose was exactly for what it is 
> used for today. It works today since you initialize preferred_node_policy 
> for all nodes, but could this ever change to only be valid for N_MEMORY 
> node states, for example?
> 

You're right about the numa_node_id() usage, I should have called
task_node(p) to read the node it's currently running but that is potentially
obscure for different reasons.

> I'm not sure what the harm in updating mpol_to_str() would be if 
> MPOL_F_MORON is to change in the future?

It just has to be caught correctly and handled and it's a little non-obvious
but ok if I see a patch that modifies how MPOL_F_MORON is used in the
future I should remember to check for this.  I withdraw my objection for
your patch so

Acked-by: Mel Gorman <mgorman@suse.de>

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