From: NeilBrown DCACHE_PAR_LOOKUP acts like a lock in that threads can block waiting for it to clear. As we plan to make changes to lock order for this lock, teach lockdep to monitor it so as to help detect bugs early. As NFS allocates an in-lookup dentry to unlink a silly-renamed file, and completes the lookup in a different thread, we need interfaces to release and the acquire ownership of the lock. This avoids lockdep complaining that a lock is still held on return to user-space. Signed-off-by: NeilBrown --- fs/dcache.c | 14 ++++++++++++++ fs/nfs/unlink.c | 3 +++ include/linux/dcache.h | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/fs/dcache.c b/fs/dcache.c index cbd5738de168..3af8b2ac699f 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1901,6 +1901,7 @@ EXPORT_SYMBOL(d_invalidate); static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name) { + static struct lock_class_key __lookup_key; struct dentry *dentry; char *dname; int err; @@ -1958,6 +1959,8 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name) dentry->waiters = NULL; INIT_HLIST_NODE(&dentry->d_sib); + lockdep_init_map(&dentry->lookup_map, "DCACHE_PAR_LOOKUP", &__lookup_key, 0); + if (dentry->d_op && dentry->d_op->d_init) { err = dentry->d_op->d_init(dentry); if (err) { @@ -2037,6 +2040,7 @@ struct dentry *d_duplicate(struct dentry *dentry) return ERR_PTR(-ENOMEM); new->d_flags |= DCACHE_PAR_LOOKUP; + lock_map_acquire_try(&new->lookup_map); spin_lock(&parent->d_lock); new->d_parent = dget_dlock(parent); hlist_add_head(&new->d_sib, &parent->d_children); @@ -2801,6 +2805,14 @@ static inline void end_dir_add(struct inode *dir, unsigned int n) static void d_wait_lookup(struct dentry *dentry) { if (likely(d_in_lookup(dentry))) { + /* + * Tell lockdep we will wait for the lookup lock, after + * dropping ->d_lock, but won't actually take it. + */ + spin_release(&dentry->d_lock.dep_map, 0); + lock_map_sync(&dentry->lookup_map); + spin_acquire(&dentry->d_lock.dep_map, 0, 1, _THIS_IP_); + dentry->d_flags |= DCACHE_LOOKUP_WAITERS; wait_var_event_spinlock(&dentry->d_flags, !d_in_lookup(dentry), @@ -2923,6 +2935,7 @@ struct dentry *__d_alloc_parallel(struct dentry *parent, } hlist_bl_add_head(&new->d_in_lookup_hash, b); hlist_bl_unlock(b); + lock_map_acquire_try(&new->lookup_map); return new; mismatch: spin_unlock(&dentry->d_lock); @@ -3021,6 +3034,7 @@ static void __d_lookup_unhash(struct dentry *dentry) b = in_lookup_hash(dentry->d_parent, dentry->d_name.hash); hlist_bl_lock(b); dentry->d_flags &= ~DCACHE_PAR_LOOKUP; + lock_map_release(&dentry->lookup_map); __hlist_bl_del(&dentry->d_in_lookup_hash); hlist_bl_unlock(b); dentry->waiters = NULL; diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c index b57cfaa4d516..c8d712204e64 100644 --- a/fs/nfs/unlink.c +++ b/fs/nfs/unlink.c @@ -67,6 +67,7 @@ static void nfs_async_unlink_release(void *calldata) struct super_block *sb = dentry->d_sb; up_read_non_owner(&NFS_I(d_inode(dentry->d_parent))->rmdir_sem); + d_lookup_acquire(dentry); d_lookup_done(dentry); nfs_free_unlinkdata(data); dput(dentry); @@ -159,6 +160,8 @@ static int nfs_call_unlink(struct dentry *dentry, struct inode *inode, struct nf return ret; } data->dentry = alias; + d_lookup_release(alias); + nfs_do_call_unlink(inode, data); return 1; } diff --git a/include/linux/dcache.h b/include/linux/dcache.h index 2b7d99ec9306..e7e3ef05313b 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -116,6 +116,8 @@ struct dentry { * possible! */ + /* lockdep tracking of DCACHE_PAR_LOOKUP locks */ + struct lockdep_map lookup_map; struct list_head d_lru; /* LRU list */ struct hlist_node d_sib; /* child of parent list */ struct hlist_head d_children; /* our children */ @@ -554,6 +556,36 @@ static inline int simple_positive(const struct dentry *dentry) unsigned long vfs_pressure_ratio(unsigned long val); +/** + * d_lookup_release - release ownership of DCACHE_PAR_LOOKUP lock + * @dentry: dentry that is locked + * + * If an in-lookup dentry is to be passed to another thread which + * will drop the in-lookup lock, then d_lookup_release() must be called + * to tell lockdep that this thread no lock holds the lock. The + * thread that receives the lock must call d_lookup_acquire() to + * acquire the lock. + */ +static inline void d_lookup_release(struct dentry *dentry) +{ + if (d_in_lookup(dentry)) + lock_map_release(&dentry->lookup_map); +} + +/** + * d_lookup_acquire - acquire ownership of DCACHE_PAR_LOOKUP lock + * @dentry: dentry that is locked + * + * If an in-lookup dentry was passed to this thread, the + * d_lookup_acquire() must be called to tell lockdep that this + * thread now owns the DCACHE_PAR_LOOKUP lock. + */ +static inline void d_lookup_acquire(struct dentry *dentry) +{ + if (d_in_lookup(dentry)) + lock_map_acquire_try(&dentry->lookup_map); +} + /** * d_inode - Get the actual inode of this dentry * @dentry: The dentry to query -- 2.50.0.107.gf914562f5916.dirty