From: NeilBrown When procfs finds a name in iterate_shared() that isn't in the dcache it *must* add it so that it can have a stable inode number to report (inodes are only accessible from the dcache in procfs). It uses d_alloc_parallel(). A planned change to locking will make it unsafe to call d_alloc_parallel() while holding the directory lock. Other filesystems which prime the dcache in iterate_shared() use d_alloc_trylock() which is safe but can fail if it races with ->lookup. As procfs cannot handle failure we need something better. procfs doesn't *need* the parent to be locked. There are no shared data structures accessed that don't have their own locking. So it is safe to drop and re-take the parent lock. We could do this around the whole iteration, but as failure of d_alloc_trylock() is rare it is more efficient to drop and retake it just around a call of d_alloc_parallel() when d_alloc_trylock does fail. Other code that drops and retakes the lock in iterate_shared needs to be careful to check S_DEAD which could be set while the lock is dropped. This is not needed in procfs as the flag is never set. As d_alloc_trylock() calls try_lookup_noperm(), we can skip that call and simplify the code. Signed-off-by: NeilBrown --- fs/proc/base.c | 36 ++++++++++++++++++------------- fs/proc/proc_sysctl.c | 49 ++++++++++++++++++++++++------------------- 2 files changed, 49 insertions(+), 36 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index 6a39de424f62..49939527763d 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2106,24 +2106,32 @@ bool proc_fill_cache(struct file *file, struct dir_context *ctx, unsigned type = DT_UNKNOWN; ino_t ino = 1; - child = try_lookup_noperm(&qname, dir); + child = d_alloc_trylock(dir, &qname); if (IS_ERR(child)) goto end_instantiate; - if (!child) { + if (child == ERR_PTR(-EWOULDBLOCK)) { + /* + * Need to drop directory lock, which isn't really + * needed here anyway. As rmdir never happens in procfs + * we don't need to be concerned about S_DEAD being set + * while unlocked. + */ + inode_unlock_shared(dir->d_inode); child = d_alloc_parallel(dir, &qname); - if (IS_ERR(child)) - goto end_instantiate; - if (d_in_lookup(child)) { - struct dentry *res; - res = instantiate(child, task, ptr); - d_lookup_done(child); - if (unlikely(res)) { - dput(child); - child = res; - if (IS_ERR(child)) - goto end_instantiate; - } + inode_lock_shared(dir->d_inode); + } + if (IS_ERR(child)) + goto end_instantiate; + if (d_in_lookup(child)) { + struct dentry *res; + res = instantiate(child, task, ptr); + d_lookup_done(child); + if (unlikely(res)) { + dput(child); + child = res; + if (IS_ERR(child)) + goto end_instantiate; } } inode = d_inode(child); diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 04a382178c65..de96a08a1e64 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -686,29 +686,34 @@ static bool proc_sys_fill_cache(struct file *file, ino_t ino = 0; unsigned type = DT_UNKNOWN; - qname.name = table->procname; - qname.len = strlen(table->procname); - qname.hash = full_name_hash(dir, qname.name, qname.len); - - child = d_lookup(dir, &qname); - if (!child) { + qname = QSTR(table->procname); + child = d_alloc_trylock(dir, &qname); + if (child == ERR_PTR(-EWOULDBLOCK)) { + /* + * Need to drop directory lock, which isn't really + * needed here anyway. As rmdir never happens in procfs + * we don't need to be concerned about S_DEAD being set + * while unlocked. + */ + inode_unlock_shared(dir->d_inode); child = d_alloc_parallel(dir, &qname); - if (IS_ERR(child)) - return false; - if (d_in_lookup(child)) { - struct dentry *res; - inode = proc_sys_make_inode(dir->d_sb, head, table); - res = d_splice_alias_ops(inode, child, - &proc_sys_dentry_operations); - d_lookup_done(child); - if (unlikely(res)) { - dput(child); - - if (IS_ERR(res)) - return false; - - child = res; - } + inode_lock_shared(dir->d_inode); + } + if (IS_ERR(child)) + return false; + if (d_in_lookup(child)) { + struct dentry *res; + inode = proc_sys_make_inode(dir->d_sb, head, table); + res = d_splice_alias_ops(inode, child, + &proc_sys_dentry_operations); + d_lookup_done(child); + if (unlikely(res)) { + dput(child); + + if (IS_ERR(res)) + return false; + + child = res; } } inode = d_inode(child); -- 2.50.0.107.gf914562f5916.dirty