From: NeilBrown ovl_lookup currently needs to check if a dentry with the same name has already been added to the dcache as readdir might need to do. This is an unnecessary performance cost to manage a rare race. If ovl could know which in-lookup dentries raced with readdir, it could limit the extra lookup to just those. So add d_alloc_noblock_return() which provides the in-lookup dentry when it returns -EWOULDBLOCK. ovl_readdir() can use this this and flag the dentry such that ovl_lookup() and easily check if a repeat lookup is needed. Signed-off-by: NeilBrown --- fs/dcache.c | 50 ++++++++++++++++++++++++++++++++++++---- fs/overlayfs/namei.c | 23 ++++++++++-------- fs/overlayfs/overlayfs.h | 2 ++ fs/overlayfs/readdir.c | 7 ++++-- include/linux/dcache.h | 2 ++ 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/fs/dcache.c b/fs/dcache.c index a2ddfe811df3..2f11257b725b 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -2749,7 +2749,8 @@ enum alloc_para { static inline struct dentry *__d_alloc_parallel(struct dentry *parent, const struct qstr *name, - enum alloc_para how) + enum alloc_para how, + struct dentry **dentryp) { unsigned int hash = name->hash; struct hlist_bl_head *b = in_lookup_hash(parent, hash); @@ -2836,7 +2837,10 @@ struct dentry *__d_alloc_parallel(struct dentry *parent, case ALLOC_PARA_FAIL: spin_unlock(&dentry->d_lock); dput(new); - dput(dentry); + if (dentryp) + *dentryp = dentry; + else + dput(dentry); return ERR_PTR(-EWOULDBLOCK); case ALLOC_PARA_WAIT: wait_var_event_spinlock(&dentry->d_flags, @@ -2899,7 +2903,7 @@ struct dentry *__d_alloc_parallel(struct dentry *parent, struct dentry *d_alloc_parallel(struct dentry *parent, const struct qstr *name) { - return __d_alloc_parallel(parent, name, ALLOC_PARA_WAIT); + return __d_alloc_parallel(parent, name, ALLOC_PARA_WAIT, NULL); } EXPORT_SYMBOL(d_alloc_parallel); @@ -2931,11 +2935,49 @@ struct dentry *d_alloc_noblock(struct dentry *parent, de = try_lookup_noperm(name, parent); if (!de) - de = __d_alloc_parallel(parent, name, ALLOC_PARA_FAIL); + de = __d_alloc_parallel(parent, name, ALLOC_PARA_FAIL, NULL); return de; } EXPORT_SYMBOL(d_alloc_noblock); +/** + * d_alloc_noblock_return() - find or allocate a new dentry + * @parent - dentry of the parent + * @name - name of the dentry within that parent. + * @dentryp - place to store the blocking dentry + * + * A new dentry is allocated and, providing it is unique, added to the + * relevant index. + * If an existing dentry is found with the same parent/name that is + * not d_in_lookup() then that is returned instead. + * If the existing dentry is d_in_lookup(), d_alloc_noblock() + * returns with error %-EWOULDBLOCK and the blocking dentry is passed + * in @dentryp. The dentry must be dput() by the caller. + * + * Thus if the returned dentry is d_in_lookup() then the caller has + * exclusive access until it completes the lookup. + * If the returned dentry is not d_in_lookup() then a lookup has + * already completed. + * + * The @name need not already have ->hash set. + * + * Returns: the dentry, whether found or allocated, or an error + * %-ENOMEM, %-EWOULDBLOCK, and anything returned by ->d_hash(). + */ +struct dentry *d_alloc_noblock_return(struct dentry *parent, + struct qstr *name, + struct dentry **dentryp) +{ + struct dentry *de; + + de = try_lookup_noperm(name, parent); + if (!de) + de = __d_alloc_parallel(parent, name, ALLOC_PARA_FAIL, + dentryp); + return de; +} +EXPORT_SYMBOL(d_alloc_noblock_return); + /* * - Unhash the dentry * - Retrieve and clear the waitqueue head in dentry diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c index 69032eb2b1e2..524e661c3c8d 100644 --- a/fs/overlayfs/namei.c +++ b/fs/overlayfs/namei.c @@ -1400,16 +1400,19 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, if (dentry->d_name.len > ofs->namelen) return ERR_PTR(-ENAMETOOLONG); - /* - * The existance of this in-lookup dentry might have forced - * readdir to do the lookup with a new dentry. If so we must - * return that one. - */ - alias = try_lookup_noperm(&QSTR_LEN(dentry->d_name.name, - dentry->d_name.len), - dentry->d_parent); - if (alias && !IS_ERR(alias)) - return alias; + if (ovl_dentry_test_flag(OVL_E_RACED_READDIR, dentry)) { + ovl_dentry_clear_flag(OVL_E_RACED_READDIR, dentry); + /* + * The existance of this in-lookup dentry might have + * forced readdir to do the lookup with a new dentry. + * If so we must return that one. + */ + alias = try_lookup_noperm(&QSTR_LEN(dentry->d_name.name, + dentry->d_name.len), + dentry->d_parent); + if (alias && !IS_ERR(alias)) + return alias; + } with_ovl_creds(dentry->d_sb) err = ovl_lookup_layers(&ctx, &d); diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index b75df37f70ac..bd6f1a25aca1 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -71,6 +71,8 @@ enum ovl_entry_flag { OVL_E_CONNECTED, /* Lower stack may contain xwhiteout entries */ OVL_E_XWHITEOUTS, + /* dentry was found in-lookup during readdir */ + OVL_E_RACED_READDIR, }; enum { diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index e03b32491941..e483bd939a8c 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -553,7 +553,7 @@ static int ovl_cache_update(const struct path *path, struct ovl_cache_entry *p, { struct dentry *dir = path->dentry; struct ovl_fs *ofs = OVL_FS(dir->d_sb); - struct dentry *this = NULL; + struct dentry *this = NULL, *in_lookup; enum ovl_path_type type; u64 ino = p->real_ino; int xinobits = ovl_xino_bits(ofs); @@ -574,7 +574,8 @@ static int ovl_cache_update(const struct path *path, struct ovl_cache_entry *p, } } /* This checks also for xwhiteouts */ - this = d_alloc_noblock(dir, &QSTR_LEN(p->name, p->len)); + this = d_alloc_noblock_return(dir, &QSTR_LEN(p->name, p->len), + &in_lookup); if (this == ERR_PTR(-EWOULDBLOCK)) { /* * Some other thead is looking up this name and will @@ -583,6 +584,8 @@ static int ovl_cache_update(const struct path *path, struct ovl_cache_entry *p, * lookup gets a turn it will find and return this * dentry. */ + ovl_dentry_set_flag(OVL_E_RACED_READDIR, in_lookup); + dput(in_lookup); this = d_alloc_name(dir, p->name); } if (!IS_ERR(this) && !d_unhashed(this)) { diff --git a/include/linux/dcache.h b/include/linux/dcache.h index 662b98185337..db7cdcbac775 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -258,6 +258,8 @@ extern struct dentry * d_alloc(struct dentry *, const struct qstr *); extern struct dentry * d_alloc_anon(struct super_block *); extern struct dentry * d_alloc_parallel(struct dentry *, const struct qstr *); extern struct dentry * d_alloc_noblock(struct dentry *, struct qstr *); +extern struct dentry * d_alloc_noblock_return(struct dentry *, struct qstr *, + struct dentry **); extern struct dentry * d_splice_alias(struct inode *, struct dentry *); struct dentry *d_duplicate(struct dentry *dentry); /* weird procfs mess; *NOT* exported */ -- 2.50.0.107.gf914562f5916.dirty