[patch 00/10] Use struct path in struct nameidata - Kernel

This is a discussion on [patch 00/10] Use struct path in struct nameidata - Kernel ; On Fri, Sep 28, 2007 at 10:43:50PM +0200, Andreas Gruenbacher wrote: > Here is another cleanup on top of Jan's set. Comments? > > > The name path_put_conditional (formerly, dput_path) is a little unclear. > Replace (path_put_conditional + path_put) with ...

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2
Results 21 to 26 of 26

Thread: [patch 00/10] Use struct path in struct nameidata

  1. Re: [patch] Combine path_put and path_put_conditional

    On Fri, Sep 28, 2007 at 10:43:50PM +0200, Andreas Gruenbacher wrote:
    > Here is another cleanup on top of Jan's set. Comments?
    >
    >
    > The name path_put_conditional (formerly, dput_path) is a little unclear.
    > Replace (path_put_conditional + path_put) with path_walk_put_both,
    > "put a pair of paths after a path_walk" (see the kerneldoc).


    I think this function is a good idea. The name seems odd to me, but
    I don't really have a better idea either.

    > +static void path_walk_put_both(struct path *path1, struct path *path2)
    > +{
    > + dput(path1->dentry);
    > + dput(path2->dentry);
    > + mntput(path1->mnt);
    > + if (path1->mnt != path2->mnt)
    > + mntput(path2->mnt);
    > }


    Why do you opencode the path_put for path1?

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [patch 09/10] Use struct path in fs_struct

    On Fri, Sep 28, 2007 at 10:39:48PM +0200, Andreas Gruenbacher wrote:
    > On Friday 28 September 2007 20:42, Christoph Hellwig wrote:
    > > __d_path should probably switch to taking a struct path * aswell.

    >
    > Indeed, it now easily can. Here we go...
    >
    >
    > One less parameter to __d_path
    >
    > All callers to __d_path pass the dentry and vfsmount of a struct
    > path to __d_path. Pass the struct path directly, instead.


    Looks good. If you have some sparse time left the dentry and vfsmnt
    arguments of __d_path and d_path should probably be switched over
    to a struct path aswell. For about half of the callers that works
    out easily because they have a struct file, and some need some
    reshuffling (e.g. /proc/ symlink code or the dcookies that want to
    store a struct path aswell)

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [patch 09/10] Use struct path in fs_struct

    On Sat, Sep 29, 2007 at 11:24:26AM +0200, Christoph Hellwig wrote:
    > On Fri, Sep 28, 2007 at 10:39:48PM +0200, Andreas Gruenbacher wrote:
    > > On Friday 28 September 2007 20:42, Christoph Hellwig wrote:
    > > > __d_path should probably switch to taking a struct path * aswell.

    > >
    > > Indeed, it now easily can. Here we go...
    > >
    > >
    > > One less parameter to __d_path
    > >
    > > All callers to __d_path pass the dentry and vfsmount of a struct
    > > path to __d_path. Pass the struct path directly, instead.

    >
    > Looks good. If you have some sparse time left the dentry and vfsmnt
    > arguments of __d_path and d_path should probably be switched over
    > to a struct path aswell. For about half of the callers that works
    > out easily because they have a struct file, and some need some
    > reshuffling (e.g. /proc/ symlink code or the dcookies that want to
    > store a struct path aswell)


    And thinking about it the function should also grow a better name,
    say print_path. And if you really touch it moving the kerneldoc
    comment to d_path from __d_path would also be very useful.
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [patch] Combine path_put and path_put_conditional

    On Fri, Sep 28, Andreas Gruenbacher wrote:

    > The name path_put_conditional (formerly, dput_path) is a little unclear.
    > Replace (path_put_conditional + path_put) with path_walk_put_both,
    > "put a pair of paths after a path_walk" (see the kerneldoc).


    Hmm, I don't know. To put both the nd and path is at the moment only used in
    some error paths. I have another series of patches pending which is using
    path_put_conditional outside of error paths. So please don't remove
    it. Besides that the naming completely hides that the conditional release of
    the vfsmount reference. Besides that I would name it path_put_both() just to
    make it more "beautiful" wrt the other path_put*() functions.

    > @@ -996,8 +1006,8 @@ return_reval:
    > return_base:
    > return 0;
    > out_dput:
    > - path_put_conditional(&next, nd);
    > - break;
    > + path_walk_put_both(&next, &nd->path);
    > + goto return_err;
    > }
    > path_put(&nd->path);
    > return_err:
    > @@ -1777,11 +1787,15 @@ ok:
    > return 0;
    >
    > exit_dput:
    > - path_put_conditional(&path, nd);
    > + path_walk_put_both(&path, &nd->path);
    > + goto exit_intent;
    > +
    > exit:
    > + path_put(&nd->path);
    > +
    > +exit_intent:
    > if (!IS_ERR(nd->intent.open.file))
    > release_open_intent(nd);
    > - path_put(&nd->path);
    > return error;
    >
    > do_link:


    IMHO introducing another label just to use it here isn't worth the change.
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [patch] Combine path_put and path_put_conditional

    Hi Andreas,

    On Friday 28 September 2007, Andreas Gruenbacher wrote:
    > The name path_put_conditional (formerly, dput_path) is a little unclear.
    > Replace (path_put_conditional + path_put) with path_walk_put_both,
    > "put a pair of paths after a path_walk" (see the kerneldoc).

    ^^^^^^^^^^^^^

    So why not name it path_walk_put_pair() then?

    Rationale: "_both" is just counting, "_pair"
    means they are related somehow.


    Best regards

    Ingo Oeser
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [patch 00/10] Use struct path in struct nameidata

    Any reasons this didn't go into -mm? It's all straight-forward cleanup
    and it would be a pity not to see it in 2.6.24

    On Thu, Sep 27, 2007 at 04:12:00PM +0200, jblunck@suse.de wrote:
    > This is a respin of the patch series Andreas posted last month. It leaves out
    > the restructuring of the intent which will be done at a later point in time.
    >
    > There are three preparing patches that remove unneeded code IMHO. I haven't
    > got feedback from Takashi since he is on holiday. Please, can somebody else
    > acknowledge the changes?
    >
    > Comments?
    > Jan
    > --

    ---end quoted text---
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2