From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=209.85.167.170 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1793297417; b=wOSurpLEgaDH8ZfyotcWvHvSX6QQkmA39n4ZpaFFazGinwo4C2fmtzArDRdADJ8t0tot VPDMBF4kc9l705OZAdw+7X56r1KmGqnlHTZ4z/f388RbL3IbLiiscW3PRipvZP5YE9A9I xiee6Z8Sy++3T6pwIYPZ86asz1917yLp2tqY7Y3EJiT4ZlBZQTK4Zk+4Z7X28nIoQtiT4 BVpHf0pm3nmG6INvSf7P7lcj/FvAQ2Ed4SohmAhCMg3LNX10Otq2H8G4VloVSRtAkIbTF 6JGWe6s89CrEro4Pb76v77JsnKdn2FuG84daimqyPbA41SLe2AtYiVyI+ppW34Eu3LQ== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1793297417; h=DMARC-Filter:Received:DKIM-Signature:X-Google-DKIM-Signature: X-Forwarded-Encrypted:X-Gm-Message-State:X-Gm-Gg:X-Received:Received: From:To:Cc:Subject:Date:Message-ID:X-Mailer:In-Reply-To:References: MIME-Version:Content-Transfer-Encoding; bh=DJ2DAyUZ/cE8PWYufte6QskZeIdiAS+8BIHT7ee4M9A=; b=sGlrRIzzZmt2n8P3QkZ4U274IGPQxSV5VuH+NWOweXDyyEBGIB6JVHhNDH15/zUvARHQ oOvE4el9Yo1XeAokJac/h4u+u1UEmYfHtPa+PAHHgGbh4KHdW7cApVhZXhZoU/7be7kNB AWp4Uei+t4wjA2Ijm6gHtz8wcfjQEhDPDpOvf+6PmSFJr+lXNhiddQMEZuUJI3U/xBbfI OyFTDn9jl5JOTWAYfnXdiSPcgfPRTYXg3EOgwWyLBU+4SKMJjQoQP5/3VUcHF+L9dFHog pFGEpaROLgzlfEHx4F6/Xpz8wuvHPcBH65/MCM3cvAkYimWqYZg0vPhCQDwqcQof1fA== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass header.from=hammerspace.com; spf=pass smtp.mailfrom=hammerspace.com; dkim=pass header.d=hammerspace.com header.i=@hammerspace.com header.a=rsa-sha256 header.s=google header.b=n5Ycg7Ix; arc=none smtp.remote-ip=209.85.167.170 Received: from mail-oi1-f170.google.com (mail-oi1-f170.google.com [209.85.167.170]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D82ED606AB4A for ; Thu, 29 Oct 2026 18:10:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.2 smtp.subspace.kernel.org D82ED606AB4A Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=hammerspace.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=hammerspace.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key, unprotected) header.d=hammerspace.com header.i=@hammerspace.com header.a=rsa-sha256 header.s=google header.b=n5Ycg7Ix Received: by mail-oi1-f170.google.com with SMTP id 5614622812f47-4b190cdc9feso1407539b6e.0 for ; Thu, 29 Oct 2026 11:10:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hammerspace.com; s=google; t=1793297416; x=1793902216; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=DJ2DAyUZ/cE8PWYufte6QskZeIdiAS+8BIHT7ee4M9A=; b=n5Ycg7IxyZ+K65mf71RWcIDKfS2t3bQ1TMnTVvBEHF/qTOn8KHah5qx/iuaDmb0YCr AbogfBSMjHHUVO6j7S42cuw8dxSO2XuLXYxAcEYk/c6TUBHa4koTsGhBhVOcyPyzOOig Gkf0lC3j2BgUBb22KiadsllXwF6xONDrHxisxOMzk8U7+VLj3z7Hy/kbi1M0m99h/gXQ MY7QMxI4MCT23si6o0qTNVhd2Os7ijq8tTT5aO+FYwhAW7boMuui+yBWbPihESGwf4zI TgHJMhmie8psihtkrQiKu/mVgNrN2GVpvetVKtl7Xs3KqnV1w9UjkLYc5yAnmkhW0Vue DtSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1793297416; x=1793902216; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=DJ2DAyUZ/cE8PWYufte6QskZeIdiAS+8BIHT7ee4M9A=; b=LcblaxISFvhKEuzU1j6gSIJIsBEaGXmpRx7MISc13N/1zuSxpQNTDsMUs0v53KcwdL KyS8eCzJ0YitC98Y9LcoRul5Q/pv0o75d7uiYcMlJ+AFZdz6uwMtkTtPv80gSA1Gsav8 8Hc6C4bu3g0nFKFLNvtKk0N/FjTUXSg9A6fiG3aEQOfQ6lGK6xk+vS2Ny9UZ0EFgUKKD ZlfLRnsc5TeMbgbJcZEBBJzBx6TmmCQkrsPUKihOF64VD85XP3KwvrIq45QXMaR61QcW HintUqD73AeeE2e0olxiWsY8AIrHa19oK3kMwrLyhZfqzedyedu2j7COznTGtZY5qeIL V4XA== X-Forwarded-Encrypted: i=1; AKwUvBwMwG+GzFv/h5Bcb28wqjQ5WfYkPvX2lalQVIIb7Ahi4Xgd9/VsAqKxVeseLFZo5Lj3FO/WobZdz/w=@vger.kernel.org X-Gm-Message-State: AFq9FYIS/ePAbzzVicFCmPyx7DQYlcBCaJzeHTBRhk06mCiqgKiM5v03 Rh6+9zjlQ+UR4Dc55YNKLyDKdjzPsF1vGzP6RG1cAscG2ECESOQQmGV6GHfiOdFNp8w= X-Gm-Gg: AYBFou3yRBjpL/khxaXjYoBGZIbeqULHG1dXPiwZSxyknwqjfvKPGnNDUyjrMb5fW9K vpjVlaHcLdaCdvIHtZvx/2SziFvikYVoThOCAz5hP4kpHsVZGJTBD6qSoHxqTQvMyz59Y/srvhn bGnn2+uSjH2qyT7UG/v91PntS7kG7U1loSLiS3OdiirFQk9neiJL6McXGiIdTU/CZ1alr03r9dv DUe9xeIJH+66nB6bnFXtuJocyHm4kYkhVDIOapLlf2U+oqrrvGIM5V5HWXly4qKUaDtQZu5u/FN WHT2sZtHptA0zbTX7/eJWiHv8VSd1o6NWjnrnmVtgr4QhRYKknXlz1+QhBT0vsx3IiiVfAljWjW rraRMM04W2ycvjlWRnvmuceTLyflFEHtP49WW4w0toR6qgmr3dKDYKCSeHeMwSmFlLJ4fAZ9AO+ tY4e33uRIjFWLlWvzZVgi2MZ1Gk3Ae6nknaTROpHD/ws6XGNQ5xAI+1FhBaCt9eZUftEzJ9T3zo 8wZ0qXX9eFWjJ/1PfBseSXf0x464fO1YKnGtlGC3vG0xCD7a93eoRPXQkmc0vkhCQF7P1yJWqcT qBK9/rTJC1ettdcgWYt7xAUlWldpRK81orc9MUSvqv2EummqhzufiA6XDZJbMc4IMbx9kkhngJB MqkhXmY5yT20JF2WyIg== X-Received: by 2002:a05:6808:1b27:b0:53c:7298:3598 with SMTP id 5614622812f47-56382b885d6mr4651460b6e.31.1793297415482; Thu, 29 Oct 2026 11:10:15 -0700 (PDT) Received: from localhost (75-163-200-235.clsp.qwest.net. [75.163.200.235]) by smtp.gmail.com with ESMTPSA id 5614622812f47-563c5a4d8b7sm4093861b6e.8.2026.10.29.11.10.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Oct 2026 11:10:12 -0700 (PDT) From: Jonathan Flynn To: NeilBrown , Alexander Viro , Christian Brauner , Trond Myklebust , Anna Schumaker Cc: Jeff Layton , Chuck Lever , Jan Kara , Jonathan Corbet , linux-doc@vger.kernel.org, linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH RFC 08/12] nfs: keep the dentry of a shared-parent removal hashed Date: Thu, 29 Oct 2026 12:09:53 -0600 Message-ID: <20261029180957.387138-9-jonathan.flynn@hammerspace.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20261029180957.387138-1-jonathan.flynn@hammerspace.com> References: <20261028041023.3738553-1-neilb@ownmail.net> <20261029180957.387138-1-jonathan.flynn@hammerspace.com> X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Transfer-Encoding: 8bit With the directory locked shared, a removal stays ordered against lookups and opens of the same name only through the name's dentry, whose ->d_revalidate() waits while the dentry is marked. Three paths act on a cached dentry without that wait: nfs_prime_dcache(), which unhashes a dentry that a directory listing disagrees with and otherwise refreshes its inode from the listing; nfs4_file_open() when the open fails; and recovery of open state. Make each of them leave a dentry marked by such a removal alone, so that it stays hashed until the removal itself deletes it, and so that a listing that may predate the removal does not stop the removal from dropping the link count. Have nfs_prime_dcache() also leave alone a dentry that has been unhashed. When the listing holds a reference, a successful removal unhashes the dentry, rather than making it negative, before it clears the mark, and a refresh from a listing that predates the removal could then put back the link count that the removal took away. Without that reference the removal makes the dentry negative instead, and the listing treats the name as one it has no dentry for: the case that "nfs: do not cache a name from a listing that a removal overlapped" handles. Of the three, only the listing runs beside such a removal by design, since it holds the directory shared too. The other two hold no directory lock, and can meet a marked dentry only through a reference taken after it was marked, by a path that does not wait for the mark: an open by file handle, which NFSv4.1 and later mounts support. A lockless path walk that checked the dentry before it was marked fails its recheck and retries, because marking it also invalidates the dentry's sequence count. Nothing marks a dentry this way yet, so the one change is that nfs_prime_dcache() leaves alone a dentry that something else has unhashed, rather than refreshing its inode or caching a replacement for it. Signed-off-by: Jonathan Flynn --- fs/nfs/dir.c | 42 +++++++++++++++++++++++++++++++++++++++ fs/nfs/internal.h | 50 +++++++++++++++++++++++++++++++++++++++++++++++ fs/nfs/nfs4file.c | 9 ++++++++- fs/nfs/nfs4proc.c | 3 ++- 4 files changed, 102 insertions(+), 2 deletions(-) diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c index 68a1e578e1..2a4c059064 100644 --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -756,6 +756,48 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry, return; if (!d_in_lookup(dentry)) { + bool hands_off; + + /* + * Leave alone a dentry that a removal made under a shared + * parent anchors on, or that has been unhashed. readdir holds + * the parent shared as well, so it can run beside such a + * removal, and while the REMOVE is in flight neither of the + * two things done below is safe: unhashing the dentry would + * let a lookup or open of the name overtake the REMOVE, and + * refreshing its inode from this reply would advance the + * attribute generation that the remover's nfs_drop_nlink() + * compares against, so that the removed file kept its link + * count. Nor is a refresh safe once the REMOVE has succeeded, + * since this reply can still list the name: on a server that + * reports how its change attribute behaves, nfs_refresh_inode() + * takes attributes with a newer change attribute whatever + * their generation, and would put back the link count that + * the removal took away. + * + * Both are tested once, before anything else is done with the + * dentry, in a ->d_lock section of their own. This caller has + * held a counted reference since before the test, and the + * remover stores the sentinel only on a dentry whose count it + * reads as one in the section it stores in: either the remover + * sees this reference and hands the removal back, or the + * sentinel was stored first. If the REMOVE then succeeds, the + * VFS calls d_delete() before ->unlink_shared_done() clears the + * sentinel. A d_delete() that finds this reference only + * unhashes the dentry, under ->d_lock, so a test that finds the + * sentinel gone comes after that section and sees the unhash. + * One that ran before this reference was taken left the dentry + * negative instead, and the code below drops a negative dentry + * and treats the name as one it has no dentry for. -ENOENT + * ends either way too, through nfs_dentry_handle_enoent(), and + * any other failure leaves the file in place. + */ + spin_lock(&dentry->d_lock); + hands_off = nfs_dentry_is_shared_anchor(dentry) || + d_unhashed(dentry); + spin_unlock(&dentry->d_lock); + if (hands_off) + goto out; /* Is there a mountpoint here? If so, just exit */ if (!nfs_fsid_equal(&NFS_SB(dentry->d_sb)->fsid, &entry->fattr->fsid)) diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h index 7cda8b7b3f..907ea27ea2 100644 --- a/fs/nfs/internal.h +++ b/fs/nfs/internal.h @@ -102,6 +102,56 @@ static inline bool nfs_dentry_is_blocked(struct dentry *dentry) return nfs_fsdata_blocked(READ_ONCE(dentry->d_fsdata)); } +/* + * Does a removal made under a shared parent anchor on this dentry? The + * caller holds ->d_lock; READ_ONCE() as in nfs_dentry_is_blocked(). + */ +static inline bool nfs_dentry_is_shared_anchor(struct dentry *dentry) +{ + lockdep_assert_held(&dentry->d_lock); + return nfs_fsdata_blocked_shared(READ_ONCE(dentry->d_fsdata)); +} + +/** + * nfs_d_drop_unless_anchored - unhash a dentry unless a removal anchors on it + * @dentry: dentry to unhash + * + * As d_drop(), except that a dentry carrying NFS_FSDATA_BLOCKED_SHARED is + * left hashed: a removal of its name is in flight under a shared parent, + * and unhashing the dentry would let a lookup or open of the same name + * overtake the REMOVE. The test and the unhash are one ->d_lock section, + * and ->d_lock is the lock the sentinel is stored under. The sentinel is + * all that is consulted - not the parent's lock and not its operations + * table - and a further condition, if one is ever added here, has to come + * after that test, not in front of it. + * + * The callers hold a reference to the dentry and no lock on the parent. + * The remover anchors only on a dentry that nobody else holds, so they can + * meet an anchor only if that reference was taken after the sentinel was + * stored, by a path that did not then consult ->d_revalidate(), which would + * have waited. An RCU-walk that consulted ->d_revalidate() before the + * sentinel was stored is not such a path: the store is followed, in the + * same ->d_lock section, by write_seqcount_invalidate(), so the walk's + * ->d_seq recheck in legitimize_path() fails and it retries in ref-walk, + * where ->d_revalidate() waits. An open by file handle is such a path: + * its reference comes from d_obtain_alias(). + * + * The sentinel is cleared without ->d_lock, so the test can still see it + * for an instant after the REMOVE has completed and leave hashed a dentry + * that would otherwise have been dropped. That direction is safe: the + * remover's own d_delete() normally unhashes the dentry, and an open by + * path that returns -EOPENSTALE is retried by the VFS, first in ref-walk + * if it was begun in RCU-walk and, if it fails again, with LOOKUP_REVAL, + * whose ->d_revalidate() asks the server. + */ +static inline void nfs_d_drop_unless_anchored(struct dentry *dentry) +{ + spin_lock(&dentry->d_lock); + if (!nfs_dentry_is_shared_anchor(dentry)) + __d_drop(dentry); + spin_unlock(&dentry->d_lock); +} + static inline fmode_t flags_to_mode(int flags) { fmode_t res = (__force fmode_t)flags & FMODE_EXEC; diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index 9a434f5dda..ccb801ab38 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -101,7 +101,14 @@ nfs4_file_open(struct inode *inode, struct file *filp) return err; out_drop: - d_drop(dentry); + /* + * Unhash, unless a removal made under a shared parent anchors on + * the dentry. For an open by path, -EOPENSTALE makes the VFS redo + * the lookup either way, and that lookup waits for the removal in + * ->d_revalidate(); an open by file handle is not looked up again, + * as it never was. + */ + nfs_d_drop_unless_anchored(dentry); err = -EOPENSTALE; goto out_put_ctx; } diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 808bb8f1c1..fbf9e97815 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2791,8 +2791,9 @@ static int _nfs4_open_expired(struct nfs_open_context *ctx, struct nfs4_state *s */ opendata->o_arg.open_flags = O_DIRECT; ret = nfs4_open_recover(opendata, state); + /* Unhash, unless a removal under a shared parent anchors here */ if (ret == -ESTALE) - d_drop(ctx->dentry); + nfs_d_drop_unless_anchored(ctx->dentry); nfs4_opendata_put(opendata); return ret; } -- 2.43.0