public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: Dominique Martinet <asmadeus@codewreck.org>
Cc: Eric Van Hensbergen <ericvh@kernel.org>,
	 Latchesar Ionkov <lucho@ionkov.net>,
	Christian Schoenebeck <linux_oss@crudebyte.com>,
	 Andrew Morton <akpm@linux-foundation.org>,
	Eryu Guan <eguan@linux.alibaba.com>,
	 Yiwen Jiang <jiangyiwen@huawei.com>,
	v9fs@lists.linux.dev, linux-kernel@vger.kernel.org,
	 stable@vger.kernel.org
Subject: Re: [PATCH] 9p: fix WARN_ON when dropping nlink on files with nlink=0
Date: Mon, 20 Apr 2026 07:31:14 -0700	[thread overview]
Message-ID: <aeY32gOaV5jw1s8F@gmail.com> (raw)
In-Reply-To: <aZGRkaFZPXfZW8a0@codewreck.org>

hello Dominique,

On Sun, Feb 15, 2026 at 06:27:45PM +0900, Dominique Martinet wrote:
> Thanks for the patch and sorry for the delay

I had this fix in my tree, and I had forgot about. Now that I moved to
the 7.1, I started seeing it again, so, let's revamp this.

> hmm.. I'm not actually sure if we should call drop_nlink() at all in
> cacheless mode, actually..
> We don't really care about nlink in this context, as inode should be
> gone immediately anyway, or does nlink hitting zero imply something
> else?
> 
> So we could get the v9fs_session_info out of the inode
> (v9fs_inode2v9ses) and just return if CACHE_META is set?

Agreed -- in cacheless mode the server is authoritative and the inode is
on its way out, so adjusting i_nlink locally buys us nothing. I don't
see anything else keying off nlink hitting zero in that path.

> Others opinion would be great, but I get the feeling that just checking
> before update only makes the race smaller, and not totally fixed.

What about sometehing like:

commit 369beed134ff0b7ce1cf68d0c46b08ec07d625db
Author: Breno Leitao <leitao@debian.org>
Date:   Mon Jan 26 02:23:37 2026 -0800

    9p: skip nlink update in cacheless mode to fix WARN_ON
    
    v9fs_dec_count() unconditionally calls drop_nlink() on regular files,
    even when the inode's nlink is already zero. In cacheless mode the
    client refetches inode metadata from the server (the source of truth)
    on every operation, so by the time v9fs_remove() returns, the locally
    cached nlink may already reflect the post-unlink value:
    
      1. Client initiates unlink, server processes it and sets nlink to 0
      2. Client refetches inode metadata (nlink=0) before unlink returns
      3. Client's v9fs_remove() completes successfully
      4. Client calls v9fs_dec_count() which calls drop_nlink() on nlink=0
    
    This race is easily triggered under heavy unlink workloads, such as
    stress-ng's unlink stressor, producing the following warning:
    
      WARNING: fs/inode.c:417 at drop_nlink+0x4c/0xc8
      Call trace:
       drop_nlink+0x4c/0xc8
       v9fs_remove+0x1e0/0x250 [9p]
       v9fs_vfs_unlink+0x20/0x38 [9p]
       vfs_unlink+0x13c/0x258
       ...
    
    In cacheless mode the server is authoritative and the inode is on its
    way out, so locally adjusting nlink buys nothing. Skip v9fs_dec_count()
    entirely when neither CACHE_META nor CACHE_LOOSE is set, which both
    avoids the warning and removes a class of nlink races (two concurrent
    unlinkers observing nlink > 0 and both calling drop_nlink()) that an
    nlink == 0 guard alone would only narrow rather than close.
    
    Fixes: ac89b2ef9b55 ("9p: don't maintain dir i_nlink if the exported fs doesn't either")
    Cc: stable@vger.kernel.org
    Suggested-by: Dominique Martinet <asmadeus@codewreck.org>
    Signed-off-by: Breno Leitao <leitao@debian.org>

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index d1508b1fe1092..50cf837979d9c 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -488,10 +488,19 @@ static int v9fs_at_to_dotl_flags(int flags)
  * - ext4 (with dir_nlink feature enabled) sets nlink to 1 if a dir has more
  *   than EXT4_LINK_MAX (65000) links.
  *
+ * In cacheless mode the server is the source of truth for nlink and the
+ * inode is going away immediately, so locally adjusting i_nlink buys
+ * nothing and races with concurrent metadata fetches that may already
+ * have observed the post-unlink value (nlink == 0).
+ *
  * @inode: inode whose nlink is being dropped
  */
 static void v9fs_dec_count(struct inode *inode)
 {
+	struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
+
+	if (!(v9ses->cache & (CACHE_META | CACHE_LOOSE)))
+		return;
 	if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2)
 		drop_nlink(inode);
 }

  reply	other threads:[~2026-04-20 14:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-26 10:23 [PATCH] 9p: fix WARN_ON when dropping nlink on files with nlink=0 Breno Leitao
2026-02-15  9:27 ` Dominique Martinet
2026-04-20 14:31   ` Breno Leitao [this message]
2026-04-20 15:59     ` Dominique Martinet
2026-04-20 16:26       ` Breno Leitao

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=aeY32gOaV5jw1s8F@gmail.com \
    --to=leitao@debian.org \
    --cc=akpm@linux-foundation.org \
    --cc=asmadeus@codewreck.org \
    --cc=eguan@linux.alibaba.com \
    --cc=ericvh@kernel.org \
    --cc=jiangyiwen@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=stable@vger.kernel.org \
    --cc=v9fs@lists.linux.dev \
    /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