From: Mike Snitzer <snitzer@kernel.org>
To: NeilBrown <neil@brown.name>
Cc: "Trond Myklebust" <trondmy@kernel.org>,
"Anna Schumaker" <anna@kernel.org>,
"Pali Rohár" <pali@kernel.org>,
"Vincent Mailhol" <mailhol.vincent@wanadoo.fr>,
"Chuck Lever" <chuck.lever@oracle.com>,
"Jeff Layton" <jlayton@kernel.org>,
linux-nfs@vger.kernel.org
Subject: Re: [RFC PATCH for 6.16-rcX] Revert "nfs_localio: change nfsd_file_put_local() to take a pointer to __rcu pointer"
Date: Mon, 14 Jul 2025 10:45:59 -0400 [thread overview]
Message-ID: <aHUYJ-C0-d64xQon@kernel.org> (raw)
In-Reply-To: <175246504876.2234665.13723785598314130070@noble.neil.brown.name>
On Mon, Jul 14, 2025 at 01:50:48PM +1000, NeilBrown wrote:
> On Wed, 09 Jul 2025, Mike Snitzer wrote:
> > [Preface: this revert makes it much less likely to "lose the race",
> > whereby causing nfsd_shutdown_net() to hang, so we'd do well to take
> > the time/care to properly fix whatever is lacking in Neil's commit
> > c25a89770d1f]
>
> Was this the first time you posted on this issue? If so it seem strange
> to start a discussion with a revert with out a clear undertstanding of
> the problem...
Might seem strange, but it seems strange for code to have gotten
upstream without having been properly tested to reveal such basic
issues. And when I embarked on what should've been a quick revert,
only to find that the series of changes weren't even bisect safe, that
only gave me more justification to rip all the code out in the hopes
of restoring known solid LOCALIO functionality (from v6.14).
>
> Maybe
>
> --- a/fs/nfs_common/nfslocalio.c
> +++ b/fs/nfs_common/nfslocalio.c
> @@ -177,7 +177,7 @@ static bool nfs_uuid_put(nfs_uuid_t *nfs_uuid)
> /* nfs_close_local_fh() is doing the
> * close and we must wait. until it unlinks
> */
> - wait_var_event_spinlock(nfl,
> + wait_var_event_spinlock(nfl->nfs_uuid,
> list_first_entry_or_null(
> &nfs_uuid->files,
> struct nfs_file_localio,
>
>
> will fix the problem - I'm waiting on the wrong address, which could
> cause various things to hang.
Yes, as I just replied to your official patch posting for this, I will
test. But the "maybe" nature lacks confidence and that needs to
improve. ;) Are you able to test LOCALIO?
I'll work to get better at making time to treat your LOCALIO changes
as if they are my own and to fully embrace associated review/test
work. But that work never quite reaches the same level of investment
for me to do so as when I develop the change.. maybe "normal" but I
need to get past that. We're in this together!
Thanks for your time on this Neil!
Mike
next prev parent reply other threads:[~2025-07-14 14:46 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-09 0:46 [PATCH 0/6 v2] nfs_localio: fixes for races and errors from older compilers NeilBrown
2025-05-09 0:46 ` [PATCH 1/6] nfs_localio: use cmpxchg() to install new nfs_file_localio NeilBrown
2025-05-09 0:46 ` [PATCH 2/6] nfs_localio: always hold nfsd net ref with nfsd_file ref NeilBrown
2025-05-09 0:46 ` [PATCH 3/6] nfs_localio: simplify interface to nfsd for getting nfsd_file NeilBrown
2025-05-09 0:46 ` [PATCH 4/6] nfs_localio: duplicate nfs_close_local_fh() NeilBrown
2025-05-09 0:46 ` [PATCH 5/6] nfs_localio: protect race between nfs_uuid_put() and nfs_close_local_fh() NeilBrown
2025-05-09 0:46 ` [PATCH 6/6] nfs_localio: change nfsd_file_put_local() to take a pointer to __rcu pointer NeilBrown
2025-05-09 11:03 ` kernel test robot
2025-07-08 14:20 ` [RFC PATCH for 6.16-rcX] Revert "nfs_localio: change nfsd_file_put_local() to take a pointer to __rcu pointer" Mike Snitzer
2025-07-14 3:13 ` [for-6.16-final PATCH 0/9] NFSD/NFS/LOCALIO: stable fixes and revert 6.16 LOCALIO changes Mike Snitzer
2025-07-14 3:13 ` [for-6.16-final PATCH 1/9] Revert "NFSD: Clean up kdoc for nfsd_open_local_fh()" Mike Snitzer
2025-07-14 3:13 ` [for-6.16-final PATCH 2/9] Revert "nfs_localio: change nfsd_file_put_local() to take a pointer to __rcu pointer" Mike Snitzer
2025-07-14 3:13 ` [for-6.16-final PATCH 3/9] Revert "nfs_localio: protect race between nfs_uuid_put() and nfs_close_local_fh()" Mike Snitzer
2025-07-14 3:13 ` [for-6.16-final PATCH 4/9] Revert "nfs_localio: duplicate nfs_close_local_fh()" Mike Snitzer
2025-07-14 3:13 ` [for-6.16-final PATCH 5/9] Revert "nfs_localio: simplify interface to nfsd for getting nfsd_file" Mike Snitzer
2025-07-14 3:13 ` [for-6.16-final PATCH 6/9] Revert "nfs_localio: always hold nfsd net ref with nfsd_file ref" Mike Snitzer
2025-07-14 3:13 ` [for-6.16-final PATCH 7/9] Revert "nfs_localio: use cmpxchg() to install new nfs_file_localio" Mike Snitzer
2025-07-14 3:13 ` [for-6.16-final PATCH 8/9] nfs/localio: avoid bouncing LOCALIO if nfs_client_is_local() Mike Snitzer
2025-07-14 4:19 ` NeilBrown
2025-07-14 14:37 ` Mike Snitzer
2025-07-14 12:23 ` Jeff Layton
2025-07-14 3:13 ` [for-6.16-final PATCH 9/9] nfs/localio: add localio_async_probe modparm Mike Snitzer
2025-07-14 4:23 ` NeilBrown
2025-07-14 12:28 ` Jeff Layton
2025-07-14 14:08 ` Mike Snitzer
2025-07-14 3:50 ` [RFC PATCH for 6.16-rcX] Revert "nfs_localio: change nfsd_file_put_local() to take a pointer to __rcu pointer" NeilBrown
2025-07-14 14:45 ` Mike Snitzer [this message]
2025-07-15 22:52 ` [PATCH 0/3] Fix localio hangs Trond Myklebust
2025-07-15 22:52 ` [PATCH 1/3] NFS/localio: nfs_close_local_fh() fix check for file closed Trond Myklebust
2025-07-15 22:52 ` [PATCH 2/3] NFS/localio: nfs_uuid_put() fix the wait for file unlink events Trond Myklebust
2025-07-15 22:52 ` [PATCH 3/3] NFS/localio: nfs_uuid_put() fix the wake up after unlinking the file Trond Myklebust
2025-07-16 1:09 ` [PATCH 1/3] NFS/localio: nfs_close_local_fh() fix check for file closed NeilBrown
2025-07-16 1:22 ` [PATCH 2/3] NFS/localio: nfs_uuid_put() fix the wait for file unlink events NeilBrown
2025-07-16 2:29 ` Trond Myklebust
2025-07-16 3:51 ` NeilBrown
2025-07-16 1:31 ` [PATCH 3/3] NFS/localio: nfs_uuid_put() fix the wake up after unlinking the file NeilBrown
2025-07-16 4:17 ` Trond Myklebust
2025-07-16 5:07 ` NeilBrown
2025-07-16 15:19 ` Trond Myklebust
2025-07-16 15:59 ` [PATCH v2 0/3] Fix localio hangs Trond Myklebust
2025-07-16 15:59 ` [PATCH v2 1/3] NFS/localio: nfs_close_local_fh() fix check for file closed Trond Myklebust
2025-07-16 15:59 ` [PATCH v2 2/3] NFS/localio: nfs_uuid_put() fix races with nfs_open/close_local_fh() Trond Myklebust
2025-07-16 15:59 ` [PATCH v2 3/3] NFS/localio: nfs_uuid_put() fix the wake up after unlinking the file Trond Myklebust
2025-07-16 22:09 ` [PATCH v2 0/3] Fix localio hangs NeilBrown
2025-07-16 23:27 ` Mike Snitzer
2025-07-18 0:18 ` NeilBrown
2025-05-09 16:01 ` [PATCH 0/6 v2] nfs_localio: fixes for races and errors from older compilers Chuck Lever
2025-05-09 21:02 ` Mike Snitzer
2025-05-10 0:16 ` Paul E. McKenney
2025-05-10 2:44 ` NeilBrown
2025-05-10 3:01 ` NeilBrown
2025-05-10 16:02 ` Chuck Lever
2025-05-10 19:57 ` Mike Snitzer
2025-05-16 15:33 ` Chuck Lever
2025-05-18 10:46 ` Pali Rohár
2025-05-19 3:49 ` NeilBrown
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=aHUYJ-C0-d64xQon@kernel.org \
--to=snitzer@kernel.org \
--cc=anna@kernel.org \
--cc=chuck.lever@oracle.com \
--cc=jlayton@kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=mailhol.vincent@wanadoo.fr \
--cc=neil@brown.name \
--cc=pali@kernel.org \
--cc=trondmy@kernel.org \
/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