mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>
To: NeilBrown <neil@brown.name>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>, <linux-fsdevel@vger.kernel.org>,
	Jeff Layton <jlayton@kernel.org>,
	Amir Goldstein <amir73il@gmail.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	<linux-kernel@vger.kernel.org>, <ravitejax.veesam@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH v4 6/7] VFS: add lockdep monitoring of DCACHE_PAR_LOOKUP lock.
Date: Wed, 30 Sep 2026 22:01:55 +0530	[thread overview]
Message-ID: <e3772a51-eabc-4c31-b73d-f3187bf1b3b2@intel.com> (raw)
In-Reply-To: <20260904215142.1060510-7-neilb@ownmail.net>

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


  reply	other threads:[~2026-09-30 16:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04 21:48 [PATCH v4 0/7] VFS: prepare for changes to directory locking NeilBrown
2026-09-04 21:48 ` [PATCH v4 1/7] VFS: fix various typos in documentation for start_creating start_removing etc NeilBrown
2026-09-04 21:48 ` [PATCH v4 2/7] VFS: enhance d_splice_alias() to handle hashed dentries NeilBrown
2026-09-04 21:48 ` [PATCH v4 3/7] VFS: introduce d_alloc_trylock() NeilBrown
2026-09-04 21:48 ` [PATCH v4 4/7] VFS: add d_duplicate() NeilBrown
2026-09-04 21:48 ` [PATCH v4 5/7] VFS: Add LOOKUP_SHARED flag NeilBrown
2026-09-04 21:48 ` [PATCH v4 6/7] VFS: add lockdep monitoring of DCACHE_PAR_LOOKUP lock NeilBrown
2026-09-30 16:31   ` Borah, Chaitanya Kumar [this message]
2026-09-30 21:08     ` NeilBrown
2026-10-01 10:39       ` Borah, Chaitanya Kumar
2026-09-04 21:48 ` [PATCH v4 7/7] VFS: reserve a d_flags bit for fs-specific usage NeilBrown
2026-09-15 21:11 ` [PATCH v4 0/7] VFS: prepare for changes to directory locking NeilBrown
2026-09-25 14:28 ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e3772a51-eabc-4c31-b73d-f3187bf1b3b2@intel.com \
    --to=chaitanya.kumar.borah@intel.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=neil@brown.name \
    --cc=ravitejax.veesam@intel.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®