* Re: [PATCH v4 6/7] VFS: add lockdep monitoring of DCACHE_PAR_LOOKUP lock.
[not found] ` <20260904215142.1060510-7-neilb@ownmail.net>
@ 2026-09-30 16:31 ` Borah, Chaitanya Kumar
2026-09-30 21:08 ` NeilBrown
0 siblings, 1 reply; 3+ messages in thread
From: Borah, Chaitanya Kumar @ 2026-09-30 16:31 UTC (permalink / raw)
To: NeilBrown, Alexander Viro, Christian Brauner
Cc: Jan Kara, linux-fsdevel, Jeff Layton, Amir Goldstein,
Miklos Szeredi, linux-kernel, ravitejax.veesam,
intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Hello Neil,
On 9/5/2026 3:18 AM, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
>
> 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.
>
This seems to be causing regression in our linux-next CI [1] since
next-20260928.
<4>[ 10.773931] ======================================================
<4>[ 10.780181] WARNING: possible circular locking dependency detected
<4>[ 10.786407] 7.3.0-rc5-next-20260928-next-20260928-g6375e61c01e9+
#1 Not tainted
<4>[ 10.793773] ------------------------------------------------------
<4>[ 10.800017] podman/794 is trying to acquire lock:
<4>[ 10.804801] ffff888133cb8388
(&type->i_mutex_dir_key#3){++++}-{4:4}, at: lookup_slow+0x31/0x60
<4>[ 10.814849]
but task is already holding lock:
<4>[ 10.820743] ffff88812dbd3048 (DCACHE_PAR_LOOKUP){+.+.}-{0:0}, at:
__d_alloc_parallel+0x53a/0x920
<4>[ 10.830976]
which lock already depends on the new lock.
Detailed log can be seen found in [2].
We confirmed that reverting the patch solves the issue.
Could you please check why the patch causes this regression and provide
a fix if necessary?
Regards
Chaitanya
[1] https://intel-gfx-ci.01.org/tree/linux-next/combined-alt.html?
[2]
https://intel-gfx-ci.01.org/tree/linux-next/next-20260928/bat-arls-6/boot0.txt
--Bisect Logs--
git bisect start
# status: waiting for both good and bad commits
# bad: [6375e61c01e93e35ee7acd336a689ac1fae4b509] Add linux-next
specific files for 20260928
git bisect bad 6375e61c01e93e35ee7acd336a689ac1fae4b509
# status: waiting for good commit(s), bad commit known
# good: [5dad87615c9861cfa366ca984b52f581e861df20] tty: add break_wait
kernel-doc
git bisect good 5dad87615c9861cfa366ca984b52f581e861df20
# bad: [3bd2d86b7cc084d0a610760cfa40349497ea8920] Merge branch
'for-next' of https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git
git bisect bad 3bd2d86b7cc084d0a610760cfa40349497ea8920
# good: [3cc80c8dbcd8475e98ae86759750d0a140b063fd] Merge branch
'for-next' of
https://git.kernel.org/pub/scm/linux/kernel/git/gclement/mvebu.git
git bisect good 3cc80c8dbcd8475e98ae86759750d0a140b063fd
# bad: [3ac0b1643b3b216ce37d5ec4352fee8a01ecd37d] Merge branch 'fs-next'
of linux-next
git bisect bad 3ac0b1643b3b216ce37d5ec4352fee8a01ecd37d
# good: [48a0400362e2648ba3ab56df8eceb782abfb7d17] Merge branch
'for-next' of
https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git
git bisect good 48a0400362e2648ba3ab56df8eceb782abfb7d17
# good: [1ba62e451b02da61d4786a11da46501cd253cd2e] Merge branch
'for-next' of
https://git.kernel.org/pub/scm/linux/kernel/git/hubcap/linux.git
git bisect good 1ba62e451b02da61d4786a11da46501cd253cd2e
# bad: [b298f749884547ec4b746bdffc293fdd3b5a64ec] Merge branch
'vfs-7.4.misc' into vfs.all
git bisect bad b298f749884547ec4b746bdffc293fdd3b5a64ec
# good: [cb70d8c361808b08d13dc4bdddb59cd92c48f84c] Merge branch
'vfs-7.4.file' into vfs.all
git bisect good cb70d8c361808b08d13dc4bdddb59cd92c48f84c
# good: [d2b6b01e5969a762e92832cb6c9e2a470a842fcb] Merge branch
'vfs-7.4.iomap' into vfs.all
git bisect good d2b6b01e5969a762e92832cb6c9e2a470a842fcb
# good: [a647a53bc89a046bee4a12a0a5627f2dfaaef807] dcache: report a
Tasks-RCU quiescent state in dentry_kill()
git bisect good a647a53bc89a046bee4a12a0a5627f2dfaaef807
# good: [a2fb05f5133f83b2359b4b15d091bcf21e8b02bc] Merge patch series
"kernfs: don't hold kernfs_rwsem across dir_emit()"
git bisect good a2fb05f5133f83b2359b4b15d091bcf21e8b02bc
# bad: [3879f51857325da9bf3cfb073280257cd16ae067] Merge patch series
"VFS: prepare for changes to directory locking"
git bisect bad 3879f51857325da9bf3cfb073280257cd16ae067
# good: [17c7d109d8c9a6744b40ded06cc596cd6a80ec49] VFS: introduce
d_alloc_trylock()
git bisect good 17c7d109d8c9a6744b40ded06cc596cd6a80ec49
# good: [cc47a1ba1983116ee7a720a79197eb5299ae5d31] VFS: Add
LOOKUP_SHARED flag.
git bisect good cc47a1ba1983116ee7a720a79197eb5299ae5d31
# bad: [fd96e30426ff3e1352309ccf6843dc7fd9d38fde] VFS: reserve a d_flags
bit for fs-specific usage
git bisect bad fd96e30426ff3e1352309ccf6843dc7fd9d38fde
# bad: [59492f9991dc19d5cc2c34295f1403d0f211e734] VFS: add lockdep
monitoring of DCACHE_PAR_LOOKUP lock.
git bisect bad 59492f9991dc19d5cc2c34295f1403d0f211e734
# first bad commit: [59492f9991dc19d5cc2c34295f1403d0f211e734] VFS: add
lockdep monitoring of DCACHE_PAR_LOOKUP lock.
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
> fs/dcache.c | 15 +++++++++++++++
> fs/nfs/unlink.c | 3 +++
> include/linux/dcache.h | 32 ++++++++++++++++++++++++++++++++
> 3 files changed, 50 insertions(+)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index cbd5738de168..83790c7a4dee 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,15 @@ 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, _THIS_IP_);
> + lock_map_acquire(&dentry->lookup_map);
> + lock_map_release(&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 +2936,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 +3035,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
^ permalink raw reply [flat|nested] 3+ messages in thread