[PATCH] NFS: defer the final superblock deactivation

[PATCH] NFS: defer the final superblock deactivation

[Mike Snitzer]

A writeback worker can self-deadlock on sb->s_umount when it drops the
last active reference to an NFS superblock:

  wb_workfn
    __writeback_inodes_wb
      super_trylock_shared(sb)          <- s_umount held shared
      writeback_sb_inodes
        nfs_writepages
          nfs_do_writepage              <- fatal error: out_launder
            nfs_write_error
              nfs_release_request
                nfs_put_lock_context
                  __put_nfs_open_context
                    nfs_sb_deactive
                      deactivate_super
                        down_write(&sb->s_umount)   <- deadlock

Open contexts pin the superblock through server->active. Once the
mount has been detached (an expired automounted submount, or a lazy
umount), the open context attached to an outstanding write request can
hold the last s_active reference. If the server then fails writes with
an error that nfs_error_is_fatal_on_server() treats as fatal,
nfs_do_writepage() releases the request inline from the flusher, and
the final deactivate_super() tries to take s_umount exclusively while
the same task already holds it shared.

This was hit on a 6.12-based client whose server began rejecting
requests with AUTH_TOOWEAK (-EACCES). The flush worker hung in
deactivate_super(), and every later automount of the same filesystem
blocked in grab_super() behind it.

Keep dropping non-final references inline, but hand the final
deactivate_super() to nfsiod so it runs without s_umount held. The work
item lives in struct nfs_server, which cannot be freed until that final
reference is dropped, and nfsiod_workqueue is drained by nfsiod_stop()
at module unload.

Commit 324d003b0cd8 ("NFS: add nfs_sb_deactive_async to avoid
deadlock") added a similar deferral for a different deadlock. It was
reverted by commit 322b2b9032f4 ("Revert "NFS: add nfs_sb_deactive_async
to avoid deadlock"") once that deadlock was fixed in the RPC layer. The
revert did not address the s_umount recursion above.

Cc: stable@vger.kernel.org [no Fixes tag due to no clear regression point]
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfs/client.c           |  1 +
 fs/nfs/internal.h         |  1 +
 fs/nfs/super.c            | 28 ++++++++++++++++++++++++++--
 include/linux/nfs_fs_sb.h |  1 +
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index dbb5131375f8d..6e85fb9d9afde 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1094,6 +1094,7 @@ struct nfs_server *nfs_alloc_server(void)
 	INIT_LIST_HEAD(&server->ss_src_copies);
 
 	atomic_set(&server->active, 0);
+	INIT_WORK(&server->deactivate_work, nfs_sb_deactive_workfn);
 	atomic_long_set(&server->nr_active_delegations, 0);
 
 	server->io_stats = nfs_alloc_iostats();
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 23e9b3d1fdd51..604bc4b0c1ebb 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -537,6 +537,7 @@ extern int __init register_nfs_fs(void);
 extern void __exit unregister_nfs_fs(void);
 extern bool nfs_sb_active(struct super_block *sb);
 extern void nfs_sb_deactive(struct super_block *sb);
+extern void nfs_sb_deactive_workfn(struct work_struct *work);
 extern int nfs_client_for_each_server(struct nfs_client *clp,
 				      int (*fn)(struct nfs_server *, void *),
 				      void *data);
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index fa284fb684746..b7ba7be44cb93 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -203,12 +203,36 @@ bool nfs_sb_active(struct super_block *sb)
 }
 EXPORT_SYMBOL_GPL(nfs_sb_active);
 
+void nfs_sb_deactive_workfn(struct work_struct *work)
+{
+	struct nfs_server *server = container_of(work, struct nfs_server,
+						 deactivate_work);
+
+	/* May free @server; do not touch it after this call. */
+	deactivate_super(server->super);
+}
+
+/*
+ * Drop the superblock reference held on behalf of server->active.
+ *
+ * The final s_active reference must not be dropped synchronously: callers
+ * such as __put_nfs_open_context() can run from writeback, where
+ * __writeback_inodes_wb() already holds sb->s_umount shared, so the
+ * down_write() in deactivate_super() would self-deadlock. Non-final
+ * references are dropped inline; the final one is handed to nfsiod.
+ *
+ * The queued work owns an s_active reference until it runs, so no other
+ * caller can see s_active == 1 and try to queue it again while pending.
+ */
 void nfs_sb_deactive(struct super_block *sb)
 {
 	struct nfs_server *server = NFS_SB(sb);
 
-	if (atomic_dec_and_test(&server->active))
-		deactivate_super(sb);
+	if (!atomic_dec_and_test(&server->active))
+		return;
+	if (atomic_add_unless(&sb->s_active, -1, 1))
+		return;
+	queue_work(nfsiod_workqueue, &server->deactivate_work);
 }
 EXPORT_SYMBOL_GPL(nfs_sb_deactive);
 
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index ea0746b515ba9..c4e6f98ccd28d 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -280,6 +280,7 @@ struct nfs_server {
 	void (*destroy)(struct nfs_server *);
 
 	atomic_t active; /* Keep trace of any activity to this server */
+	struct work_struct	deactivate_work; /* deferred final deactivate_super() */
 
 	/* mountd-related mount options */
 	struct sockaddr_storage	mountd_address;
-- 
2.43.0