RE: [PATCH] namei: revert old behaviour for filename_lookup with LOOKUP_PARENT flag
Thanks for the detailed explanation. I understand the reasoning now.
Thanks,
Vineeth
From: Al Viro [v...@ftp.linux.org.uk] on behalf of Al Viro
[v...@zeniv.linux.org.uk]
Sent: Thursday, October 13, 2016 4:31 PM
To: Remanan Pillai, Vineeth
Cc: Christoph Hellwig; linux-fsde...@vger.kernel.org;
linux-kernel@vger.kernel.org; Kamata, Munehisa; Liguori, Anthony
Subject: Re: [PATCH] namei: revert old behaviour for filename_lookup with
LOOKUP_PARENT flag
On Thu, Oct 13, 2016 at 03:14:23PM -0700, Vineeth Remanan Pillai wrote:
> Unfortunately, I also do not have enough context about the customer
> code that uses it. Since kern_path was exported function and the
> behavior changed across releases, this patch is just trying to revert
> to the old behavior.
> I clearly get what you are trying to say. It would have been really
> beneficial to get the context so as to understand use case and figure out
> alternative or better approach. But I think we should have the functionality
> as before and if kern_path is not the right API for this purpose, probably
> we
> should deprecate it in a phased manner.
kern_path() is not the right API for this purpose. Never had been. It is,
OTOH, the right API for other purposes, so it's not going to disappear.
If you check the history of mainline tree, you'll see no such call sites.
All way back to 2008 when kern_path() had been introduced, it had never
been given LOOKUP_PARENT in arguments:
; git log -p -Skern_path | grep ^\+.*\<kern_path\>
+ err = kern_path(mtd_dev, LOOKUP_FOLLOW, );
+ error = kern_path(pathname, LOOKUP_FOLLOW, );
+ ret = kern_path(filename, LOOKUP_FOLLOW, );
+ ret = kern_path(pathname->name, LOOKUP_FOLLOW, );
+ err = kern_path(name, LOOKUP_FOLLOW, path);
+ error = kern_path(journal_path, LOOKUP_FOLLOW, );
+ error = kern_path(journal_path, LOOKUP_FOLLOW, );
+ rc = kern_path(name, LOOKUP_FOLLOW, );
+ ret = kern_path(filename, LOOKUP_FOLLOW, );
+ status = kern_path(nfs4_recoverydir(), LOOKUP_FOLLOW, );
+static struct ctl_path kern_path[] = { { .procname = "kernel", }, { } };
+ register_sysctl_paths(kern_path, pid_ns_ctl_table);
+ r = kern_path(syspath, LOOKUP_FOLLOW, );
+ status = kern_path(recdir, LOOKUP_FOLLOW, );
+ /* Drop refcount obtained by kern_path(). */
+ rc = kern_path(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, );
+ ecryptfs_printk(KERN_WARNING, "kern_path() failed\n");
+ if (kern_path(tomoyo_loader, LOOKUP_FOLLOW, )) {
+ if (kern_path(dev_name, LOOKUP_FOLLOW, )) {
+ err = kern_path(mtd_dev, LOOKUP_FOLLOW, );
+ ret = kern_path(cache->rootdirname, LOOKUP_DIRECTORY, );
+ error = kern_path(pathname, LOOKUP_FOLLOW, );
+ err = kern_path(mountpoint, LOOKUP_FOLLOW, );
+ int err = kern_path(pathname, 0, );
+ err = kern_path(name, LOOKUP_FOLLOW, );
+ error = kern_path(name, LOOKUP_FOLLOW, );
+ error = kern_path(dev_name, LOOKUP_FOLLOW, );
+ if (kern_path(tomoyo_loader, LOOKUP_FOLLOW, )) {
+ if (pathname && kern_path(pathname, LOOKUP_FOLLOW, ) == 0) {
+ if (pathname && kern_path(pathname, 0, ) == 0) {
+ err = kern_path(tree->pathname, 0, );
+ err = kern_path(tree->pathname, 0, );
+ err = kern_path(new, 0, );
+ err = kern_path(old, 0, );
+ err = kern_path(tree->pathname, 0, );
+ error = kern_path(pathname, LOOKUP_FOLLOW, );
+ ret = kern_path(symname, LOOKUP_FOLLOW|LOOKUP_DIRECTORY, path);
+ rc = kern_path(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, );
+ err = kern_path(sunname->sun_path, LOOKUP_FOLLOW, );
+ err = kern_path(buf, 0, _path);
+ err = kern_path(nxp->ex_path, 0, );
+ err = kern_path(nxp->ex_path, 0, );
+ if (kern_path(name, 0, )) {
+ status = kern_path(rec_dirname, LOOKUP_FOLLOW | LOOKUP_DIRECTORY,
+ status = kern_path(recdir, LOOKUP_FOLLOW, );
+ error = kern_path(fo_path, 0, );
+ err = kern_path(buf, 0, _path);
+ error = kern_path(name, LOOKUP_FOLLOW, );
+ err = kern_path(name, LOOKUP_FOLLOW, );
+ err = kern_path(name, LOOKUP_FOLLOW, );
+ err = kern_path(name, LOOKUP_FOLLOW, );
+ err = kern_path(old_name, LOOKUP_FOLLOW, _path);
+ err = kern_path(old_name, LOOKUP_FOLLOW, _path);
+ retval = kern_path(dir_name, LOOKUP_FOLLOW, );
+int kern_path(const char *name, unsigned int flags, struct path *path)
+EXPORT_SYMBOL(kern_path);
+extern int kern_path(const char *, unsigned, struct path *);
;
As you can see, it had only been given LOOKUP_FOLLOW | LOOKUP_DIRECTORY,
LOOKUP_FOLLOW or 0. Old behaviour in a sense of kern_path() accepting
LOOKUP_PARENT is not going to co
RE: [PATCH] namei: revert old behaviour for filename_lookup with LOOKUP_PARENT flag
Thanks for the detailed explanation. I understand the reasoning now.
Thanks,
Vineeth
From: Al Viro [v...@ftp.linux.org.uk] on behalf of Al Viro
[v...@zeniv.linux.org.uk]
Sent: Thursday, October 13, 2016 4:31 PM
To: Remanan Pillai, Vineeth
Cc: Christoph Hellwig; linux-fsde...@vger.kernel.org;
linux-kernel@vger.kernel.org; Kamata, Munehisa; Liguori, Anthony
Subject: Re: [PATCH] namei: revert old behaviour for filename_lookup with
LOOKUP_PARENT flag
On Thu, Oct 13, 2016 at 03:14:23PM -0700, Vineeth Remanan Pillai wrote:
> Unfortunately, I also do not have enough context about the customer
> code that uses it. Since kern_path was exported function and the
> behavior changed across releases, this patch is just trying to revert
> to the old behavior.
> I clearly get what you are trying to say. It would have been really
> beneficial to get the context so as to understand use case and figure out
> alternative or better approach. But I think we should have the functionality
> as before and if kern_path is not the right API for this purpose, probably
> we
> should deprecate it in a phased manner.
kern_path() is not the right API for this purpose. Never had been. It is,
OTOH, the right API for other purposes, so it's not going to disappear.
If you check the history of mainline tree, you'll see no such call sites.
All way back to 2008 when kern_path() had been introduced, it had never
been given LOOKUP_PARENT in arguments:
; git log -p -Skern_path | grep ^\+.*\
+ err = kern_path(mtd_dev, LOOKUP_FOLLOW, );
+ error = kern_path(pathname, LOOKUP_FOLLOW, );
+ ret = kern_path(filename, LOOKUP_FOLLOW, );
+ ret = kern_path(pathname->name, LOOKUP_FOLLOW, );
+ err = kern_path(name, LOOKUP_FOLLOW, path);
+ error = kern_path(journal_path, LOOKUP_FOLLOW, );
+ error = kern_path(journal_path, LOOKUP_FOLLOW, );
+ rc = kern_path(name, LOOKUP_FOLLOW, );
+ ret = kern_path(filename, LOOKUP_FOLLOW, );
+ status = kern_path(nfs4_recoverydir(), LOOKUP_FOLLOW, );
+static struct ctl_path kern_path[] = { { .procname = "kernel", }, { } };
+ register_sysctl_paths(kern_path, pid_ns_ctl_table);
+ r = kern_path(syspath, LOOKUP_FOLLOW, );
+ status = kern_path(recdir, LOOKUP_FOLLOW, );
+ /* Drop refcount obtained by kern_path(). */
+ rc = kern_path(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, );
+ ecryptfs_printk(KERN_WARNING, "kern_path() failed\n");
+ if (kern_path(tomoyo_loader, LOOKUP_FOLLOW, )) {
+ if (kern_path(dev_name, LOOKUP_FOLLOW, )) {
+ err = kern_path(mtd_dev, LOOKUP_FOLLOW, );
+ ret = kern_path(cache->rootdirname, LOOKUP_DIRECTORY, );
+ error = kern_path(pathname, LOOKUP_FOLLOW, );
+ err = kern_path(mountpoint, LOOKUP_FOLLOW, );
+ int err = kern_path(pathname, 0, );
+ err = kern_path(name, LOOKUP_FOLLOW, );
+ error = kern_path(name, LOOKUP_FOLLOW, );
+ error = kern_path(dev_name, LOOKUP_FOLLOW, );
+ if (kern_path(tomoyo_loader, LOOKUP_FOLLOW, )) {
+ if (pathname && kern_path(pathname, LOOKUP_FOLLOW, ) == 0) {
+ if (pathname && kern_path(pathname, 0, ) == 0) {
+ err = kern_path(tree->pathname, 0, );
+ err = kern_path(tree->pathname, 0, );
+ err = kern_path(new, 0, );
+ err = kern_path(old, 0, );
+ err = kern_path(tree->pathname, 0, );
+ error = kern_path(pathname, LOOKUP_FOLLOW, );
+ ret = kern_path(symname, LOOKUP_FOLLOW|LOOKUP_DIRECTORY, path);
+ rc = kern_path(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, );
+ err = kern_path(sunname->sun_path, LOOKUP_FOLLOW, );
+ err = kern_path(buf, 0, _path);
+ err = kern_path(nxp->ex_path, 0, );
+ err = kern_path(nxp->ex_path, 0, );
+ if (kern_path(name, 0, )) {
+ status = kern_path(rec_dirname, LOOKUP_FOLLOW | LOOKUP_DIRECTORY,
+ status = kern_path(recdir, LOOKUP_FOLLOW, );
+ error = kern_path(fo_path, 0, );
+ err = kern_path(buf, 0, _path);
+ error = kern_path(name, LOOKUP_FOLLOW, );
+ err = kern_path(name, LOOKUP_FOLLOW, );
+ err = kern_path(name, LOOKUP_FOLLOW, );
+ err = kern_path(name, LOOKUP_FOLLOW, );
+ err = kern_path(old_name, LOOKUP_FOLLOW, _path);
+ err = kern_path(old_name, LOOKUP_FOLLOW, _path);
+ retval = kern_path(dir_name, LOOKUP_FOLLOW, );
+int kern_path(const char *name, unsigned int flags, struct path *path)
+EXPORT_SYMBOL(kern_path);
+extern int kern_path(const char *, unsigned, struct path *);
;
As you can see, it had only been given LOOKUP_FOLLOW | LOOKUP_DIRECTORY,
LOOKUP_FOLLOW or 0. Old behaviour in a sense of kern_path() accepting
LOOKUP_PARENT is not going to come back - it wou
