[PATCH 0/1] NFSv4/flexfiles: Fix layout merge mirror check.

[PATCH 0/1] NFSv4/flexfiles: Fix layout merge mirror check.

[Jonathan Curley]

I believe this diff solves a number of EAGAIN, disconnect and livelock
issues in cases where the layout needs to be refreshed due to the
mirror state changing. ff_lseg_match_mirrors will always return true
which means we aggressively merge lsegs in a variety of cases.

The problematic interaction happens in pnfs_generic_layout_insert_lseg:

if (do_merge(lseg, lp)) {
    mark_lseg_invalid(lp, free_me);
    continue;
}

My reading of this code is if we decide that the new lseg that we are
inserting is mergeable with the existing lseg, we mutate the state of
the lseg that we are inserting and then we mark the existing cached
lseg invalid.

In the stress test results that I've reviewed, marking the lseg
invalid causes a large number of undesirable side effects. This is
because there can be large number of parallel syscalls that currently
hold a reference to that lseg.

Marking the lseg invalid generally causes the syscall to return EAGAIN
when it wakes up. I also see code paths where we RESET_TO_PNFS. I also
see lots of disconnects which I believe are coming from
ff_layout_cancel_io. One way I believe we can make it to that path is
if parallel IO calls pnfs_update_layout in the race between when we
mark_lseg_invalid after we've decided to merge but before we actually
insert it.

I think this code path could be further improved by inventing another
way of marking merged layouts. I don't think they need to be
invalidated, perhaps a less destructive state like "stale" could be
invented that lets existing IO finish before cleaning up the lseg.


Jonathan Curley (1):
  NFSv4/flexfiles: Fix layout merge mirror check.

 fs/nfs/flexfilelayout/flexfilelayout.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/1] NFSv4/flexfiles: Fix layout merge mirror check.

[Jonathan Curley]

Typo in ff_lseg_match_mirrors makes the diff ineffective. This results
in merge happening all the time. Merge happening all the time is
problematic because it marks lsegs invalid. Marking lsegs invalid
causes all outstanding IO to get restarted with EAGAIN and connections
to get closed.

Closing connections constantly triggers race conditions in the RDMA
implementation...

Fixes: 660d1eb22301c ("pNFS/flexfile: Don't merge layout segments if the mirrors don't match")
Signed-off-by: Jonathan Curley <jcurley@purestorage.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 6d9aea16ef44..addf4357610e 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -334,7 +334,7 @@ ff_lseg_match_mirrors(struct pnfs_layout_segment *l1,
 		struct pnfs_layout_segment *l2)
 {
 	const struct nfs4_ff_layout_segment *fl1 = FF_LAYOUT_LSEG(l1);
-	const struct nfs4_ff_layout_segment *fl2 = FF_LAYOUT_LSEG(l1);
+	const struct nfs4_ff_layout_segment *fl2 = FF_LAYOUT_LSEG(l2);
 	u32 i;
 
 	if (fl1->mirror_array_cnt != fl2->mirror_array_cnt)
-- 
2.34.1

Re: [PATCH 1/1] NFSv4/flexfiles: Fix layout merge mirror check.

[Trond Myklebust]

On Mon, 2025-09-08 at 17:35 +0000, Jonathan Curley wrote:
> Typo in ff_lseg_match_mirrors makes the diff ineffective. This
> results
> in merge happening all the time. Merge happening all the time is
> problematic because it marks lsegs invalid. Marking lsegs invalid
> causes all outstanding IO to get restarted with EAGAIN and
> connections
> to get closed.
> 
> Closing connections constantly triggers race conditions in the RDMA
> implementation...
> 
> Fixes: 660d1eb22301c ("pNFS/flexfile: Don't merge layout segments if
> the mirrors don't match")
> Signed-off-by: Jonathan Curley <jcurley@purestorage.com>
> ---
>  fs/nfs/flexfilelayout/flexfilelayout.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c
> b/fs/nfs/flexfilelayout/flexfilelayout.c
> index 6d9aea16ef44..addf4357610e 100644
> --- a/fs/nfs/flexfilelayout/flexfilelayout.c
> +++ b/fs/nfs/flexfilelayout/flexfilelayout.c
> @@ -334,7 +334,7 @@ ff_lseg_match_mirrors(struct pnfs_layout_segment
> *l1,
>  		struct pnfs_layout_segment *l2)
>  {
>  	const struct nfs4_ff_layout_segment *fl1 =
> FF_LAYOUT_LSEG(l1);
> -	const struct nfs4_ff_layout_segment *fl2 =
> FF_LAYOUT_LSEG(l1);
> +	const struct nfs4_ff_layout_segment *fl2 =
> FF_LAYOUT_LSEG(l2);
>  	u32 i;
>  
>  	if (fl1->mirror_array_cnt != fl2->mirror_array_cnt)

D'oh! Well spotted... That definitely deserves a fixes tag.

Thanks!
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com