Message-ID: <alpine.DEB.1.00.0803081403460.12095@chino.kir.corp.google.com>
Date:	Sat, 8 Mar 2008 14:09:11 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	Lee Schermerhorn <Lee.Schermerhorn@...com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Paul Jackson <pj@....com>,
	Christoph Lameter <clameter@....com>, Andi Kleen <ak@...e.de>,
	linux-kernel@...r.kernel.org, linux-mm <linux-mm@...ck.org>,
	Eric Whitney <eric.whitney@...com>
Subject: Re: Regression:  Re: [patch -mm 2/4] mempolicy: create mempolicy_operations
 structure

On Sat, 8 Mar 2008, Lee Schermerhorn wrote:

> > Excuse me, but there was significant discussion about this on LKML and I 
> > eventually did force MPOL_DEFAULT to require a non-empty nodemask 

Correction: s/non-empty/empty

> > specifically because of your demand that it should.  It didn't originally 
> > require this in my patchset, and now you're removing the exact same 
> > requirement that you demanded.
> > 
> > You said on February 13:
> > 
> > 	1) we've discussed the issue of returning EINVAL for non-empty
> > 	nodemasks with MPOL_DEFAULT.  By removing this restriction, we run
> > 	the risk of breaking applications if we should ever want to define
> > 	a semantic to non-empty node mask for MPOL_DEFAULT.
> > 
> > If you want to remove this requirement now (please get agreement from 
> > Paul) and are sure of your position, you'll at least need an update to 
> > Documentation/vm/numa-memory-policy.txt.
> 
> Excuse me.  I thought that the discussion--my position, anyway--was
> about preserving existing behavior for MPOL_DEFAULT which is to require
> an EMPTY [or NULL--same effect] nodemask.  Not a NON-EMPTY one.  See:
> http://www.kernel.org/doc/man-pages/online/pages/man2/set_mempolicy.2.html
> It does appear that your patches now require a non-empty nodemask.  This
> was intentional?
> 

The first and second set did not have this requirement, but the third set 
does (not currently in -mm), so I've changed it back.  Hopefully there's 
no confusion and we can settle on a solution without continuously 
revisiting the topic.

My position was originally to allow any type of nodemask to be passed with 
MPOL_DEFAULT since its not used.  You asked for strict argument checking 
and so after some debate I changed it to require an empty nodemask mainly 
because I didn't want the patchset to stall on such a minor point.  But in 
your regression fix, you expressed the desire once again to allow it to 
accept any nodemask because the testsuite does not check for it.

So if you'd like to do that, I'd encourage you to submit it as a separate 
patch and open it up for review.

What is currently in -mm and what I will be posting shortly is the updated 
regression fix.  All of these patches require that MPOL_DEFAULT include a 
NULL pointer or empty nodemask passed via the two syscalls.

> Note:  in the subject patch, I didn't enforce this behavior because your
> patch didn't [it enforced just the opposite], and I've pretty much given
> up.  Although I prefer current behavior [before your series], if we
> change it, we will need to change the man pages to remove the error
> condition for non-empty nodemasks with MPOL_DEFAULT.
> 

With my patches it still requires a NULL pointer or empty nodemask and 
I've updated Documentation/vm/numa_memory_policy.txt to explicitly say its 
an error if a non-empty nodemask is passed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists