From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 411F8CDB465 for ; Mon, 16 Oct 2023 15:43:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BE3308D00AA; Mon, 16 Oct 2023 11:43:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B6C138D0001; Mon, 16 Oct 2023 11:43:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9E4F18D00AA; Mon, 16 Oct 2023 11:43:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 885E98D0001 for ; Mon, 16 Oct 2023 11:43:16 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 5A3651409FB for ; Mon, 16 Oct 2023 15:43:16 +0000 (UTC) X-FDA: 81351743592.11.B4822D9 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf12.hostedemail.com (Postfix) with ESMTP id 90AE44000B for ; Mon, 16 Oct 2023 15:43:14 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=UD94NMhb; spf=pass (imf12.hostedemail.com: domain of jlayton@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jlayton@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1697470994; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=qNxyWJ25eEZ0hL7evWYlRGjCPw6Q1G+VNwIQ6DpDynE=; b=f6z0KHStt7TxZsBA4kYlxPaazWBmoSBHj/qMDr1COZITczhppbIaQ/zC+lITby3hLzTj+4 vREWG/QX0cweR1ms1ZoLb9/fx9+kucEoUkxqyyoms6FqhqhnWhJZZysnuXhnqszHKdA11E fpnJABOJEuuPORWnpyUh2XCKLroSgx4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697470994; a=rsa-sha256; cv=none; b=DBVYIfJ5oT6M2BXH0SBcrCB7/NpbC+/+lcSGZjYx8GGlePE5vT8hTvkEU1/epKIiwTzICR bqbsAgGyAHF5s1+ODeVFVByLYv6rbuKUh7tC3hALYHu4Z50BuUVQGtanLdMwk0MsPR5BZq dEkB9paJ4J6NBQI5lL/+PdrurW9Pmws= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=UD94NMhb; spf=pass (imf12.hostedemail.com: domain of jlayton@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=jlayton@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id B003E61005; Mon, 16 Oct 2023 15:43:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 70438C433C9; Mon, 16 Oct 2023 15:43:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1697470988; bh=YQSftXDjlX2jFmFmLwmBjWy0+Qa2LejmfwOCODfHmbA=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=UD94NMhbAF0Hn1+u54/EiSInIpU2roomIDJ5upRsOMwiD4gpbndghYR0Y3z/fv+4c n4RR0XUcaGCV89ZTPYhcNgqO1pzHheP28GSQknY0DyiLSf2evkv3sUV85BJr1YFviP MWbvFXCBei0EDjy7na4F6lkyEd6BpjIflzijk2BSo5n64vyvFUKUaywLeMgxM6c7Rt Z7UZn8+coDOKKDanrSAqOmPEgCwOuy9Y2hJJXS82+jQlA/Zvf9E31UGGdpKBiyMzhj WhpkDThHBqGqaCh9truLhLS1Yn9oJHo701osnpf3pIFjG/1CAVk74vRPJx13IigKTT GrIkgpaY8NKdw== Message-ID: <11ec6f637698feb04963c6a7c39a5ca80af95464.camel@kernel.org> Subject: Re: [RFC PATCH 02/53] netfs: Track the fpos above which the server has no data From: Jeff Layton To: David Howells , Steve French Cc: Matthew Wilcox , Marc Dionne , Paulo Alcantara , Ronnie Sahlberg , Shyam Prasad N , Tom Talpey , Dominique Martinet , Ilya Dryomov , Christian Brauner , 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, linux-cachefs@redhat.com Date: Mon, 16 Oct 2023 11:43:05 -0400 In-Reply-To: <20231013155727.2217781-3-dhowells@redhat.com> References: <20231013155727.2217781-1-dhowells@redhat.com> <20231013155727.2217781-3-dhowells@redhat.com> Content-Type: text/plain; charset="ISO-8859-15" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) MIME-Version: 1.0 X-Rspamd-Queue-Id: 90AE44000B X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: ujhujka3cxngbnazmgyzwum3pcxpfa5j X-HE-Tag: 1697470994-191047 X-HE-Meta: U2FsdGVkX180DUXTeR0uNfm/lQZjHEB6Qk8uJz93PL5JWwBOJL3u7hLXXI6VvLl20okLoImcqm9AxFExx+wnRcP2TIAZvfHZEfv5u7EfEWdTjTwwnKa3SmKh+HdLE71PNisDXpyAhvRhT9GcqWRsRiZYE4pKxQlMYlIc+Vo5vHkk4GwgtDvMm7ytzqK72hG+PMMNLjEgLSflchEEo8ef8BXzjIxLjMDQUNglKdL/bPu4xkX6EtaXoLUM66ZVJumN9rP3ve+nS7P2eENdq+M3jdyAJ+6/SCKy3FI3c7iBj83/w00pmWJVjlubIUwyAe74dt/w4zDCMWpQ7xYETkeukHeRwYGxaY50aV86j+3whR7tNtPBNfGzsZnfJgofGRGRLTMGbsmjO6yzJrJBJ+Wb7L1G4p5r1Tg+dkOW3bFMRyinFtx5lfkSeHkE/BEI7Cq3SA9Uy8zS83tiGXZ9rDR2AsoJjdsYWEP9RNGhKi2w/zjR/ombXE36aIS2R26dVgXSqQ1RAs+CKiKQWlvBWQ7tTTUYb0cOvQn/XoJYkmC/b2hFTzosZBR8R73o/8c+SMRid0/dlYA0MyQ/NwgdS8beJNX93QUpD7p0yDmBDSO4qFSNSYYCmPQ6XESyAqz8E9eG7oa0Q21ZdXBUpmD3GRUhUXgU0+9v99yeM9u4QHvjZ3wt3P5YKtWx86Y7yxEsHRXh+qp7OkuRXBOqwT8kwJpbE6WjLRB26QX/GZlVpggvkrapHC//aSdKesNBply9htxX4l0obwDVfBZfJBEWfuD1ixJH7EJa+tfkxg/lti7yFh9xztp1SCnvtgJNFWGGCjCcVuhqgW7Aq2T8tlrah9G1rgFu+9MfAo3ry6c9tao1hdI0zdAc8kq0Ke6BPSMtZv8DyuvYFi8dgVQr4iYGYeI4cOnUG//TRtrt7HanNhlL3jtEV3Rd1HxI9EBinzGjJvaB3JIHoBC7Cs5slK1A30z x+lpRWL9 JHtBFXBupKrx81Zpv33uJL7kZkUjn2RwpuNCsIDCYrsFP5LcHGmiqhTMhxuL3JiWb8qsGg14Iwj3tn5+arYPQ4zRcDdRuauF0EMu6EKT2rEyeFJ4pKipKk+vh7wFoILfPCNytlxRikJKhpSVLWJwbVIedgBy5xuTCCG4lzVoZXaTQYDn1dGHW4FjNz+IZHj1WzIvToJYEfjvNDFZnZxfX+M+Trk/zrUzp18IpXTG89g/pEwl6FTfeoEwsTMVgG3C/EEcfUI97U9S1HrkJAFE3h8FLY6C8od18E+C92klNRf4bH2Mitekq17CaCrYA8GIrRWrGnElxJP4cLK8= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, 2023-10-13 at 16:56 +0100, David Howells wrote: > Track the file position above which the server is not expected to have an= y > data and preemptively assume that we can simply fill blocks with zeroes > locally rather than attempting to download them - even if we've written > data back to the server. Assume that any data that was written back abov= e > that position is held in the local cache. Call this the "zero point". >=20 > Make use of this to optimise away some reads from the server. We need to > set the zero point in the following circumstances: >=20 > (1) When we see an extant remote inode and have no cache for it, we set > the zero_point to i_size. >=20 > (2) On local inode creation, we set zero_point to 0. >=20 > (3) On local truncation down, we reduce zero_point to the new i_size if > the new i_size is lower. >=20 > (4) On local truncation up, we don't change zero_point. >=20 > (5) On local modification, we don't change zero_point. >=20 > (6) On remote invalidation, we set zero_point to the new i_size. >=20 > (7) If stored data is culled from the local cache, we must set zero_poin= t > above that if the data also got written to the server. >=20 When you say culled here, it sounds like you're just throwing out the dirty cache without writing the data back. That shouldn't be allowed though, so I must be misunderstanding what you mean here. Can you explain? > (8) If dirty data is written back to the server, but not the local cache= , > we must set zero_point above that. >=20 How do you write back without writing to the local cache? I'm guessing this means you're doing a non-buffered write? > Assuming the above, any read from the server at or above the zero_point > position will return all zeroes. >=20 > The zero_point value can be stored in the cache, provided the above rules > are applied to it by any code that culls part of the local cache. >=20 > Signed-off-by: David Howells > cc: Jeff Layton > cc: linux-cachefs@redhat.com > cc: linux-fsdevel@vger.kernel.org > cc: linux-mm@kvack.org > --- > fs/afs/inode.c | 13 +++++++------ > fs/netfs/buffered_read.c | 40 +++++++++++++++++++++++++--------------- > include/linux/netfs.h | 5 +++++ > 3 files changed, 37 insertions(+), 21 deletions(-) >=20 > diff --git a/fs/afs/inode.c b/fs/afs/inode.c > index 1c794a1896aa..46bc5574d6f5 100644 > --- a/fs/afs/inode.c > +++ b/fs/afs/inode.c > @@ -252,6 +252,7 @@ static void afs_apply_status(struct afs_operation *op= , > vnode->netfs.remote_i_size =3D status->size; > if (change_size) { > afs_set_i_size(vnode, status->size); > + vnode->netfs.zero_point =3D status->size; > inode_set_ctime_to_ts(inode, t); > inode->i_atime =3D t; > } > @@ -865,17 +866,17 @@ static void afs_setattr_success(struct afs_operatio= n *op) > static void afs_setattr_edit_file(struct afs_operation *op) > { > struct afs_vnode_param *vp =3D &op->file[0]; > - struct inode *inode =3D &vp->vnode->netfs.inode; > + struct afs_vnode *vnode =3D vp->vnode; > =20 > if (op->setattr.attr->ia_valid & ATTR_SIZE) { > loff_t size =3D op->setattr.attr->ia_size; > loff_t i_size =3D op->setattr.old_i_size; > =20 > - if (size < i_size) > - truncate_pagecache(inode, size); > - if (size !=3D i_size) > - fscache_resize_cookie(afs_vnode_cache(vp->vnode), > - vp->scb.status.size); > + if (size !=3D i_size) { > + truncate_pagecache(&vnode->netfs.inode, size); > + netfs_resize_file(&vnode->netfs, size); > + fscache_resize_cookie(afs_vnode_cache(vnode), size); > + } Isn't this an existing bug? AFS is not setting remote_i_size in the setattr path currently? I think this probably ought to be done in a preliminary AFS patch. > > } > } > =20 > diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c > index 2cd3ccf4c439..a2852fa64ad0 100644 > --- a/fs/netfs/buffered_read.c > +++ b/fs/netfs/buffered_read.c > @@ -147,6 +147,22 @@ static void netfs_rreq_expand(struct netfs_io_reques= t *rreq, > } > } > =20 > +/* > + * Begin an operation, and fetch the stored zero point value from the co= okie if > + * available. > + */ > +static int netfs_begin_cache_operation(struct netfs_io_request *rreq, > + struct netfs_inode *ctx) > +{ > + int ret =3D -ENOBUFS; > + > + if (ctx->ops->begin_cache_operation) { > + ret =3D ctx->ops->begin_cache_operation(rreq); > + /* TODO: Get the zero point value from the cache */ > + } > + return ret; > +} > + > /** > * netfs_readahead - Helper to manage a read request > * @ractl: The description of the readahead request > @@ -180,11 +196,9 @@ void netfs_readahead(struct readahead_control *ractl= ) > if (IS_ERR(rreq)) > return; > =20 > - if (ctx->ops->begin_cache_operation) { > - ret =3D ctx->ops->begin_cache_operation(rreq); > - if (ret =3D=3D -ENOMEM || ret =3D=3D -EINTR || ret =3D=3D -ERESTARTSYS= ) > - goto cleanup_free; > - } > + ret =3D netfs_begin_cache_operation(rreq, ctx); > + if (ret =3D=3D -ENOMEM || ret =3D=3D -EINTR || ret =3D=3D -ERESTARTSYS) > + goto cleanup_free; > =20 > netfs_stat(&netfs_n_rh_readahead); > trace_netfs_read(rreq, readahead_pos(ractl), readahead_length(ractl), > @@ -238,11 +252,9 @@ int netfs_read_folio(struct file *file, struct folio= *folio) > goto alloc_error; > } > =20 > - if (ctx->ops->begin_cache_operation) { > - ret =3D ctx->ops->begin_cache_operation(rreq); > - if (ret =3D=3D -ENOMEM || ret =3D=3D -EINTR || ret =3D=3D -ERESTARTSYS= ) > - goto discard; > - } > + ret =3D netfs_begin_cache_operation(rreq, ctx); > + if (ret =3D=3D -ENOMEM || ret =3D=3D -EINTR || ret =3D=3D -ERESTARTSYS) > + goto discard; > =20 > netfs_stat(&netfs_n_rh_readpage); > trace_netfs_read(rreq, rreq->start, rreq->len, netfs_read_trace_readpag= e); > @@ -390,11 +402,9 @@ int netfs_write_begin(struct netfs_inode *ctx, > rreq->no_unlock_folio =3D folio_index(folio); > __set_bit(NETFS_RREQ_NO_UNLOCK_FOLIO, &rreq->flags); > =20 > - if (ctx->ops->begin_cache_operation) { > - ret =3D ctx->ops->begin_cache_operation(rreq); > - if (ret =3D=3D -ENOMEM || ret =3D=3D -EINTR || ret =3D=3D -ERESTARTSYS= ) > - goto error_put; > - } > + ret =3D netfs_begin_cache_operation(rreq, ctx); > + if (ret =3D=3D -ENOMEM || ret =3D=3D -EINTR || ret =3D=3D -ERESTARTSYS) > + goto error_put; > =20 > netfs_stat(&netfs_n_rh_write_begin); > trace_netfs_read(rreq, pos, len, netfs_read_trace_write_begin); > diff --git a/include/linux/netfs.h b/include/linux/netfs.h > index b447cb67f599..282511090ead 100644 > --- a/include/linux/netfs.h > +++ b/include/linux/netfs.h > @@ -129,6 +129,8 @@ struct netfs_inode { > struct fscache_cookie *cache; > #endif > loff_t remote_i_size; /* Size of the remote file */ > + loff_t zero_point; /* Size after which we assume there's no data > + * on the server */ While I understand the concept, I'm not yet sure I understand how this new value will be used. It might be better to merge this patch in with the patch that adds the first user of this data. > }; > =20 > /* > @@ -330,6 +332,7 @@ static inline void netfs_inode_init(struct netfs_inod= e *ctx, > { > ctx->ops =3D ops; > ctx->remote_i_size =3D i_size_read(&ctx->inode); > + ctx->zero_point =3D ctx->remote_i_size; > #if IS_ENABLED(CONFIG_FSCACHE) > ctx->cache =3D NULL; > #endif > @@ -345,6 +348,8 @@ static inline void netfs_inode_init(struct netfs_inod= e *ctx, > static inline void netfs_resize_file(struct netfs_inode *ctx, loff_t new= _i_size) > { > ctx->remote_i_size =3D new_i_size; > + if (new_i_size < ctx->zero_point) > + ctx->zero_point =3D new_i_size; > } > =20 > /** >=20 --=20 Jeff Layton