Received-SPF: pass (google.com: domain of linux-nfs+bounces-710-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
	Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
	Kingdom.
	Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
In-Reply-To: <d1d4f3996f55cb98ab6297844a51bc905e2ce631.camel@kernel.org>
References: <d1d4f3996f55cb98ab6297844a51bc905e2ce631.camel@kernel.org> <20231213152350.431591-1-dhowells@redhat.com> <20231213152350.431591-37-dhowells@redhat.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: dhowells@redhat.com, Steve French <smfrench@gmail.com>,
    Matthew Wilcox <willy@infradead.org>,
    Marc Dionne <marc.dionne@auristor.com>,
    Paulo Alcantara <pc@manguebit.com>,
    Shyam Prasad N <sprasad@microsoft.com>, Tom Talpey <tom@talpey.com>,
    Dominique Martinet <asmadeus@codewreck.org>,
    Eric Van Hensbergen <ericvh@kernel.org>,
    Ilya Dryomov <idryomov@gmail.com>,
    Christian Brauner <christian@brauner.io>, linux-cachefs@redhat.com,
    linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org,
    linux-nfs@vger.kernel.org, ceph-devel@vger.kernel.org,
    v9fs@lists.linux.dev, linux-fsdevel@vger.kernel.org,
    linux-mm@kvack.org, netdev@vger.kernel.org,
    linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 36/39] netfs: Implement a write-through caching option
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1075259.1703004686.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date: Tue, 19 Dec 2023 16:51:26 +0000
Message-ID: <1075260.1703004686@warthog.procyon.org.uk>

Jeff Layton <jlayton@kernel.org> wrote:

> > This can't be used with content encryption as that may require expansi=
on of
> > the write RPC beyond the write being made.
> > =

> > This doesn't affect writes via mmap - those are written back in the no=
rmal
> > way; similarly failed writethrough writes are marked dirty and left to
> > writeback to retry.  Another option would be to simply invalidate them=
, but
> > the contents can be simultaneously accessed by read() and through mmap=
.
> > =

> =

> I do wish Linux were less of a mess in this regard. Different
> filesystems behave differently when writeback fails.

Cifs is particularly, um, entertaining in this regard as it allows the wri=
te
to fail on the server due to a checksum failure if the source data changes
during the write and then just retries it later.

> That said, the modern consensus with local filesystems is to just leave
> the pages clean when buffered writeback fails, but set a writeback error
> on the inode. That at least keeps dirty pages from stacking up in the
> cache. In the case of something like a netfs, we usually invalidate the
> inode and the pages -- netfs's usually have to spontaneously deal with
> that anyway, so we might as well.
> =

> Marking the pages dirty here should mean that they'll effectively get a
> second try at writeback, which is a change in behavior from most
> filesystems. I'm not sure it's a bad one, but writeback can take a long
> time if you have a laggy network.

I'm not sure what the best thing to do is.  If everything is doing
O_DSYNC/writethrough I/O on an inode and there is no mmap, then invalidati=
ng
the pages is probably not a bad way to deal with failure here.

> When a write has already failed once, why do you think it'll succeed on
> a second attempt (and probably with page-aligned I/O, I guess)?

See above with cifs.  I wonder if the pages being written to should be mad=
e RO
and page_mkwrite() forced to lock against DSYNC writethrough.

> Another question: when the writeback is (re)attempted, will it end up
> just doing page-aligned I/O, or is the byte range still going to be
> limited to the written range?

At the moment, it then happens exactly as it would if it wasn't doing
writethrough - so it will write partial folios if it's doing a streaming w=
rite
and will do full folios otherwise.

> The more I consider it, I think it might be a lot simpler to just "fail
> fast" here rather than remarking the write dirty.

You may be right - but, again, mmap:-/

David