mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Greg KH <gregkh@linuxfoundation.org>,
	Lubos Rendek <l.rendek@proxmox.com>
Cc: stable@vger.kernel.org, regressions@lists.linux.dev,
	Chuck Lever <cel@kernel.org>,
	linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [REGRESSION] 7.2.4: nfsd sends CB_OFFLOAD with all-zero stateid, async NFSv4.2 COPY never completes
Date: Sun, 11 Oct 2026 08:07:36 -0400	[thread overview]
Message-ID: <1eca2a80d8e90375682adeadcaf8b007ae2cc720.camel@kernel.org> (raw)
In-Reply-To: <2026101121-staining-scheme-358a@gregkh>

On Sun, 2026-10-11 at 09:19 +0200, Greg KH wrote:
> On Sun, Oct 11, 2026 at 06:30:40AM +1100, Lubos Rendek wrote:
> > Since 7.2.4, an NFSv4.2 server-side COPY between two exports of a Linux
> > NFS server never completes, because nfsd sends CB_OFFLOAD with an
> > all-zero stateid.
> 
> Can you just use git bisect to determine the offending commit?
> 
> And is this also the same for 7.3-rc?
> 
> 

I took a look: I believe this is a bug in 14b978e8d05c ("nfsd: fix
stale s2s_cp_stateids IDR entry for async COPY"), which is d0beaee498e1
in mainline.

In v7.3, this bug gets fixed by f307b4f7ed17 ("nfsd: split nfsd4_copy
into transient and durable async copy objects"), which was part of the
refactor of the async COPY code.

We should probably just take Lubos' two-line fix as a separate one-off
patch for stable. Backporting the COPY code refactor is too risky IMO.

Clanker analysis follows if you're interested. I think it's largely
correct:

-----------------8<-----------------------

◆ Diagnosis confirmed: the report is correct, and this is a stable-only regression from an
  incomplete backport.

  Root cause is the order of dup_copy_fields() vs nfs4_init_copy_state() in nfsd4_copy():

  • result points at the transient &u->copy.cp_res.
  • dup_copy_fields() copies cp_res (including cb_stateid) from transient to async_copy.
  • nfsd4_send_cb_offload() builds CB_OFFLOAD from async_copy->cp_res.

  Pre-d0beaee498e1, mainline did init → write result->cb_stateid → dup_copy_fields(), so both the
  COPY reply and async_copy->cp_res.cb_stateid got the stateid. But the s2s_cp_stateids IDR stored a
  pointer to &u->copy.cp_stateid, which is transient COMPOUND buffer memory.

  d0beaee498e1 ("nfsd: fix stale s2s_cp_stateids IDR entry for async COPY", stable 14b978e8d05c)
  fixed the IDR lifetime by doing dup_copy_fields() first, then
  nfs4_init_copy_state(nn, async_copy), then writing only result->cb_stateid. That leaves
  async_copy->cp_res.cb_stateid zero, because the dup already happened. Result: COPY reply has a
  valid stateid, CB_OFFLOAD has all-zero, client can't match it in nfs42_proc_copy and hangs. This
  matches your packet table exactly.

  Mainline is fixed by f307b4f7ed17 ("nfsd: split nfsd4_copy into transient and durable async copy
  objects"), which moves the durable stateid out of the duplicated struct so it can safely do init →
  write cb_stateid → dup again. Current code in fs/nfsd/nfs4proc.c:2242 has the explicit comment
  "dup after writing cb_stateid; duplicating first would leave the callback stateid zeroed." I
  verified the callback path in fs/nfsd/nfs4proc.c:2104 copies from cp_copy.cp_res, and
  dup_copy_fields in fs/nfsd/nfs4proc.c:2051 copies cp_res.

  Your minimal stable fix is the right shape for stable. A plain reorder won't work on the old
  layout because dup_copy_fields() also copies cp_stateid, so init-then-dup would clobber
  async_copy->cp_stateid with the transient's zero value. Copying the new stateid into both
  result->cb_stateid and async_copy->cp_res.cb_stateid after init preserves the IDR fix and fixes
  the callback. The alternative is backporting the full restructuring series (f307b4f7ed17,
  80c376183411, etc.), which is heavier for stable.

  #regzbot introduced: v7.2.3..v7.2.4 looks correct. The 6.12 sync-copy observation is expected:
  that branch largely forces COPY synchronous, so it doesn't reach the async/CB_OFFLOAD path.

  I did not run the reproducer or inspect a 7.2.y tree locally; verification was by reading mainline
  and the d0beaee498e1 before/after versions in this checkout.

-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2026-10-11 12:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-10 19:30 Lubos Rendek
2026-10-11  7:19 ` Greg KH
2026-10-11 12:07   ` Jeff Layton [this message]
2026-10-11 20:33   ` Lubos Rendek
2026-10-11 16:48 ` Sasha Levin
2026-10-12  9:36   ` Fabian Grünbichler
2026-10-12 14:59     ` Sasha Levin
2026-10-13 10:15       ` Jeff Layton

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=1eca2a80d8e90375682adeadcaf8b007ae2cc720.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=cel@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=l.rendek@proxmox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=regressions@lists.linux.dev \
    --cc=stable@vger.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

all inboxes | Powered by JetHome®