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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch] Combine path_put and path_put_conditional
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). Signed-off-by: Andreas Gruenbacher <[EMAIL PROTECTED]> Index: linux-2.6/fs/namei.c === --- linux-2.6.orig/fs/namei.c +++ linux-2.6/fs/namei.c @@ -582,11 +582,22 @@ fail: return PTR_ERR(link); } -static void path_put_conditional(struct path *path, struct nameidata *nd) -{ - dput(path->dentry); - if (path->mnt != nd->path.mnt) - mntput(path->mnt); +/** + * path_walk_put_both - put a pair of paths after a path_walk + * @path1: first path to put + * @path2: second path to put + * + * When walking a path we keep the same vfsmnt reference while on the same + * filesystem, and grab a reference to the new vfsmnt when crossing mount + * points. Put both @path1 and @path2 under this assumption. + */ +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); } static inline void path_to_nameidata(struct path *path, struct nameidata *nd) @@ -654,8 +665,7 @@ static inline int do_follow_link(struct nd->depth--; return err; loop: - path_put_conditional(path, nd); - path_put(&nd->path); + path_walk_put_both(path, &nd->path); return err; } @@ -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: - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/