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;
Message-ID: <71b3ff942fdf6f070f6cd59f29e04081d3f94c38.camel@kernel.org>
Subject: Re: Too many ENOSPC errors
From:   Jeff Layton <jlayton@kernel.org>
To:     Chris Perl <cperl@janestreet.com>
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Matthew Wilcox <willy@infradead.org>
Date:   Mon, 12 Jun 2023 13:30:45 -0400
In-Reply-To: <cd4e8b16a8c154e6bda38021eef2c0d56badb43c.camel@kernel.org>
References: <CAAih9mhU-UKGvSKBsCqRmkN6zvsD_ThofU94tCOECVJ2G9iW=w@mail.gmail.com>
         <9b3e6161f290246eb8003767b2b34596a10f5d71.camel@kernel.org>
         <CAAih9miBhTWZVt2N_39tzzOfJfjyUKeq30tD2OOQZdQhRSFhSw@mail.gmail.com>
         <cd4e8b16a8c154e6bda38021eef2c0d56badb43c.camel@kernel.org>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.3 (3.48.3-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Mon, 2023-06-12 at 11:58 -0400, Jeff Layton wrote:

>=20
> Got it: I think I see what's happening. filemap_sample_wb_err just calls
> errseq_sample, which does this:
>=20
> errseq_t errseq_sample(errseq_t *eseq)                                   =
                          =20
> {                                                                        =
                          =20
>         errseq_t old =3D READ_ONCE(*eseq);                               =
                            =20
>                                                                          =
                          =20
>         /* If nobody has seen this error yet, then we can be the first. *=
/                         =20
>         if (!(old & ERRSEQ_SEEN))                                        =
                          =20
>                 old =3D 0;                                               =
                            =20
>         return old;                                                      =
                          =20
> }                                                         =20
>=20
> Because no one has seen that error yet (ERRSEQ_SEEN is clear), the write
> ends up being the first to see it and it gets back a 0, even though the
> error happened before the sample.
>=20
> 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 that
> samples the value regardless of whether it has already been seen:
>=20
> errseq_t errseq_peek(errseq_t *eseq)
> {
> 	return READ_ONCE(*eseq);
> }
>=20
> ...but we'll also need to fix up errseq_check to handle differences
> between the SEEN bit.
>=20
> 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.

Cheers,
--=20
Jeff Layton <jlayton@kernel.org>