Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAAih9mhU-UKGvSKBsCqRmkN6zvsD_ThofU94tCOECVJ2G9iW=w@mail.gmail.com>
  <9b3e6161f290246eb8003767b2b34596a10f5d71.camel@kernel.org>
  <CAAih9miBhTWZVt2N_39tzzOfJfjyUKeq30tD2OOQZdQhRSFhSw@mail.gmail.com>
  <cd4e8b16a8c154e6bda38021eef2c0d56badb43c.camel@kernel.org> <71b3ff942fdf6f070f6cd59f29e04081d3f94c38.camel@kernel.org>
In-Reply-To: <71b3ff942fdf6f070f6cd59f29e04081d3f94c38.camel@kernel.org>
From:   Chris Perl <cperl@janestreet.com>
Date:   Mon, 12 Jun 2023 13:49:58 -0400
Message-ID: <CAAih9mhcOq2XqL0Q0sgkrpfJudpL9knV8yq+Uk1s2mJRRxau8Q@mail.gmail.com>
Subject: Re: Too many ENOSPC errors
To:     Jeff Layton <jlayton@kernel.org>
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Matthew Wilcox <willy@infradead.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Jun 12, 2023 at 1:30=E2=80=AFPM Jeff Layton <jlayton@kernel.org> wr=
ote:
>
> On Mon, 2023-06-12 at 11:58 -0400, Jeff Layton wrote:
>
> >
> > Got it: I think I see what's happening. filemap_sample_wb_err just call=
s
> > errseq_sample, which does this:
> >
> > errseq_t errseq_sample(errseq_t *eseq)
> > {
> >         errseq_t old =3D READ_ONCE(*eseq);
> >
> >         /* If nobody has seen this error yet, then we can be the first.=
 */
> >         if (!(old & ERRSEQ_SEEN))
> >                 old =3D 0;
> >         return old;
> > }
> >
> > Because no one has seen that error yet (ERRSEQ_SEEN is clear), the writ=
e
> > ends up being the first to see it and it gets back a 0, even though the
> > error happened before the sample.
> >
> > The above behavior is what we want for the sample that we do at open()
> > time, but not what's needed for this use-case. We need a new helper tha=
t
> > samples the value regardless of whether it has already been seen:
> >
> > errseq_t errseq_peek(errseq_t *eseq)
> > {
> >       return READ_ONCE(*eseq);
> > }
> >
> > ...but we'll also need to fix up errseq_check to handle differences
> > between the SEEN bit.
> >
> > I'll see if I can spin up a patch for that. Stay tuned.
>
> This may not be fixable with the way that NFS is trying to use errseq_t.
>
> The fundamental problem is that we need to mark the errseq_t in the
> mapping as SEEN when we sample it, to ensure that a later error is
> recorded and not ignored.
>
> But...if the error hasn't been reported yet and we mark it SEEN here,
> and then a later error doesn't occur, then a later open won't have its
> errseq_t set to 0, and that unseen error could be lost.
>
> It's a bit of a pity: as originally envisioned, the errseq_t mechanism
> would provide for this sort of use case, but we added this patch not
> long after the original code went in, and it changed those semantics:
>
>     b4678df184b3 errseq: Always report a writeback error once
>
> I don't see a good way to do this using the current errseq_t mechanism,
> given these competing needs. I'll keep thinking about it though. Maybe
> we could add some sort of store and forward mechanism for fsync on NFS?
> That could get rather complex though.

Can/should it be marked SEEN when the initial close(2) from tee(1)
reports the error?

Part of the reason I had originally asked about `nfs_file_flush' (i.e.
what close(2) calls) using `file_check_and_advance_wb_err' instead of
`filemap_check_wb_err' was because I was drawn to comparing
`nfs_file_flush' against `nfs_file_fsync' as it seems like in the 3.10
based EL7 kernels, the former used to delegate to the latter (by way
of `vfs_fsync') and so they had consistent behavior, whereas now they
do not.