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/

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/

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/

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/

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/