* [RFC PATCH v2] ovl: suppress negative dentry in lookup
@ 2020-05-12 7:13 Chengguang Xu
2020-05-12 7:50 ` Amir Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: Chengguang Xu @ 2020-05-12 7:13 UTC (permalink / raw)
To: miklos, amir73il; +Cc: linux-unionfs, Chengguang Xu
When a file is only in a lower layer or in no layer at all, after
lookup a negative dentry will be generated in the upper layer or
even worse many negetive dentries will be generated in upper/lower
layers. These negative dentries will be useless after construction
of overlayfs' own dentry and may keep in the memory long time even
after unmount of overlayfs instance. This patch tries to kill
unnecessary negative dentry during lookup.
Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
---
v1->v2:
- Only drop negative dentry after slow lookup.
fs/namei.c | 9 ++++++---
fs/overlayfs/namei.c | 35 ++++++++++++++++++++++++++++++++++-
include/linux/namei.h | 8 ++++++++
3 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index a320371899cf..1cc2960c7804 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1386,7 +1386,7 @@ static inline int handle_mounts(struct nameidata *nd, struct dentry *dentry,
* This looks up the name in dcache and possibly revalidates the found dentry.
* NULL is returned if the dentry does not exist in the cache.
*/
-static struct dentry *lookup_dcache(const struct qstr *name,
+struct dentry *lookup_dcache(const struct qstr *name,
struct dentry *dir,
unsigned int flags)
{
@@ -1402,6 +1402,7 @@ static struct dentry *lookup_dcache(const struct qstr *name,
}
return dentry;
}
+EXPORT_SYMBOL(lookup_dcache);
/*
* Parent directory has inode locked exclusive. This is one
@@ -1500,7 +1501,7 @@ static struct dentry *lookup_fast(struct nameidata *nd,
}
/* Fast lookup failed, do it the slow way */
-static struct dentry *__lookup_slow(const struct qstr *name,
+struct dentry *__lookup_slow(const struct qstr *name,
struct dentry *dir,
unsigned int flags)
{
@@ -1536,6 +1537,7 @@ static struct dentry *__lookup_slow(const struct qstr *name,
}
return dentry;
}
+EXPORT_SYMBOL(__lookup_slow);
static struct dentry *lookup_slow(const struct qstr *name,
struct dentry *dir,
@@ -2460,7 +2462,7 @@ int vfs_path_lookup(struct dentry *dentry, struct vfsmount *mnt,
}
EXPORT_SYMBOL(vfs_path_lookup);
-static int lookup_one_len_common(const char *name, struct dentry *base,
+int lookup_one_len_common(const char *name, struct dentry *base,
int len, struct qstr *this)
{
this->name = name;
@@ -2491,6 +2493,7 @@ static int lookup_one_len_common(const char *name, struct dentry *base,
return inode_permission(base->d_inode, MAY_EXEC);
}
+EXPORT_SYMBOL(lookup_one_len_common);
/**
* try_lookup_one_len - filesystem helper to lookup single pathname component
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 723d17744758..d8e71173ea75 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -191,6 +191,39 @@ static bool ovl_is_opaquedir(struct dentry *dentry)
return ovl_check_dir_xattr(dentry, OVL_XATTR_OPAQUE);
}
+static struct dentry *ovl_lookup_positive_unlocked(const char *name,
+ struct dentry *base,
+ int len)
+{
+ struct qstr this;
+ struct dentry *ret;
+ bool not_found = false;
+ int err;
+
+ err = lookup_one_len_common(name, base, len, &this);
+ if (err)
+ return ERR_PTR(err);
+
+ ret = lookup_dcache(&this, base, 0);
+ if (ret)
+ return ret;
+
+ inode_lock_shared(base->d_inode);
+ ret = __lookup_slow(&this, base, 0);
+ if (!IS_ERR(ret) &&
+ d_flags_negative(ret->d_flags)) {
+ not_found = true;
+ d_drop(ret);
+ }
+ inode_unlock_shared(base->d_inode);
+
+ if (not_found) {
+ dput(ret);
+ ret = ERR_PTR(-ENOENT);
+ }
+ return ret;
+}
+
static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
const char *name, unsigned int namelen,
size_t prelen, const char *post,
@@ -200,7 +233,7 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
int err;
bool last_element = !post[0];
- this = lookup_positive_unlocked(name, base, namelen);
+ this = ovl_lookup_positive_unlocked(name, base, namelen);
if (IS_ERR(this)) {
err = PTR_ERR(this);
this = NULL;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index a4bb992623c4..c65b863657eb 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -66,6 +66,14 @@ extern struct dentry *user_path_create(int, const char __user *, struct path *,
extern void done_path_create(struct path *, struct dentry *);
extern struct dentry *kern_path_locked(const char *, struct path *);
+extern int lookup_one_len_common(const char *name, struct dentry *base,
+ int len, struct qstr *this);
+extern struct dentry *lookup_dcache(const struct qstr *name,
+ struct dentry *base,
+ unsigned int flags);
+extern struct dentry *__lookup_slow(const struct qstr *name,
+ struct dentry *dir,
+ unsigned int flags);
extern struct dentry *try_lookup_one_len(const char *, struct dentry *, int);
extern struct dentry *lookup_one_len(const char *, struct dentry *, int);
extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH v2] ovl: suppress negative dentry in lookup
2020-05-12 7:13 [RFC PATCH v2] ovl: suppress negative dentry in lookup Chengguang Xu
@ 2020-05-12 7:50 ` Amir Goldstein
2020-05-12 8:32 ` Miklos Szeredi
0 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2020-05-12 7:50 UTC (permalink / raw)
To: Chengguang Xu; +Cc: Miklos Szeredi, overlayfs
On Tue, May 12, 2020 at 10:13 AM Chengguang Xu <cgxu519@mykernel.net> wrote:
>
> When a file is only in a lower layer or in no layer at all, after
> lookup a negative dentry will be generated in the upper layer or
> even worse many negetive dentries will be generated in upper/lower
> layers. These negative dentries will be useless after construction
> of overlayfs' own dentry and may keep in the memory long time even
> after unmount of overlayfs instance. This patch tries to kill
> unnecessary negative dentry during lookup.
>
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
> ---
> v1->v2:
> - Only drop negative dentry after slow lookup.
>
> fs/namei.c | 9 ++++++---
> fs/overlayfs/namei.c | 35 ++++++++++++++++++++++++++++++++++-
> include/linux/namei.h | 8 ++++++++
> 3 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index a320371899cf..1cc2960c7804 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1386,7 +1386,7 @@ static inline int handle_mounts(struct nameidata *nd, struct dentry *dentry,
> * This looks up the name in dcache and possibly revalidates the found dentry.
> * NULL is returned if the dentry does not exist in the cache.
> */
> -static struct dentry *lookup_dcache(const struct qstr *name,
> +struct dentry *lookup_dcache(const struct qstr *name,
> struct dentry *dir,
> unsigned int flags)
> {
> @@ -1402,6 +1402,7 @@ static struct dentry *lookup_dcache(const struct qstr *name,
> }
> return dentry;
> }
> +EXPORT_SYMBOL(lookup_dcache);
>
> /*
> * Parent directory has inode locked exclusive. This is one
> @@ -1500,7 +1501,7 @@ static struct dentry *lookup_fast(struct nameidata *nd,
> }
>
> /* Fast lookup failed, do it the slow way */
> -static struct dentry *__lookup_slow(const struct qstr *name,
> +struct dentry *__lookup_slow(const struct qstr *name,
> struct dentry *dir,
> unsigned int flags)
> {
> @@ -1536,6 +1537,7 @@ static struct dentry *__lookup_slow(const struct qstr *name,
> }
> return dentry;
> }
> +EXPORT_SYMBOL(__lookup_slow);
>
> static struct dentry *lookup_slow(const struct qstr *name,
> struct dentry *dir,
> @@ -2460,7 +2462,7 @@ int vfs_path_lookup(struct dentry *dentry, struct vfsmount *mnt,
> }
> EXPORT_SYMBOL(vfs_path_lookup);
>
> -static int lookup_one_len_common(const char *name, struct dentry *base,
> +int lookup_one_len_common(const char *name, struct dentry *base,
> int len, struct qstr *this)
> {
> this->name = name;
> @@ -2491,6 +2493,7 @@ static int lookup_one_len_common(const char *name, struct dentry *base,
>
> return inode_permission(base->d_inode, MAY_EXEC);
> }
> +EXPORT_SYMBOL(lookup_one_len_common);
>
> /**
> * try_lookup_one_len - filesystem helper to lookup single pathname component
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index 723d17744758..d8e71173ea75 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -191,6 +191,39 @@ static bool ovl_is_opaquedir(struct dentry *dentry)
> return ovl_check_dir_xattr(dentry, OVL_XATTR_OPAQUE);
> }
>
> +static struct dentry *ovl_lookup_positive_unlocked(const char *name,
> + struct dentry *base,
> + int len)
> +{
> + struct qstr this;
> + struct dentry *ret;
> + bool not_found = false;
> + int err;
> +
> + err = lookup_one_len_common(name, base, len, &this);
> + if (err)
> + return ERR_PTR(err);
> +
> + ret = lookup_dcache(&this, base, 0);
> + if (ret)
> + return ret;
> +
> + inode_lock_shared(base->d_inode);
> + ret = __lookup_slow(&this, base, 0);
> + if (!IS_ERR(ret) &&
> + d_flags_negative(ret->d_flags)) {
> + not_found = true;
> + d_drop(ret);
> + }
> + inode_unlock_shared(base->d_inode);
> +
> + if (not_found) {
> + dput(ret);
> + ret = ERR_PTR(-ENOENT);
> + }
> + return ret;
> +}
> +
I think there was a misunderstanding.
This helper should be in vfs code, not duplicating vfs code
and please don't duplicate code in vfs either.
I think you can use a lookup flag (LOOKUP_POSITIVE_CACHE???)
to describe the desired behavior and implement it inside
lookup_slow(). Document the semantics as well as explain
in the context of the helper the cases where modules might
find this useful (because they have higher level caches).
Besides the fact that this helper really needs review by Al
and that duplicating subtle code is wrong in so many levels,
I suppose the functionality could prove useful to other subsystems
as well.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH v2] ovl: suppress negative dentry in lookup
2020-05-12 7:50 ` Amir Goldstein
@ 2020-05-12 8:32 ` Miklos Szeredi
2020-05-12 8:55 ` Amir Goldstein
0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2020-05-12 8:32 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Chengguang Xu, overlayfs, Al Viro, linux-fsdevel
On Tue, May 12, 2020 at 10:50:31AM +0300, Amir Goldstein wrote:
> This helper should be in vfs code, not duplicating vfs code
> and please don't duplicate code in vfs either.
>
> I think you can use a lookup flag (LOOKUP_POSITIVE_CACHE???)
> to describe the desired behavior and implement it inside
> lookup_slow(). Document the semantics as well as explain
> in the context of the helper the cases where modules might
> find this useful (because they have higher level caches).
>
> Besides the fact that this helper really needs review by Al
> and that duplicating subtle code is wrong in so many levels,
> I suppose the functionality could prove useful to other subsystems
> as well.
Something like this (untested). Needs splitup and changelogs.
Thanks,
Miklos
---
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index c31f362fa098..e52a3b35ebac 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -752,7 +752,7 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
while (*s && *s != sep)
s++;
- child = lookup_positive_unlocked(p, dentry, s - p);
+ child = lookup_positive_unlocked(p, dentry, s - p, 0);
dput(dentry);
dentry = child;
} while (!IS_ERR(dentry));
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b7f2e971ecbc..df4f37a6a9ab 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -299,7 +299,7 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent)
if (!parent)
parent = debugfs_mount->mnt_root;
- dentry = lookup_positive_unlocked(name, parent, strlen(name));
+ dentry = lookup_positive_unlocked(name, parent, strlen(name), 0);
if (IS_ERR(dentry))
return NULL;
return dentry;
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index e23752d9a79f..e39af6313ad9 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -407,7 +407,7 @@ static struct dentry *ecryptfs_lookup(struct inode *ecryptfs_dir_inode,
name = encrypted_and_encoded_name;
}
- lower_dentry = lookup_one_len_unlocked(name, lower_dir_dentry, len);
+ lower_dentry = lookup_one_len_unlocked(name, lower_dir_dentry, len, 0);
if (IS_ERR(lower_dentry)) {
ecryptfs_printk(KERN_DEBUG, "%s: lookup_one_len() returned "
"[%ld] on lower_dentry = [%s]\n", __func__,
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 2dd55b172d57..a4276d14aebb 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -145,7 +145,7 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
if (err)
goto out_err;
dprintk("%s: found name: %s\n", __func__, nbuf);
- tmp = lookup_one_len_unlocked(nbuf, parent, strlen(nbuf));
+ tmp = lookup_one_len_unlocked(nbuf, parent, strlen(nbuf), 0);
if (IS_ERR(tmp)) {
dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
err = PTR_ERR(tmp);
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index 9dc7e7a64e10..92e7f264baa1 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -224,7 +224,7 @@ struct dentry *kernfs_node_dentry(struct kernfs_node *kn,
return ERR_PTR(-EINVAL);
}
dtmp = lookup_positive_unlocked(kntmp->name, dentry,
- strlen(kntmp->name));
+ strlen(kntmp->name), 0);
dput(dentry);
if (IS_ERR(dtmp))
return dtmp;
diff --git a/fs/namei.c b/fs/namei.c
index a320371899cf..e70b7a14bdcc 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1532,6 +1532,9 @@ static struct dentry *__lookup_slow(const struct qstr *name,
if (unlikely(old)) {
dput(dentry);
dentry = old;
+ } else if ((flags & LOOKUP_NO_NEGATIVE) &&
+ d_is_negative(dentry)) {
+ d_drop(dentry);
}
}
return dentry;
@@ -2562,7 +2565,8 @@ EXPORT_SYMBOL(lookup_one_len);
* i_mutex held, and will take the i_mutex itself if necessary.
*/
struct dentry *lookup_one_len_unlocked(const char *name,
- struct dentry *base, int len)
+ struct dentry *base, int len,
+ unsigned int flags)
{
struct qstr this;
int err;
@@ -2572,9 +2576,9 @@ struct dentry *lookup_one_len_unlocked(const char *name,
if (err)
return ERR_PTR(err);
- ret = lookup_dcache(&this, base, 0);
+ ret = lookup_dcache(&this, base, flags);
if (!ret)
- ret = lookup_slow(&this, base, 0);
+ ret = lookup_slow(&this, base, flags);
return ret;
}
EXPORT_SYMBOL(lookup_one_len_unlocked);
@@ -2588,9 +2592,10 @@ EXPORT_SYMBOL(lookup_one_len_unlocked);
* this one avoids such problems.
*/
struct dentry *lookup_positive_unlocked(const char *name,
- struct dentry *base, int len)
+ struct dentry *base, int len,
+ unsigned int flags)
{
- struct dentry *ret = lookup_one_len_unlocked(name, base, len);
+ struct dentry *ret = lookup_one_len_unlocked(name, base, len, flags);
if (!IS_ERR(ret) && d_flags_negative(smp_load_acquire(&ret->d_flags))) {
dput(ret);
ret = ERR_PTR(-ENOENT);
diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index aae514d40b64..19628922969c 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -855,7 +855,7 @@ compose_entry_fh(struct nfsd3_readdirres *cd, struct svc_fh *fhp,
} else
dchild = dget(dparent);
} else
- dchild = lookup_positive_unlocked(name, dparent, namlen);
+ dchild = lookup_positive_unlocked(name, dparent, namlen, 0);
if (IS_ERR(dchild))
return rv;
if (d_mountpoint(dchild))
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 996ac01ee977..0c3c7928a319 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3066,7 +3066,7 @@ nfsd4_encode_dirent_fattr(struct xdr_stream *xdr, struct nfsd4_readdir *cd,
__be32 nfserr;
int ignore_crossmnt = 0;
- dentry = lookup_positive_unlocked(name, cd->rd_fhp->fh_dentry, namlen);
+ dentry = lookup_positive_unlocked(name, cd->rd_fhp->fh_dentry, namlen, 0);
if (IS_ERR(dentry))
return nfserrno(PTR_ERR(dentry));
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 0db23baf98e7..193857487060 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -200,7 +200,8 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
int err;
bool last_element = !post[0];
- this = lookup_positive_unlocked(name, base, namelen);
+ this = lookup_positive_unlocked(name, base, namelen,
+ LOOKUP_NO_NEGATIVE);
if (IS_ERR(this)) {
err = PTR_ERR(this);
this = NULL;
@@ -657,7 +658,7 @@ struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh)
if (err)
return ERR_PTR(err);
- index = lookup_positive_unlocked(name.name, ofs->indexdir, name.len);
+ index = lookup_positive_unlocked(name.name, ofs->indexdir, name.len, 0);
kfree(name.name);
if (IS_ERR(index)) {
if (PTR_ERR(index) == -ENOENT)
@@ -689,7 +690,7 @@ struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
if (err)
return ERR_PTR(err);
- index = lookup_positive_unlocked(name.name, ofs->indexdir, name.len);
+ index = lookup_positive_unlocked(name.name, ofs->indexdir, name.len, 0);
if (IS_ERR(index)) {
err = PTR_ERR(index);
if (err == -ENOENT) {
@@ -1137,7 +1138,7 @@ bool ovl_lower_positive(struct dentry *dentry)
struct dentry *lowerdir = poe->lowerstack[i].dentry;
this = lookup_positive_unlocked(name->name, lowerdir,
- name->len);
+ name->len, 0);
if (IS_ERR(this)) {
switch (PTR_ERR(this)) {
case -ENOENT:
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index b6a4f692d345..f588839ebe2e 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2488,7 +2488,7 @@ int dquot_quota_on_mount(struct super_block *sb, char *qf_name,
struct dentry *dentry;
int error;
- dentry = lookup_positive_unlocked(qf_name, sb->s_root, strlen(qf_name));
+ dentry = lookup_positive_unlocked(qf_name, sb->s_root, strlen(qf_name), 0);
if (IS_ERR(dentry))
return PTR_ERR(dentry);
diff --git a/include/linux/namei.h b/include/linux/namei.h
index a4bb992623c4..4896eeeeea46 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -49,6 +49,8 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
/* LOOKUP_* flags which do scope-related checks based on the dirfd. */
#define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
+#define LOOKUP_NO_NEGATIVE 0x200000 /* Hint: don't cache negative */
+
extern int path_pts(struct path *path);
extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
@@ -68,8 +70,8 @@ extern struct dentry *kern_path_locked(const char *, struct path *);
extern struct dentry *try_lookup_one_len(const char *, struct dentry *, int);
extern struct dentry *lookup_one_len(const char *, struct dentry *, int);
-extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int);
-extern struct dentry *lookup_positive_unlocked(const char *, struct dentry *, int);
+extern struct dentry *lookup_one_len_unlocked(const char *, struct dentry *, int, unsigned int);
+extern struct dentry *lookup_positive_unlocked(const char *, struct dentry *, int, unsigned int);
extern int follow_down_one(struct path *);
extern int follow_down(struct path *);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH v2] ovl: suppress negative dentry in lookup
2020-05-12 8:32 ` Miklos Szeredi
@ 2020-05-12 8:55 ` Amir Goldstein
2020-05-12 9:30 ` Miklos Szeredi
0 siblings, 1 reply; 6+ messages in thread
From: Amir Goldstein @ 2020-05-12 8:55 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Chengguang Xu, overlayfs, Al Viro, linux-fsdevel
On Tue, May 12, 2020 at 11:32 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, May 12, 2020 at 10:50:31AM +0300, Amir Goldstein wrote:
>
> > This helper should be in vfs code, not duplicating vfs code
> > and please don't duplicate code in vfs either.
> >
> > I think you can use a lookup flag (LOOKUP_POSITIVE_CACHE???)
> > to describe the desired behavior and implement it inside
> > lookup_slow(). Document the semantics as well as explain
> > in the context of the helper the cases where modules might
> > find this useful (because they have higher level caches).
> >
> > Besides the fact that this helper really needs review by Al
> > and that duplicating subtle code is wrong in so many levels,
> > I suppose the functionality could prove useful to other subsystems
> > as well.
>
> Something like this (untested). Needs splitup and changelogs.
>
> Thanks,
> Miklos
>
> ---
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index c31f362fa098..e52a3b35ebac 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -752,7 +752,7 @@ cifs_get_root(struct smb_vol *vol, struct super_block *sb)
> while (*s && *s != sep)
> s++;
>
> - child = lookup_positive_unlocked(p, dentry, s - p);
> + child = lookup_positive_unlocked(p, dentry, s - p, 0);
> dput(dentry);
> dentry = child;
> } while (!IS_ERR(dentry));
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index b7f2e971ecbc..df4f37a6a9ab 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -299,7 +299,7 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent)
> if (!parent)
> parent = debugfs_mount->mnt_root;
>
> - dentry = lookup_positive_unlocked(name, parent, strlen(name));
> + dentry = lookup_positive_unlocked(name, parent, strlen(name), 0);
> if (IS_ERR(dentry))
> return NULL;
> return dentry;
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index e23752d9a79f..e39af6313ad9 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -407,7 +407,7 @@ static struct dentry *ecryptfs_lookup(struct inode *ecryptfs_dir_inode,
> name = encrypted_and_encoded_name;
> }
>
> - lower_dentry = lookup_one_len_unlocked(name, lower_dir_dentry, len);
> + lower_dentry = lookup_one_len_unlocked(name, lower_dir_dentry, len, 0);
> if (IS_ERR(lower_dentry)) {
> ecryptfs_printk(KERN_DEBUG, "%s: lookup_one_len() returned "
> "[%ld] on lower_dentry = [%s]\n", __func__,
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index 2dd55b172d57..a4276d14aebb 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -145,7 +145,7 @@ static struct dentry *reconnect_one(struct vfsmount *mnt,
> if (err)
> goto out_err;
> dprintk("%s: found name: %s\n", __func__, nbuf);
> - tmp = lookup_one_len_unlocked(nbuf, parent, strlen(nbuf));
> + tmp = lookup_one_len_unlocked(nbuf, parent, strlen(nbuf), 0);
> if (IS_ERR(tmp)) {
> dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
> err = PTR_ERR(tmp);
> diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
> index 9dc7e7a64e10..92e7f264baa1 100644
> --- a/fs/kernfs/mount.c
> +++ b/fs/kernfs/mount.c
> @@ -224,7 +224,7 @@ struct dentry *kernfs_node_dentry(struct kernfs_node *kn,
> return ERR_PTR(-EINVAL);
> }
> dtmp = lookup_positive_unlocked(kntmp->name, dentry,
> - strlen(kntmp->name));
> + strlen(kntmp->name), 0);
> dput(dentry);
> if (IS_ERR(dtmp))
> return dtmp;
> diff --git a/fs/namei.c b/fs/namei.c
> index a320371899cf..e70b7a14bdcc 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1532,6 +1532,9 @@ static struct dentry *__lookup_slow(const struct qstr *name,
> if (unlikely(old)) {
> dput(dentry);
> dentry = old;
> + } else if ((flags & LOOKUP_NO_NEGATIVE) &&
> + d_is_negative(dentry)) {
> + d_drop(dentry);
> }
> }
> return dentry;
> @@ -2562,7 +2565,8 @@ EXPORT_SYMBOL(lookup_one_len);
> * i_mutex held, and will take the i_mutex itself if necessary.
> */
> struct dentry *lookup_one_len_unlocked(const char *name,
> - struct dentry *base, int len)
> + struct dentry *base, int len,
> + unsigned int flags)
> {
> struct qstr this;
> int err;
> @@ -2572,9 +2576,9 @@ struct dentry *lookup_one_len_unlocked(const char *name,
> if (err)
> return ERR_PTR(err);
>
> - ret = lookup_dcache(&this, base, 0);
> + ret = lookup_dcache(&this, base, flags);
> if (!ret)
> - ret = lookup_slow(&this, base, 0);
> + ret = lookup_slow(&this, base, flags);
> return ret;
> }
> EXPORT_SYMBOL(lookup_one_len_unlocked);
> @@ -2588,9 +2592,10 @@ EXPORT_SYMBOL(lookup_one_len_unlocked);
> * this one avoids such problems.
> */
> struct dentry *lookup_positive_unlocked(const char *name,
> - struct dentry *base, int len)
> + struct dentry *base, int len,
> + unsigned int flags)
> {
> - struct dentry *ret = lookup_one_len_unlocked(name, base, len);
> + struct dentry *ret = lookup_one_len_unlocked(name, base, len, flags);
> if (!IS_ERR(ret) && d_flags_negative(smp_load_acquire(&ret->d_flags))) {
> dput(ret);
> ret = ERR_PTR(-ENOENT);
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index aae514d40b64..19628922969c 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -855,7 +855,7 @@ compose_entry_fh(struct nfsd3_readdirres *cd, struct svc_fh *fhp,
> } else
> dchild = dget(dparent);
> } else
> - dchild = lookup_positive_unlocked(name, dparent, namlen);
> + dchild = lookup_positive_unlocked(name, dparent, namlen, 0);
> if (IS_ERR(dchild))
> return rv;
> if (d_mountpoint(dchild))
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 996ac01ee977..0c3c7928a319 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3066,7 +3066,7 @@ nfsd4_encode_dirent_fattr(struct xdr_stream *xdr, struct nfsd4_readdir *cd,
> __be32 nfserr;
> int ignore_crossmnt = 0;
>
> - dentry = lookup_positive_unlocked(name, cd->rd_fhp->fh_dentry, namlen);
> + dentry = lookup_positive_unlocked(name, cd->rd_fhp->fh_dentry, namlen, 0);
> if (IS_ERR(dentry))
> return nfserrno(PTR_ERR(dentry));
>
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index 0db23baf98e7..193857487060 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -200,7 +200,8 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
> int err;
> bool last_element = !post[0];
>
> - this = lookup_positive_unlocked(name, base, namelen);
> + this = lookup_positive_unlocked(name, base, namelen,
> + LOOKUP_NO_NEGATIVE);
> if (IS_ERR(this)) {
> err = PTR_ERR(this);
> this = NULL;
> @@ -657,7 +658,7 @@ struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh)
> if (err)
> return ERR_PTR(err);
>
> - index = lookup_positive_unlocked(name.name, ofs->indexdir, name.len);
> + index = lookup_positive_unlocked(name.name, ofs->indexdir, name.len, 0);
> kfree(name.name);
> if (IS_ERR(index)) {
> if (PTR_ERR(index) == -ENOENT)
> @@ -689,7 +690,7 @@ struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
> if (err)
> return ERR_PTR(err);
>
> - index = lookup_positive_unlocked(name.name, ofs->indexdir, name.len);
> + index = lookup_positive_unlocked(name.name, ofs->indexdir, name.len, 0);
> if (IS_ERR(index)) {
> err = PTR_ERR(index);
> if (err == -ENOENT) {
> @@ -1137,7 +1138,7 @@ bool ovl_lower_positive(struct dentry *dentry)
> struct dentry *lowerdir = poe->lowerstack[i].dentry;
>
> this = lookup_positive_unlocked(name->name, lowerdir,
> - name->len);
> + name->len, 0);
> if (IS_ERR(this)) {
> switch (PTR_ERR(this)) {
> case -ENOENT:
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index b6a4f692d345..f588839ebe2e 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -2488,7 +2488,7 @@ int dquot_quota_on_mount(struct super_block *sb, char *qf_name,
> struct dentry *dentry;
> int error;
>
> - dentry = lookup_positive_unlocked(qf_name, sb->s_root, strlen(qf_name));
> + dentry = lookup_positive_unlocked(qf_name, sb->s_root, strlen(qf_name), 0);
> if (IS_ERR(dentry))
> return PTR_ERR(dentry);
>
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index a4bb992623c4..4896eeeeea46 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -49,6 +49,8 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
> #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
>
> +#define LOOKUP_NO_NEGATIVE 0x200000 /* Hint: don't cache negative */
> +
The language lawyers will call this double negative, but I do
prefer this over LOOKUP_POSITIVE :-)
Thanks,
Amir.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH v2] ovl: suppress negative dentry in lookup
2020-05-12 8:55 ` Amir Goldstein
@ 2020-05-12 9:30 ` Miklos Szeredi
2020-05-13 2:15 ` Dave Chinner
0 siblings, 1 reply; 6+ messages in thread
From: Miklos Szeredi @ 2020-05-12 9:30 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Chengguang Xu, overlayfs, Al Viro, linux-fsdevel
On Tue, May 12, 2020 at 10:55 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, May 12, 2020 at 11:32 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Tue, May 12, 2020 at 10:50:31AM +0300, Amir Goldstein wrote:
> > diff --git a/include/linux/namei.h b/include/linux/namei.h
> > index a4bb992623c4..4896eeeeea46 100644
> > --- a/include/linux/namei.h
> > +++ b/include/linux/namei.h
> > @@ -49,6 +49,8 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> > /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
> > #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
> >
> > +#define LOOKUP_NO_NEGATIVE 0x200000 /* Hint: don't cache negative */
> > +
>
> The language lawyers will call this double negative, but I do
> prefer this over LOOKUP_POSITIVE :-)
Maybe LOOKUP_NOCACHE_NEGATIVE...
And yeah, LOOKUP_POSITIVE and LOOKUP_CACHE_POSITIVE are sort of
meaningless, since we cache everything by default.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH v2] ovl: suppress negative dentry in lookup
2020-05-12 9:30 ` Miklos Szeredi
@ 2020-05-13 2:15 ` Dave Chinner
0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2020-05-13 2:15 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Amir Goldstein, Chengguang Xu, overlayfs, Al Viro, linux-fsdevel
On Tue, May 12, 2020 at 11:30:43AM +0200, Miklos Szeredi wrote:
> On Tue, May 12, 2020 at 10:55 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Tue, May 12, 2020 at 11:32 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Tue, May 12, 2020 at 10:50:31AM +0300, Amir Goldstein wrote:
>
> > > diff --git a/include/linux/namei.h b/include/linux/namei.h
> > > index a4bb992623c4..4896eeeeea46 100644
> > > --- a/include/linux/namei.h
> > > +++ b/include/linux/namei.h
> > > @@ -49,6 +49,8 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> > > /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
> > > #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
> > >
> > > +#define LOOKUP_NO_NEGATIVE 0x200000 /* Hint: don't cache negative */
> > > +
> >
> > The language lawyers will call this double negative, but I do
> > prefer this over LOOKUP_POSITIVE :-)
>
> Maybe LOOKUP_NOCACHE_NEGATIVE...
"DONTCACHE" is the terminaology we've used for telling the inode
cache not to cache inodes....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-13 2:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-12 7:13 [RFC PATCH v2] ovl: suppress negative dentry in lookup Chengguang Xu
2020-05-12 7:50 ` Amir Goldstein
2020-05-12 8:32 ` Miklos Szeredi
2020-05-12 8:55 ` Amir Goldstein
2020-05-12 9:30 ` Miklos Szeredi
2020-05-13 2:15 ` Dave Chinner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).