linux-kernel - Re: [PATCH 3/5] tmpfs: handle MPOL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1268877338.4773.151.camel@useless.americas.hpqcorp.net>
Date:	Wed, 17 Mar 2010 21:55:38 -0400
From:	Lee Schermerhorn <Lee.Schermerhorn@...com>
To:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc:	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	LKML <linux-kernel@...r.kernel.org>, kiran@...lex86.org,
	cl@...ux-foundation.org, mel@....ul.ie, stable@...nel.org,
	linux-mm <linux-mm@...ck.org>, akpm@...ux-foundation.org
Subject: Re: [PATCH 3/5] tmpfs: handle MPOL_LOCAL mount option properly

On Thu, 2010-03-18 at 08:52 +0900, KOSAKI Motohiro wrote:
> > On Tue, 16 Mar 2010, KOSAKI Motohiro wrote:
> > 
> > > commit 71fe804b6d5 (mempolicy: use struct mempolicy pointer in
> > > shmem_sb_info) added mpol=local mount option. but its feature is
> > > broken since it was born. because such code always return 1 (i.e.
> > > mount failure).
> > > 
> > > This patch fixes it.
> > > 
> > > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> > > Cc: Ravikiran Thirumalai <kiran@...lex86.org>
> > 
> > Thank you both for finding and fixing these mpol embarrassments.
> > 
> > But if this "mpol=local" feature was never documented (not even in the
> > commit log), has been broken since birth 20 months ago, and nobody has
> > noticed: wouldn't it be better to save a little bloat and just rip it out?
> 
> I have no objection if lee agreed, lee?
> Of cource, if we agree it, we can make the new patch soon :)
> 

Well, given the other issues with mpol_parse_str(), I suspect the entire
tmpfs mpol option is not used all that often in the mainline kernel.  I
recall that this feature was introduced by SGI for some of their
customers who may depend on it.  There have been cases where I could
have used it were it supported for the SYSV shm and MAP_ANON_MAP_SHARED
internal tmpfs mount.  Further, note that the addition of "mpol=local"
occurred between when the major "enterprise distributions" selected a
new mainline kernel.  Production users of those distros, who are the
likely users of this feature, tend not to live on the bleeding edge.
So, maybe we shouldn't read too much into it not being discovered until
now.

That being said, I suppose I wouldn't be all that opposed to deprecating
the entire tmpfs mpol option, and see who yells.   If the mpol mount
options stays, I'd like to see the 'local' option stay.  It's a
legitimate behavior that one can specify via the system calls and see
via numa_maps, so I think the mpol mount option, if it exists, should
support a way to specify it.  

As for bloat, the additional code on the mount option side to support it
is:

        case MPOL_LOCAL:
                /*
                 * Don't allow a nodelist;  mpol_new() checks flags
                 */
                if (nodelist)
                        goto out;
                mode = MPOL_PREFERRED;
                break;

Lee

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