From: Gao Xiang <hsiangkao@linux.alibaba.com> To: Jeffle Xu <jefflexu@linux.alibaba.com> Cc: dhowells@redhat.com, linux-cachefs@redhat.com, xiang@kernel.org, chao@kernel.org, linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org, joseph.qi@linux.alibaba.com, bo.liu@linux.alibaba.com, tao.peng@linux.alibaba.com, gerry@linux.alibaba.com, eguan@linux.alibaba.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 05/23] netfs: add inode parameter to netfs_alloc_read_request() Date: Tue, 4 Jan 2022 22:00:00 +0800 [thread overview] Message-ID: <YdRS4AnHbWpHwl/8@B-P7TQMD6M-0146.local> (raw) In-Reply-To: <20211227125444.21187-6-jefflexu@linux.alibaba.com> On Mon, Dec 27, 2021 at 08:54:26PM +0800, Jeffle Xu wrote: > When working as the local cache, the @file parameter of > netfs_alloc_read_request() represents the backed file inside netfs. It > is for two use: 1) we can derive the corresponding inode from file, > 2) works as the argument for ops->init_rreq(). > > In the new introduced demand-read mode, netfs_readpage() will be called > by the upper fs to read from backing files. However in this new mode, > the backed file may not be opened, and thus the @file argument is NULL > in this case. > > For netfs_readpage(), @file parameter represents the backed file inside > netfs, while @folio parameter represents one page cache inside the > address space of this backed file. We can still derive the inode from > the @folio parameter, even when @file parameter is NULL. > > Thus refactor netfs_alloc_read_request() somewhat for this change. > > Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com> I'm not sure how other folks think. Yet in principle, personally I think it's reasonable that something like read_cache_page_gfp() could be used directly with fscache backend as well. So for such internal read requests, @file argument is actually optional as a common practice. Apart from the commit message itself (which I think it could be simplified a bit), generally it looks good to me. Thanks, Gao Xiang > --- > fs/netfs/read_helper.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/fs/netfs/read_helper.c b/fs/netfs/read_helper.c > index 8c58cff420ba..ca84918b6b5d 100644 > --- a/fs/netfs/read_helper.c > +++ b/fs/netfs/read_helper.c > @@ -39,7 +39,7 @@ static void netfs_put_subrequest(struct netfs_read_subrequest *subreq, > > static struct netfs_read_request *netfs_alloc_read_request( > const struct netfs_read_request_ops *ops, void *netfs_priv, > - struct file *file) > + struct inode *inode, struct file *file) > { > static atomic_t debug_ids; > struct netfs_read_request *rreq; > @@ -48,7 +48,7 @@ static struct netfs_read_request *netfs_alloc_read_request( > if (rreq) { > rreq->netfs_ops = ops; > rreq->netfs_priv = netfs_priv; > - rreq->inode = file_inode(file); > + rreq->inode = inode; > rreq->i_size = i_size_read(rreq->inode); > rreq->debug_id = atomic_inc_return(&debug_ids); > INIT_LIST_HEAD(&rreq->subrequests); > @@ -870,6 +870,7 @@ void netfs_readahead(struct readahead_control *ractl, > void *netfs_priv) > { > struct netfs_read_request *rreq; > + struct inode *inode = file_inode(ractl->file); > unsigned int debug_index = 0; > int ret; > > @@ -878,7 +879,7 @@ void netfs_readahead(struct readahead_control *ractl, > if (readahead_count(ractl) == 0) > goto cleanup; > > - rreq = netfs_alloc_read_request(ops, netfs_priv, ractl->file); > + rreq = netfs_alloc_read_request(ops, netfs_priv, inode, ractl->file); > if (!rreq) > goto cleanup; > rreq->mapping = ractl->mapping; > @@ -948,12 +949,13 @@ int netfs_readpage(struct file *file, > void *netfs_priv) > { > struct netfs_read_request *rreq; > + struct inode *inode = folio_file_mapping(folio)->host; > unsigned int debug_index = 0; > int ret; > > _enter("%lx", folio_index(folio)); > > - rreq = netfs_alloc_read_request(ops, netfs_priv, file); > + rreq = netfs_alloc_read_request(ops, netfs_priv, inode, file); > if (!rreq) { > if (netfs_priv) > ops->cleanup(folio_file_mapping(folio), netfs_priv); > @@ -1122,7 +1124,7 @@ int netfs_write_begin(struct file *file, struct address_space *mapping, > } > > ret = -ENOMEM; > - rreq = netfs_alloc_read_request(ops, netfs_priv, file); > + rreq = netfs_alloc_read_request(ops, netfs_priv, inode, file); > if (!rreq) > goto error; > rreq->mapping = folio_file_mapping(folio); > -- > 2.27.0
WARNING: multiple messages have this Message-ID (diff)
From: Gao Xiang <hsiangkao@linux.alibaba.com> To: Jeffle Xu <jefflexu@linux.alibaba.com> Cc: tao.peng@linux.alibaba.com, linux-kernel@vger.kernel.org, dhowells@redhat.com, joseph.qi@linux.alibaba.com, linux-cachefs@redhat.com, bo.liu@linux.alibaba.com, linux-fsdevel@vger.kernel.org, gerry@linux.alibaba.com, linux-erofs@lists.ozlabs.org, eguan@linux.alibaba.com Subject: Re: [PATCH v1 05/23] netfs: add inode parameter to netfs_alloc_read_request() Date: Tue, 4 Jan 2022 22:00:00 +0800 [thread overview] Message-ID: <YdRS4AnHbWpHwl/8@B-P7TQMD6M-0146.local> (raw) In-Reply-To: <20211227125444.21187-6-jefflexu@linux.alibaba.com> On Mon, Dec 27, 2021 at 08:54:26PM +0800, Jeffle Xu wrote: > When working as the local cache, the @file parameter of > netfs_alloc_read_request() represents the backed file inside netfs. It > is for two use: 1) we can derive the corresponding inode from file, > 2) works as the argument for ops->init_rreq(). > > In the new introduced demand-read mode, netfs_readpage() will be called > by the upper fs to read from backing files. However in this new mode, > the backed file may not be opened, and thus the @file argument is NULL > in this case. > > For netfs_readpage(), @file parameter represents the backed file inside > netfs, while @folio parameter represents one page cache inside the > address space of this backed file. We can still derive the inode from > the @folio parameter, even when @file parameter is NULL. > > Thus refactor netfs_alloc_read_request() somewhat for this change. > > Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com> I'm not sure how other folks think. Yet in principle, personally I think it's reasonable that something like read_cache_page_gfp() could be used directly with fscache backend as well. So for such internal read requests, @file argument is actually optional as a common practice. Apart from the commit message itself (which I think it could be simplified a bit), generally it looks good to me. Thanks, Gao Xiang > --- > fs/netfs/read_helper.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/fs/netfs/read_helper.c b/fs/netfs/read_helper.c > index 8c58cff420ba..ca84918b6b5d 100644 > --- a/fs/netfs/read_helper.c > +++ b/fs/netfs/read_helper.c > @@ -39,7 +39,7 @@ static void netfs_put_subrequest(struct netfs_read_subrequest *subreq, > > static struct netfs_read_request *netfs_alloc_read_request( > const struct netfs_read_request_ops *ops, void *netfs_priv, > - struct file *file) > + struct inode *inode, struct file *file) > { > static atomic_t debug_ids; > struct netfs_read_request *rreq; > @@ -48,7 +48,7 @@ static struct netfs_read_request *netfs_alloc_read_request( > if (rreq) { > rreq->netfs_ops = ops; > rreq->netfs_priv = netfs_priv; > - rreq->inode = file_inode(file); > + rreq->inode = inode; > rreq->i_size = i_size_read(rreq->inode); > rreq->debug_id = atomic_inc_return(&debug_ids); > INIT_LIST_HEAD(&rreq->subrequests); > @@ -870,6 +870,7 @@ void netfs_readahead(struct readahead_control *ractl, > void *netfs_priv) > { > struct netfs_read_request *rreq; > + struct inode *inode = file_inode(ractl->file); > unsigned int debug_index = 0; > int ret; > > @@ -878,7 +879,7 @@ void netfs_readahead(struct readahead_control *ractl, > if (readahead_count(ractl) == 0) > goto cleanup; > > - rreq = netfs_alloc_read_request(ops, netfs_priv, ractl->file); > + rreq = netfs_alloc_read_request(ops, netfs_priv, inode, ractl->file); > if (!rreq) > goto cleanup; > rreq->mapping = ractl->mapping; > @@ -948,12 +949,13 @@ int netfs_readpage(struct file *file, > void *netfs_priv) > { > struct netfs_read_request *rreq; > + struct inode *inode = folio_file_mapping(folio)->host; > unsigned int debug_index = 0; > int ret; > > _enter("%lx", folio_index(folio)); > > - rreq = netfs_alloc_read_request(ops, netfs_priv, file); > + rreq = netfs_alloc_read_request(ops, netfs_priv, inode, file); > if (!rreq) { > if (netfs_priv) > ops->cleanup(folio_file_mapping(folio), netfs_priv); > @@ -1122,7 +1124,7 @@ int netfs_write_begin(struct file *file, struct address_space *mapping, > } > > ret = -ENOMEM; > - rreq = netfs_alloc_read_request(ops, netfs_priv, file); > + rreq = netfs_alloc_read_request(ops, netfs_priv, inode, file); > if (!rreq) > goto error; > rreq->mapping = folio_file_mapping(folio); > -- > 2.27.0
next prev parent reply other threads:[~2022-01-04 14:00 UTC|newest] Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-27 12:54 [PATCH v1 00/23] fscache,erofs: fscache-based demand-read semantics Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 01/23] cachefiles: add cachefiles_demand devnode Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-28 2:47 ` Joseph Qi 2021-12-28 2:47 ` Joseph Qi 2021-12-28 12:34 ` JeffleXu 2021-12-28 12:34 ` JeffleXu 2021-12-27 12:54 ` [PATCH v1 02/23] cachefiles: add mode command to distinguish modes Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 03/23] cachefiles: detect backing file size in demand-read mode Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 04/23] netfs: make ops->init_rreq() optional Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 05/23] netfs: add inode parameter to netfs_alloc_read_request() Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2022-01-04 14:00 ` Gao Xiang [this message] 2022-01-04 14:00 ` Gao Xiang 2022-01-13 3:10 ` [Linux-cachefs] " JeffleXu 2022-01-13 3:10 ` JeffleXu 2022-01-13 12:09 ` Gao Xiang 2022-01-13 12:09 ` Gao Xiang 2021-12-27 12:54 ` [PATCH v1 06/23] erofs: export erofs_map_blocks() Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 07/23] erofs: add nodev mode Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2022-01-04 14:33 ` Gao Xiang 2022-01-04 14:33 ` Gao Xiang 2022-01-04 14:58 ` Gao Xiang 2022-01-04 14:58 ` Gao Xiang 2022-01-05 9:04 ` JeffleXu 2021-12-27 12:54 ` [PATCH v1 08/23] erofs: register global fscache volume Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 09/23] erofs: add cookie context helper functions Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 10/23] erofs: add anonymous inode managing page cache of blob file Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 11/23] erofs: register cookie context for bootstrap Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 12/23] erofs: implement fscache-based metadata read Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 17:07 ` kernel test robot 2021-12-27 17:07 ` kernel test robot 2021-12-27 17:07 ` kernel test robot 2021-12-27 17:17 ` kernel test robot 2021-12-27 17:17 ` kernel test robot 2021-12-27 17:17 ` kernel test robot 2021-12-27 12:54 ` [PATCH v1 13/23] erofs: implement fscache-based data read Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2022-01-03 6:32 ` JeffleXu 2022-01-03 6:32 ` JeffleXu 2022-01-04 14:40 ` Gao Xiang 2022-01-04 14:40 ` Gao Xiang 2022-01-05 2:29 ` JeffleXu 2021-12-27 12:54 ` [PATCH v1 14/23] erofs: register cookie context for data blobs Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 15/23] erofs: implement fscache-based data read " Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 16/23] erofs: add 'uuid' mount option Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 17/23] netfs: support on demand read Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 18/23] cachefiles: use idr tree managing pending " Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 19/23] cachefiles: implement .demand_read() for " Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 15:32 ` kernel test robot 2021-12-27 15:32 ` kernel test robot 2021-12-27 15:32 ` kernel test robot 2021-12-27 15:36 ` Matthew Wilcox 2021-12-27 15:36 ` Matthew Wilcox 2021-12-28 12:33 ` JeffleXu 2021-12-28 12:33 ` JeffleXu 2022-01-12 9:02 ` JeffleXu 2022-01-12 9:02 ` JeffleXu 2022-01-19 13:20 ` Matthew Wilcox 2022-01-19 13:20 ` Matthew Wilcox 2022-01-20 12:43 ` JeffleXu 2022-01-20 12:43 ` JeffleXu 2021-12-27 15:55 ` kernel test robot 2021-12-27 15:55 ` kernel test robot 2021-12-27 15:55 ` kernel test robot 2021-12-27 12:54 ` [PATCH v1 20/23] cachefiles: implement .poll() " Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 21/23] cachefiles: implement .read() " Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 22/23] cachefiles: add done command " Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu 2021-12-27 12:54 ` [PATCH v1 23/23] erofs: support on " Jeffle Xu 2021-12-27 12:54 ` Jeffle Xu
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=YdRS4AnHbWpHwl/8@B-P7TQMD6M-0146.local \ --to=hsiangkao@linux.alibaba.com \ --cc=bo.liu@linux.alibaba.com \ --cc=chao@kernel.org \ --cc=dhowells@redhat.com \ --cc=eguan@linux.alibaba.com \ --cc=gerry@linux.alibaba.com \ --cc=jefflexu@linux.alibaba.com \ --cc=joseph.qi@linux.alibaba.com \ --cc=linux-cachefs@redhat.com \ --cc=linux-erofs@lists.ozlabs.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=tao.peng@linux.alibaba.com \ --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.