lkml.org 
[lkml]   [2010]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 3/5] tmpfs: handle MPOL_LOCAL mount option properly
    From
    Date
    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@jp.fujitsu.com>
    > > > Cc: Ravikiran Thirumalai <kiran@scalex86.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





    \
     
     \ /
      Last update: 2010-03-18 02:57    [W:0.034 / U:30.052 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site