Skip to content

Commit 17eb98d

Browse files
neilbrownbrauner
authored andcommitted
VFS/ovl: add lookup_one_positive_killable()
ovl wants a lookup which won't block on a fatal signal. It currently uses down_write_killable() and then repeatedly calls to lookup_one() The lock may not be needed if the name is already in the dcache and it aids proposed future changes if the locking is kept internal to namei.c So this patch adds lookup_one_positive_killable() which is like lookup_one_positive() but will abort in the face of a fatal signal. overlayfs is changed to use this. Note that instead of always getting an exclusive lock, ovl now only gets a shared lock, and only sometimes. The exclusive lock was never needed. However down_read_killable() was only added in v4.15 but overlayfs started using down_write_killable() here in v4.7. Note that the linked list ->first_maybe_whiteout ->next_maybe_white is local to the thread so there is no concurrency in that list which could be threatened by removing the locking. Reviewed-by: Amir Goldstein <amir73il@gmail.com> Signed-off-by: NeilBrown <neil@brown.name> Signed-off-by: Christian Brauner <brauner@kernel.org>
1 parent 8f5ae30 commit 17eb98d

File tree

3 files changed

+72
-14
lines changed

3 files changed

+72
-14
lines changed

fs/namei.c

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1827,6 +1827,20 @@ static struct dentry *lookup_slow(const struct qstr *name,
18271827
return res;
18281828
}
18291829

1830+
static struct dentry *lookup_slow_killable(const struct qstr *name,
1831+
struct dentry *dir,
1832+
unsigned int flags)
1833+
{
1834+
struct inode *inode = dir->d_inode;
1835+
struct dentry *res;
1836+
1837+
if (inode_lock_shared_killable(inode))
1838+
return ERR_PTR(-EINTR);
1839+
res = __lookup_slow(name, dir, flags);
1840+
inode_unlock_shared(inode);
1841+
return res;
1842+
}
1843+
18301844
static inline int may_lookup(struct mnt_idmap *idmap,
18311845
struct nameidata *restrict nd)
18321846
{
@@ -3010,6 +3024,47 @@ struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap, struct qstr *name,
30103024
}
30113025
EXPORT_SYMBOL(lookup_one_unlocked);
30123026

3027+
/**
3028+
* lookup_one_positive_killable - lookup single pathname component
3029+
* @idmap: idmap of the mount the lookup is performed from
3030+
* @name: qstr olding pathname component to lookup
3031+
* @base: base directory to lookup from
3032+
*
3033+
* This helper will yield ERR_PTR(-ENOENT) on negatives. The helper returns
3034+
* known positive or ERR_PTR(). This is what most of the users want.
3035+
*
3036+
* Note that pinned negative with unlocked parent _can_ become positive at any
3037+
* time, so callers of lookup_one_unlocked() need to be very careful; pinned
3038+
* positives have >d_inode stable, so this one avoids such problems.
3039+
*
3040+
* This can be used for in-kernel filesystem clients such as file servers.
3041+
*
3042+
* It should be called without the parent i_rwsem held, and will take
3043+
* the i_rwsem itself if necessary. If a fatal signal is pending or
3044+
* delivered, it will return %-EINTR if the lock is needed.
3045+
*/
3046+
struct dentry *lookup_one_positive_killable(struct mnt_idmap *idmap,
3047+
struct qstr *name,
3048+
struct dentry *base)
3049+
{
3050+
int err;
3051+
struct dentry *ret;
3052+
3053+
err = lookup_one_common(idmap, name, base);
3054+
if (err)
3055+
return ERR_PTR(err);
3056+
3057+
ret = lookup_dcache(name, base, 0);
3058+
if (!ret)
3059+
ret = lookup_slow_killable(name, base, 0);
3060+
if (!IS_ERR(ret) && d_flags_negative(smp_load_acquire(&ret->d_flags))) {
3061+
dput(ret);
3062+
ret = ERR_PTR(-ENOENT);
3063+
}
3064+
return ret;
3065+
}
3066+
EXPORT_SYMBOL(lookup_one_positive_killable);
3067+
30133068
/**
30143069
* lookup_one_positive_unlocked - lookup single pathname component
30153070
* @idmap: idmap of the mount the lookup is performed from

fs/overlayfs/readdir.c

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -270,26 +270,26 @@ static bool ovl_fill_merge(struct dir_context *ctx, const char *name,
270270

271271
static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data *rdd)
272272
{
273-
int err;
273+
int err = 0;
274274
struct dentry *dentry, *dir = path->dentry;
275275
const struct cred *old_cred;
276276

277277
old_cred = ovl_override_creds(rdd->dentry->d_sb);
278278

279-
err = down_write_killable(&dir->d_inode->i_rwsem);
280-
if (!err) {
281-
while (rdd->first_maybe_whiteout) {
282-
struct ovl_cache_entry *p =
283-
rdd->first_maybe_whiteout;
284-
rdd->first_maybe_whiteout = p->next_maybe_whiteout;
285-
dentry = lookup_one(mnt_idmap(path->mnt),
286-
&QSTR_LEN(p->name, p->len), dir);
287-
if (!IS_ERR(dentry)) {
288-
p->is_whiteout = ovl_is_whiteout(dentry);
289-
dput(dentry);
290-
}
279+
while (rdd->first_maybe_whiteout) {
280+
struct ovl_cache_entry *p =
281+
rdd->first_maybe_whiteout;
282+
rdd->first_maybe_whiteout = p->next_maybe_whiteout;
283+
dentry = lookup_one_positive_killable(mnt_idmap(path->mnt),
284+
&QSTR_LEN(p->name, p->len),
285+
dir);
286+
if (!IS_ERR(dentry)) {
287+
p->is_whiteout = ovl_is_whiteout(dentry);
288+
dput(dentry);
289+
} else if (PTR_ERR(dentry) == -EINTR) {
290+
err = -EINTR;
291+
break;
291292
}
292-
inode_unlock(dir->d_inode);
293293
}
294294
ovl_revert_creds(old_cred);
295295

include/linux/namei.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap,
8080
struct dentry *lookup_one_positive_unlocked(struct mnt_idmap *idmap,
8181
struct qstr *name,
8282
struct dentry *base);
83+
struct dentry *lookup_one_positive_killable(struct mnt_idmap *idmap,
84+
struct qstr *name,
85+
struct dentry *base);
8386

8487
extern int follow_down_one(struct path *);
8588
extern int follow_down(struct path *path, unsigned int flags);

0 commit comments

Comments
 (0)