Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Andreas Dilger <adilger@dilger.ca>
Message-Id: <DC74377C-DFFD-4E26-90AB-213577DB3081@dilger.ca>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_A23EAC69-E4AD-4741-A190-B2F252287FD8";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [RFC] Better page cache error handling
Date:   Wed, 24 Feb 2021 16:41:26 -0700
In-Reply-To: <20210224134115.GP2858050@casper.infradead.org>
Cc:     Jan Kara <jack@suse.cz>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        linux-mm <linux-mm@kvack.org>, linux-kernel@vger.kernel.org,
        Christoph Hellwig <hch@lst.de>,
        Kent Overstreet <kent.overstreet@gmail.com>
To:     Matthew Wilcox <willy@infradead.org>
References: <20210205161142.GI308988@casper.infradead.org>
 <20210224123848.GA27695@quack2.suse.cz>
 <20210224134115.GP2858050@casper.infradead.org>
Precedence: bulk


--Apple-Mail=_A23EAC69-E4AD-4741-A190-B2F252287FD8
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

On Feb 24, 2021, at 6:41 AM, Matthew Wilcox <willy@infradead.org> wrote:
>=20
> On Wed, Feb 24, 2021 at 01:38:48PM +0100, Jan Kara wrote:
>>> We allocate a page and try to read it.  29 threads pile up waiting
>>> for the page lock in filemap_update_page().  The error returned by =
the
>>> original I/O is shared between all 29 waiters as well as being =
returned
>>> to the requesting thread.  The next request for index.html will send
>>> another I/O, and more waiters will pile up trying to get the page =
lock,
>>> but at no time will more than 30 threads be waiting for the I/O to =
fail.
>>=20
>> Interesting idea. It certainly improves current behavior. I just =
wonder
>> whether this isn't a partial solution to a problem and a full =
solution of
>> it would have to go in a different direction? I mean it just seems
>> wrong that each reader (let's assume they just won't overlap) has to =
retry
>> the failed IO and wait for the HW to figure out it's not going to =
work.
>> Shouldn't we cache the error state with the page? And I understand =
that we
>> then also have to deal with the problem how to invalidate the error =
state
>> when the block might eventually become readable (for stuff like =
temporary
>> IO failures). That would need some signalling from the driver to the =
page
>> cache, maybe in a form of some error recovery sequence counter or =
something
>> like that. For stuff like iSCSI, multipath, or NBD it could be doable =
I
>> believe...
>=20
> That felt like a larger change than I wanted to make.  I already have
> a few big projects on my plate!
>=20
> Also, it's not clear to me that the host can necessarily figure out =
when
> a device has fixed an error -- certainly for the three cases you list
> it can be done.  I think we'd want a timer to indicate that it's worth
> retrying instead of returning the error.
>=20
> Anyway, that seems like a lot of data to cram into a struct page.  So =
I
> think my proposal is still worth pursuing while waiting for someone to
> come up with a perfect solution.

Since you would know that the page is bad at this point (not uptodate,
does not contain valid data) you could potentially re-use some other
fields in struct page, or potentially store something in the page =
itself?
That would avoid bloating struct page with fields that are only rarely
needed.  Userspace shouldn't be able to read the page at that point if
it is not marked uptodate, but they could overwrite it, so you wouldn't
want to store any kind of complex data structure there, but you _could_
store a magic, an error value, and a timeout, that are only valid if
!uptodate (cleared if the page were totally overwritten by userspace).

Yes, it's nasty, but better than growing struct page, and better than
blocking userspace threads for tens of minutes when a block is bad.

Cheers, Andreas


--Apple-Mail=_A23EAC69-E4AD-4741-A190-B2F252287FD8
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIzBAEBCAAdFiEEDb73u6ZejP5ZMprvcqXauRfMH+AFAmA25CYACgkQcqXauRfM
H+AAERAAj0OTDKFuMVf0UEx4aC5GMp8vcZnxg7PM1tYxaxK9IkCzbmIvx0Cg52lp
5QkaINbs7U16rCrNRY8gkpl9VbU5p8zEyAGfbnZqTjxz8n++vx0LvsOxMjIgzDDn
KdcgrpkMZ0oFiSXcBjoE4+kCroWA1pM7hACvDkgBAZQV3sWZANmqZCAeDLpWz1kK
q+6shGiG/w410YHh/y72g1X2e+/9oSZ7xjGHx2IxuHXh6XEPHu8h2Oy2TusPpJ7P
VEmuCRQ19eDJSUw57F03p8MdQ3kD3dhPXWquO3vv2rcCsG/CAwF54O7CWGTwdSWl
60Zutxxlgst3zZHdgnLcHquHC2yitRz8ejSc5u/PUYYK8HQJ8wGtoc87dUUJYFZg
vBzYrOIHlBmqgoQ0lUj3WMS+GvHtyeWYRybImQXZ0Q8NL10kBmtT7GdlWhWBAdy7
YEZczQUSnOuRpJenXKVOkCHzH+Asu8Hicc9QiAii6iAts/gFw/E88dGACyM5cGqN
jkc/5bjuhev2rbQgXTesFlXUb/fVDmKKKIV/mMjHWc7vx3xHbNoRPkXy2JhVA1Km
25iTbybhiZCdm+IqMVXmSOyrzeIrsWhgA/k4JBZ0LMVPmq5mu7+QzIMYzthkGxla
7CRd0zvTR1bNWlhpffPHu3kIUzdI+7QTAdNHJA7TqF22tbgod4k=
=H6NE
-----END PGP SIGNATURE-----

--Apple-Mail=_A23EAC69-E4AD-4741-A190-B2F252287FD8--