From: JeffleXu <jefflexu@linux.alibaba.com> To: Jeff Layton <jlayton@kernel.org>, dhowells@redhat.com, xiang@kernel.org, chao@kernel.org, linux-cachefs@redhat.com, linux-erofs@lists.ozlabs.org Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH 1/2] fscache,cachefiles: add prepare_ondemand_read() callback Date: Fri, 4 Nov 2022 20:22:48 +0800 [thread overview] Message-ID: <71907c21-bd3f-81a1-86d6-a757e4484be2@linux.alibaba.com> (raw) In-Reply-To: <c0d893bf6f52702a0bd2056a8cb005861b8324ea.camel@kernel.org> On 11/4/22 7:18 PM, Jeff Layton wrote: > On Fri, 2022-11-04 at 15:26 +0800, Jingbo Xu wrote: >> Add prepare_ondemand_read() callback dedicated for the on-demand read >> scenario, so that callers from this scenario can be decoupled from >> netfs_io_subrequest. >> >> To reuse the hole detecting logic as mush as possible, both the >> implementation of prepare_read() and prepare_ondemand_read() inside >> Cachefiles call a common routine. >> >> In the near future, prepare_read() will get enhanced and more >> information will be needed and then returned to callers. Thus >> netfs_io_subrequest is a reasonable candidate for holding places for all >> these information needed in the internal implementation. >> >> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com> >> --- >> fs/cachefiles/io.c | 42 +++++++++++++++++++++++++------ >> include/linux/netfs.h | 7 ++++++ >> include/trace/events/cachefiles.h | 4 +-- >> 3 files changed, 43 insertions(+), 10 deletions(-) >> >> diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c >> index 000a28f46e59..6427259fcba9 100644 >> --- a/fs/cachefiles/io.c >> +++ b/fs/cachefiles/io.c >> @@ -385,16 +385,11 @@ static int cachefiles_write(struct netfs_cache_resources *cres, >> term_func, term_func_priv); >> } >> >> -/* >> - * Prepare a read operation, shortening it to a cached/uncached >> - * boundary as appropriate. >> - */ >> -static enum netfs_io_source cachefiles_prepare_read(struct netfs_io_subrequest *subreq, >> - loff_t i_size) >> +static enum netfs_io_source cachefiles_do_prepare_read(struct netfs_io_subrequest *subreq, >> + struct netfs_cache_resources *cres, >> + loff_t i_size) >> { >> enum cachefiles_prepare_read_trace why; >> - struct netfs_io_request *rreq = subreq->rreq; >> - struct netfs_cache_resources *cres = &rreq->cache_resources; >> struct cachefiles_object *object; >> struct cachefiles_cache *cache; >> struct fscache_cookie *cookie = fscache_cres_cookie(cres); >> @@ -501,6 +496,36 @@ static enum netfs_io_source cachefiles_prepare_read(struct netfs_io_subrequest * >> return ret; >> } >> >> +/* >> + * Prepare a read operation, shortening it to a cached/uncached >> + * boundary as appropriate. >> + */ >> +static enum netfs_io_source cachefiles_prepare_read(struct netfs_io_subrequest *subreq, >> + loff_t i_size) >> +{ >> + return cachefiles_do_prepare_read(subreq, >> + &subreq->rreq->cache_resources, i_size); >> +} >> + >> +/* >> + * Prepare an on-demand read operation, shortening it to a cached/uncached >> + * boundary as appropriate. >> + */ >> +static enum netfs_io_source cachefiles_prepare_ondemand_read(struct netfs_cache_resources *cres, >> + loff_t start, size_t *_len, loff_t i_size) >> +{ >> + enum netfs_io_source source; >> + struct netfs_io_subrequest subreq = { >> + .start = start, >> + .len = *_len, >> + .flags = 1 << NETFS_SREQ_ONDEMAND, >> + }; >> + > > Faking up a struct like this is sort of fragile. What if we change > cachefiles_do_prepare_read to consult other fields in this structure > later? Indeed it's not robust somehow. > > It might be best to have cachefiles_do_prepare_read take individual > start, len, and flags values instead of doing this. > I will give it a try if nobody objects this. Thank you for your suggestions :) -- Thanks, Jingbo
WARNING: multiple messages have this Message-ID (diff)
From: JeffleXu <jefflexu@linux.alibaba.com> To: Jeff Layton <jlayton@kernel.org>, dhowells@redhat.com, xiang@kernel.org, chao@kernel.org, linux-cachefs@redhat.com, linux-erofs@lists.ozlabs.org Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] fscache,cachefiles: add prepare_ondemand_read() callback Date: Fri, 4 Nov 2022 20:22:48 +0800 [thread overview] Message-ID: <71907c21-bd3f-81a1-86d6-a757e4484be2@linux.alibaba.com> (raw) In-Reply-To: <c0d893bf6f52702a0bd2056a8cb005861b8324ea.camel@kernel.org> On 11/4/22 7:18 PM, Jeff Layton wrote: > On Fri, 2022-11-04 at 15:26 +0800, Jingbo Xu wrote: >> Add prepare_ondemand_read() callback dedicated for the on-demand read >> scenario, so that callers from this scenario can be decoupled from >> netfs_io_subrequest. >> >> To reuse the hole detecting logic as mush as possible, both the >> implementation of prepare_read() and prepare_ondemand_read() inside >> Cachefiles call a common routine. >> >> In the near future, prepare_read() will get enhanced and more >> information will be needed and then returned to callers. Thus >> netfs_io_subrequest is a reasonable candidate for holding places for all >> these information needed in the internal implementation. >> >> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com> >> --- >> fs/cachefiles/io.c | 42 +++++++++++++++++++++++++------ >> include/linux/netfs.h | 7 ++++++ >> include/trace/events/cachefiles.h | 4 +-- >> 3 files changed, 43 insertions(+), 10 deletions(-) >> >> diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c >> index 000a28f46e59..6427259fcba9 100644 >> --- a/fs/cachefiles/io.c >> +++ b/fs/cachefiles/io.c >> @@ -385,16 +385,11 @@ static int cachefiles_write(struct netfs_cache_resources *cres, >> term_func, term_func_priv); >> } >> >> -/* >> - * Prepare a read operation, shortening it to a cached/uncached >> - * boundary as appropriate. >> - */ >> -static enum netfs_io_source cachefiles_prepare_read(struct netfs_io_subrequest *subreq, >> - loff_t i_size) >> +static enum netfs_io_source cachefiles_do_prepare_read(struct netfs_io_subrequest *subreq, >> + struct netfs_cache_resources *cres, >> + loff_t i_size) >> { >> enum cachefiles_prepare_read_trace why; >> - struct netfs_io_request *rreq = subreq->rreq; >> - struct netfs_cache_resources *cres = &rreq->cache_resources; >> struct cachefiles_object *object; >> struct cachefiles_cache *cache; >> struct fscache_cookie *cookie = fscache_cres_cookie(cres); >> @@ -501,6 +496,36 @@ static enum netfs_io_source cachefiles_prepare_read(struct netfs_io_subrequest * >> return ret; >> } >> >> +/* >> + * Prepare a read operation, shortening it to a cached/uncached >> + * boundary as appropriate. >> + */ >> +static enum netfs_io_source cachefiles_prepare_read(struct netfs_io_subrequest *subreq, >> + loff_t i_size) >> +{ >> + return cachefiles_do_prepare_read(subreq, >> + &subreq->rreq->cache_resources, i_size); >> +} >> + >> +/* >> + * Prepare an on-demand read operation, shortening it to a cached/uncached >> + * boundary as appropriate. >> + */ >> +static enum netfs_io_source cachefiles_prepare_ondemand_read(struct netfs_cache_resources *cres, >> + loff_t start, size_t *_len, loff_t i_size) >> +{ >> + enum netfs_io_source source; >> + struct netfs_io_subrequest subreq = { >> + .start = start, >> + .len = *_len, >> + .flags = 1 << NETFS_SREQ_ONDEMAND, >> + }; >> + > > Faking up a struct like this is sort of fragile. What if we change > cachefiles_do_prepare_read to consult other fields in this structure > later? Indeed it's not robust somehow. > > It might be best to have cachefiles_do_prepare_read take individual > start, len, and flags values instead of doing this. > I will give it a try if nobody objects this. Thank you for your suggestions :) -- Thanks, Jingbo
next prev parent reply other threads:[~2022-11-04 12:22 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-04 7:26 [PATCH 0/2] fscache,cachefiles: add prepare_ondemand_read() interface Jingbo Xu 2022-11-04 7:26 ` Jingbo Xu 2022-11-04 7:26 ` [PATCH 1/2] fscache,cachefiles: add prepare_ondemand_read() callback Jingbo Xu 2022-11-04 7:26 ` Jingbo Xu 2022-11-04 11:18 ` Jeff Layton 2022-11-04 11:18 ` Jeff Layton 2022-11-04 12:22 ` JeffleXu [this message] 2022-11-04 12:22 ` JeffleXu 2022-11-04 7:26 ` [PATCH 2/2] erofs: switch to prepare_ondemand_read() in fscache mode Jingbo Xu 2022-11-04 7:26 ` Jingbo Xu 2022-11-04 11:46 ` Jeff Layton 2022-11-04 11:46 ` Jeff Layton 2022-11-04 12:20 ` JeffleXu 2022-11-04 12:20 ` JeffleXu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=71907c21-bd3f-81a1-86d6-a757e4484be2@linux.alibaba.com \ --to=jefflexu@linux.alibaba.com \ --cc=chao@kernel.org \ --cc=dhowells@redhat.com \ --cc=jlayton@kernel.org \ --cc=linux-cachefs@redhat.com \ --cc=linux-erofs@lists.ozlabs.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=xiang@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.