| Message ID | 163819612321.215744.9738308885948264476.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fscache, cachefiles: Rewrite | expand |
On 11/29/21 10:28 PM, David Howells wrote: > Pass more information to the cache on how to deal with a hole if it > encounters one when trying to read from the cache. Three options are > provided: > > (1) NETFS_READ_HOLE_IGNORE. Read the hole along with the data, assuming > it to be a punched-out extent by the backing filesystem. > > (2) NETFS_READ_HOLE_CLEAR. If there's a hole, erase the requested region > of the cache and clear the read buffer. > > (3) NETFS_READ_HOLE_FAIL. Fail the read if a hole is detected. > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: linux-cachefs@redhat.com > --- > > fs/netfs/read_helper.c | 8 ++++---- > include/linux/netfs.h | 11 ++++++++++- > 2 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/fs/netfs/read_helper.c b/fs/netfs/read_helper.c > index 7dc79fa8a1f3..08df413efdf3 100644 > --- a/fs/netfs/read_helper.c > +++ b/fs/netfs/read_helper.c > @@ -170,7 +170,7 @@ static void netfs_cache_read_terminated(void *priv, ssize_t transferred_or_error > */ > static void netfs_read_from_cache(struct netfs_read_request *rreq, > struct netfs_read_subrequest *subreq, > - bool seek_data) > + enum netfs_read_from_hole read_hole) > { > struct netfs_cache_resources *cres = &rreq->cache_resources; > struct iov_iter iter; > @@ -180,7 +180,7 @@ static void netfs_read_from_cache(struct netfs_read_request *rreq, > subreq->start + subreq->transferred, > subreq->len - subreq->transferred); > > - cres->ops->read(cres, subreq->start, &iter, seek_data, > + cres->ops->read(cres, subreq->start, &iter, read_hole, > netfs_cache_read_terminated, subreq); > } > > @@ -466,7 +466,7 @@ static void netfs_rreq_short_read(struct netfs_read_request *rreq, > netfs_get_read_subrequest(subreq); > atomic_inc(&rreq->nr_rd_ops); > if (subreq->source == NETFS_READ_FROM_CACHE) > - netfs_read_from_cache(rreq, subreq, true); > + netfs_read_from_cache(rreq, subreq, NETFS_READ_HOLE_CLEAR); Hi I'm not sure why NETFS_READ_HOLE_CLEAR style should be used in 'short read' case. Besides, ``` static void netfs_read_from_cache(struct netfs_read_request *rreq, struct netfs_read_subrequest *subreq, enum netfs_read_from_hole read_hole) { struct netfs_cache_resources *cres = &rreq->cache_resources; struct iov_iter iter; netfs_stat(&netfs_n_rh_read); iov_iter_xarray(&iter, READ, &rreq->mapping->i_pages, subreq->start + subreq->transferred, subreq->len - subreq->transferred); cres->ops->read(cres, subreq->start, &iter, read_hole, netfs_cache_read_terminated, subreq); } ``` I'm not sure why 'subreq->start' is not incremented with 'subreq->transferred' when calling cres->ops->read() in 'short read' case. > else > netfs_read_from_server(rreq, subreq); > } > @@ -794,7 +794,7 @@ static bool netfs_rreq_submit_slice(struct netfs_read_request *rreq, > netfs_read_from_server(rreq, subreq); > break; > case NETFS_READ_FROM_CACHE: > - netfs_read_from_cache(rreq, subreq, false); > + netfs_read_from_cache(rreq, subreq, NETFS_READ_HOLE_IGNORE); > break; > default: > BUG(); > diff --git a/include/linux/netfs.h b/include/linux/netfs.h > index 5a46fde65759..b46c39d98bbd 100644 > --- a/include/linux/netfs.h > +++ b/include/linux/netfs.h > @@ -196,6 +196,15 @@ struct netfs_read_request_ops { > void (*cleanup)(struct address_space *mapping, void *netfs_priv); > }; > > +/* > + * How to handle reading from a hole. > + */ > +enum netfs_read_from_hole { > + NETFS_READ_HOLE_IGNORE, > + NETFS_READ_HOLE_CLEAR, > + NETFS_READ_HOLE_FAIL, > +}; > + > /* > * Table of operations for access to a cache. This is obtained by > * rreq->ops->begin_cache_operation(). > @@ -208,7 +217,7 @@ struct netfs_cache_ops { > int (*read)(struct netfs_cache_resources *cres, > loff_t start_pos, > struct iov_iter *iter, > - bool seek_data, > + enum netfs_read_from_hole read_hole, > netfs_io_terminated_t term_func, > void *term_func_priv); > >
JeffleXu <jefflexu@linux.alibaba.com> wrote: > > @@ -466,7 +466,7 @@ static void netfs_rreq_short_read(struct netfs_read_request *rreq, > > netfs_get_read_subrequest(subreq); > > atomic_inc(&rreq->nr_rd_ops); > > if (subreq->source == NETFS_READ_FROM_CACHE) > > - netfs_read_from_cache(rreq, subreq, true); > > + netfs_read_from_cache(rreq, subreq, NETFS_READ_HOLE_CLEAR); > > Hi I'm not sure why NETFS_READ_HOLE_CLEAR style should be used in 'short > read' case. The cache backing filesystem (eg. ext4) might have excised a chunk of zeros from the cache in order to optimise its extent list. This instructs the cache to zero over the cracks. Actually, I need to think a bit further on this. This was written assuming that the cache tracks its content independently - but those patches are not in with this set. > I'm not sure why 'subreq->start' is not incremented with > 'subreq->transferred' when calling cres->ops->read() in 'short read' case. subreq->start shouldn't get changed. subreq->transferred is sufficient. David
diff --git a/fs/netfs/read_helper.c b/fs/netfs/read_helper.c index 7dc79fa8a1f3..08df413efdf3 100644 --- a/fs/netfs/read_helper.c +++ b/fs/netfs/read_helper.c @@ -170,7 +170,7 @@ static void netfs_cache_read_terminated(void *priv, ssize_t transferred_or_error */ static void netfs_read_from_cache(struct netfs_read_request *rreq, struct netfs_read_subrequest *subreq, - bool seek_data) + enum netfs_read_from_hole read_hole) { struct netfs_cache_resources *cres = &rreq->cache_resources; struct iov_iter iter; @@ -180,7 +180,7 @@ static void netfs_read_from_cache(struct netfs_read_request *rreq, subreq->start + subreq->transferred, subreq->len - subreq->transferred); - cres->ops->read(cres, subreq->start, &iter, seek_data, + cres->ops->read(cres, subreq->start, &iter, read_hole, netfs_cache_read_terminated, subreq); } @@ -466,7 +466,7 @@ static void netfs_rreq_short_read(struct netfs_read_request *rreq, netfs_get_read_subrequest(subreq); atomic_inc(&rreq->nr_rd_ops); if (subreq->source == NETFS_READ_FROM_CACHE) - netfs_read_from_cache(rreq, subreq, true); + netfs_read_from_cache(rreq, subreq, NETFS_READ_HOLE_CLEAR); else netfs_read_from_server(rreq, subreq); } @@ -794,7 +794,7 @@ static bool netfs_rreq_submit_slice(struct netfs_read_request *rreq, netfs_read_from_server(rreq, subreq); break; case NETFS_READ_FROM_CACHE: - netfs_read_from_cache(rreq, subreq, false); + netfs_read_from_cache(rreq, subreq, NETFS_READ_HOLE_IGNORE); break; default: BUG(); diff --git a/include/linux/netfs.h b/include/linux/netfs.h index 5a46fde65759..b46c39d98bbd 100644 --- a/include/linux/netfs.h +++ b/include/linux/netfs.h @@ -196,6 +196,15 @@ struct netfs_read_request_ops { void (*cleanup)(struct address_space *mapping, void *netfs_priv); }; +/* + * How to handle reading from a hole. + */ +enum netfs_read_from_hole { + NETFS_READ_HOLE_IGNORE, + NETFS_READ_HOLE_CLEAR, + NETFS_READ_HOLE_FAIL, +}; + /* * Table of operations for access to a cache. This is obtained by * rreq->ops->begin_cache_operation(). @@ -208,7 +217,7 @@ struct netfs_cache_ops { int (*read)(struct netfs_cache_resources *cres, loff_t start_pos, struct iov_iter *iter, - bool seek_data, + enum netfs_read_from_hole read_hole, netfs_io_terminated_t term_func, void *term_func_priv);
Pass more information to the cache on how to deal with a hole if it encounters one when trying to read from the cache. Three options are provided: (1) NETFS_READ_HOLE_IGNORE. Read the hole along with the data, assuming it to be a punched-out extent by the backing filesystem. (2) NETFS_READ_HOLE_CLEAR. If there's a hole, erase the requested region of the cache and clear the read buffer. (3) NETFS_READ_HOLE_FAIL. Fail the read if a hole is detected. Signed-off-by: David Howells <dhowells@redhat.com> cc: linux-cachefs@redhat.com --- fs/netfs/read_helper.c | 8 ++++---- include/linux/netfs.h | 11 ++++++++++- 2 files changed, 14 insertions(+), 5 deletions(-)