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 07/23] erofs: add nodev mode Date: Tue, 4 Jan 2022 22:33:26 +0800 [thread overview] Message-ID: <YdRattisu+ITYvvZ@B-P7TQMD6M-0146.local> (raw) In-Reply-To: <20211227125444.21187-8-jefflexu@linux.alibaba.com> On Mon, Dec 27, 2021 at 08:54:28PM +0800, Jeffle Xu wrote: > Until then erofs is exactly blockdev based filesystem. In other using > scenarios (e.g. container image), erofs needs to run upon files. > > This patch introduces a new nodev mode, in which erofs could be mounted > from a bootstrap blob file containing the complete erofs image. > > The following patch will introduce a new mount option "uuid", by which > users could specify the bootstrap blob file. > > Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com> I think the order of some patches in this patchset can be improved. Take this patch as an example. This patch introduces a new mount option called "uuid", so the kernel will just accept it (which generates a user-visible impact) after this patch but it doesn't actually work. Therefore, we actually have three different behaviors here: - kernel doesn't support "uuid" mount option completely; - kernel support "uuid" but it doesn't work; - kernel support "uuid" correctly (maybe after some random patch); Actually that is bad for bisecting since there are some commits having temporary behaviors. And we don't know which commit actually fully implements this "uuid" mount option. So personally I think the proper order is just like the bottom-up approach, and make sure each patch can be tested / bisected independently. > --- > fs/erofs/data.c | 13 ++++++++--- > fs/erofs/internal.h | 1 + > fs/erofs/super.c | 56 +++++++++++++++++++++++++++++++++------------ > 3 files changed, 53 insertions(+), 17 deletions(-) > > diff --git a/fs/erofs/data.c b/fs/erofs/data.c > index 477aaff0c832..61fa431d0713 100644 > --- a/fs/erofs/data.c > +++ b/fs/erofs/data.c > @@ -11,11 +11,18 @@ > > struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr) > { > - struct address_space *const mapping = sb->s_bdev->bd_inode->i_mapping; > + struct address_space *mapping; > struct page *page; > > - page = read_cache_page_gfp(mapping, blkaddr, > - mapping_gfp_constraint(mapping, ~__GFP_FS)); Apart from the recommendation above, if my understanding is correct, I think after we implement fscache_aops, read_cache_page_gfp() can work with proper fscache mapping. So no need to implement something like erofs_readpage_from_fscache() later (at least for the case here.) Thanks, Gao Xiang > + if (sb->s_bdev) { > + mapping = sb->s_bdev->bd_inode->i_mapping; > + page = read_cache_page_gfp(mapping, blkaddr, > + mapping_gfp_constraint(mapping, ~__GFP_FS)); > + } else { > + /* TODO: data path in nodev mode */ > + page = ERR_PTR(-EINVAL); > + } > +
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 07/23] erofs: add nodev mode Date: Tue, 4 Jan 2022 22:33:26 +0800 [thread overview] Message-ID: <YdRattisu+ITYvvZ@B-P7TQMD6M-0146.local> (raw) In-Reply-To: <20211227125444.21187-8-jefflexu@linux.alibaba.com> On Mon, Dec 27, 2021 at 08:54:28PM +0800, Jeffle Xu wrote: > Until then erofs is exactly blockdev based filesystem. In other using > scenarios (e.g. container image), erofs needs to run upon files. > > This patch introduces a new nodev mode, in which erofs could be mounted > from a bootstrap blob file containing the complete erofs image. > > The following patch will introduce a new mount option "uuid", by which > users could specify the bootstrap blob file. > > Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com> I think the order of some patches in this patchset can be improved. Take this patch as an example. This patch introduces a new mount option called "uuid", so the kernel will just accept it (which generates a user-visible impact) after this patch but it doesn't actually work. Therefore, we actually have three different behaviors here: - kernel doesn't support "uuid" mount option completely; - kernel support "uuid" but it doesn't work; - kernel support "uuid" correctly (maybe after some random patch); Actually that is bad for bisecting since there are some commits having temporary behaviors. And we don't know which commit actually fully implements this "uuid" mount option. So personally I think the proper order is just like the bottom-up approach, and make sure each patch can be tested / bisected independently. > --- > fs/erofs/data.c | 13 ++++++++--- > fs/erofs/internal.h | 1 + > fs/erofs/super.c | 56 +++++++++++++++++++++++++++++++++------------ > 3 files changed, 53 insertions(+), 17 deletions(-) > > diff --git a/fs/erofs/data.c b/fs/erofs/data.c > index 477aaff0c832..61fa431d0713 100644 > --- a/fs/erofs/data.c > +++ b/fs/erofs/data.c > @@ -11,11 +11,18 @@ > > struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr) > { > - struct address_space *const mapping = sb->s_bdev->bd_inode->i_mapping; > + struct address_space *mapping; > struct page *page; > > - page = read_cache_page_gfp(mapping, blkaddr, > - mapping_gfp_constraint(mapping, ~__GFP_FS)); Apart from the recommendation above, if my understanding is correct, I think after we implement fscache_aops, read_cache_page_gfp() can work with proper fscache mapping. So no need to implement something like erofs_readpage_from_fscache() later (at least for the case here.) Thanks, Gao Xiang > + if (sb->s_bdev) { > + mapping = sb->s_bdev->bd_inode->i_mapping; > + page = read_cache_page_gfp(mapping, blkaddr, > + mapping_gfp_constraint(mapping, ~__GFP_FS)); > + } else { > + /* TODO: data path in nodev mode */ > + page = ERR_PTR(-EINVAL); > + } > +
next prev parent reply other threads:[~2022-01-04 14:33 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 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 [this message] 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=YdRattisu+ITYvvZ@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.