From: Jaegeuk Kim <jaegeuk@kernel.org> To: Chao Yu <chao@kernel.org> Cc: Chao Yu <chao.yu@linux.dev>, linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [RFC v3] f2fs: extent cache: support unaligned extent Date: Thu, 5 Aug 2021 11:36:44 -0700 [thread overview] Message-ID: <YQwvvEh8gzEFDH1c@google.com> (raw) In-Reply-To: <bb635f86-b29b-384b-cfe0-c8b78e3c9ec5@kernel.org> On 08/05, Chao Yu wrote: > On 2021/8/5 7:45, Jaegeuk Kim wrote: > > Chao, > > > > How about this? > > https://github.com/jaegeuk/f2fs/commit/d6bbe121bc24dfabfedc07ba7cb6e921fb70ece0 > > > > I'm digging one bug in __insert_extent_tree w/ the patch tho. > > > > On 08/04, Jaegeuk Kim wrote: > > > On 08/04, Chao Yu wrote: > > > > Compressed inode may suffer read performance issue due to it can not > > > > use extent cache, so I propose to add this unaligned extent support > > > > to improve it. > > > > > > > > Currently, it only works in readonly format f2fs image. > > > > > > > > Unaligned extent: in one compressed cluster, physical block number > > > > will be less than logical block number, so we add an extra physical > > > > block length in extent info in order to indicate such extent status. > > > > > > > > The idea is if one whole cluster blocks are contiguous physically, > > > > once its mapping info was readed at first time, we will cache an > > > > unaligned (or aligned) extent info entry in extent cache, it expects > > > > that the mapping info will be hitted when rereading cluster. > > > > > > > > Merge policy: > > > > - Aligned extents can be merged. > > > > - Aligned extent and unaligned extent can not be merged. > > > > > > > > Signed-off-by: Chao Yu <chao@kernel.org> > > > > --- > > > > v3: > > > > - avoid CONFIG_F2FS_FS_COMPRESSION as much as possible > > > > - clean up codes > > > > fs/f2fs/compress.c | 24 ++++++++++++ > > > > fs/f2fs/data.c | 28 +++++++++++--- > > > > fs/f2fs/extent_cache.c | 88 +++++++++++++++++++++++++++++++++++++----- > > > > fs/f2fs/f2fs.h | 42 +++++++++++++++++--- > > > > fs/f2fs/node.c | 18 +++++++++ > > > > 5 files changed, 179 insertions(+), 21 deletions(-) > > > > > > > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c > > > > index 4aa166d3d9bf..296ff37d4b08 100644 > > > > --- a/fs/f2fs/compress.c > > > > +++ b/fs/f2fs/compress.c > > > > @@ -1719,6 +1719,30 @@ void f2fs_put_page_dic(struct page *page) > > > > f2fs_put_dic(dic); > > > > } > > > > +/* > > > > + * check whether cluster blocks are contiguous, and add extent cache entry > > > > + * only if cluster blocks are logically and physically contiguous. > > > > + */ > > > > +int f2fs_cluster_blocks_are_contiguous(struct dnode_of_data *dn) > > > > +{ > > > > + bool compressed = f2fs_data_blkaddr(dn) == COMPRESS_ADDR; > > > > + int i = compressed ? 1 : 0; > > > > + block_t first_blkaddr = data_blkaddr(dn->inode, dn->node_page, > > > > + dn->ofs_in_node + i); > > > > + > > > > + for (i += 1; i < F2FS_I(dn->inode)->i_cluster_size; i++) { > > > > + block_t blkaddr = data_blkaddr(dn->inode, dn->node_page, > > > > + dn->ofs_in_node + i); > > > > + > > > > + if (!__is_valid_data_blkaddr(blkaddr)) > > > > + break; > > > > + if (first_blkaddr + i - 1 != blkaddr) > > > > + return 0; > > The merge condition looks wrong, shouldn't be: > > if (first_blkaddr + i - compressed ? 1 : 0 != blkaddr) > return 0; Great. This works. I've queued up the patch with this fix. Thanks, > > Thanks,
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org> To: Chao Yu <chao@kernel.org> Cc: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Chao Yu <chao.yu@linux.dev> Subject: Re: [f2fs-dev] [RFC v3] f2fs: extent cache: support unaligned extent Date: Thu, 5 Aug 2021 11:36:44 -0700 [thread overview] Message-ID: <YQwvvEh8gzEFDH1c@google.com> (raw) In-Reply-To: <bb635f86-b29b-384b-cfe0-c8b78e3c9ec5@kernel.org> On 08/05, Chao Yu wrote: > On 2021/8/5 7:45, Jaegeuk Kim wrote: > > Chao, > > > > How about this? > > https://github.com/jaegeuk/f2fs/commit/d6bbe121bc24dfabfedc07ba7cb6e921fb70ece0 > > > > I'm digging one bug in __insert_extent_tree w/ the patch tho. > > > > On 08/04, Jaegeuk Kim wrote: > > > On 08/04, Chao Yu wrote: > > > > Compressed inode may suffer read performance issue due to it can not > > > > use extent cache, so I propose to add this unaligned extent support > > > > to improve it. > > > > > > > > Currently, it only works in readonly format f2fs image. > > > > > > > > Unaligned extent: in one compressed cluster, physical block number > > > > will be less than logical block number, so we add an extra physical > > > > block length in extent info in order to indicate such extent status. > > > > > > > > The idea is if one whole cluster blocks are contiguous physically, > > > > once its mapping info was readed at first time, we will cache an > > > > unaligned (or aligned) extent info entry in extent cache, it expects > > > > that the mapping info will be hitted when rereading cluster. > > > > > > > > Merge policy: > > > > - Aligned extents can be merged. > > > > - Aligned extent and unaligned extent can not be merged. > > > > > > > > Signed-off-by: Chao Yu <chao@kernel.org> > > > > --- > > > > v3: > > > > - avoid CONFIG_F2FS_FS_COMPRESSION as much as possible > > > > - clean up codes > > > > fs/f2fs/compress.c | 24 ++++++++++++ > > > > fs/f2fs/data.c | 28 +++++++++++--- > > > > fs/f2fs/extent_cache.c | 88 +++++++++++++++++++++++++++++++++++++----- > > > > fs/f2fs/f2fs.h | 42 +++++++++++++++++--- > > > > fs/f2fs/node.c | 18 +++++++++ > > > > 5 files changed, 179 insertions(+), 21 deletions(-) > > > > > > > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c > > > > index 4aa166d3d9bf..296ff37d4b08 100644 > > > > --- a/fs/f2fs/compress.c > > > > +++ b/fs/f2fs/compress.c > > > > @@ -1719,6 +1719,30 @@ void f2fs_put_page_dic(struct page *page) > > > > f2fs_put_dic(dic); > > > > } > > > > +/* > > > > + * check whether cluster blocks are contiguous, and add extent cache entry > > > > + * only if cluster blocks are logically and physically contiguous. > > > > + */ > > > > +int f2fs_cluster_blocks_are_contiguous(struct dnode_of_data *dn) > > > > +{ > > > > + bool compressed = f2fs_data_blkaddr(dn) == COMPRESS_ADDR; > > > > + int i = compressed ? 1 : 0; > > > > + block_t first_blkaddr = data_blkaddr(dn->inode, dn->node_page, > > > > + dn->ofs_in_node + i); > > > > + > > > > + for (i += 1; i < F2FS_I(dn->inode)->i_cluster_size; i++) { > > > > + block_t blkaddr = data_blkaddr(dn->inode, dn->node_page, > > > > + dn->ofs_in_node + i); > > > > + > > > > + if (!__is_valid_data_blkaddr(blkaddr)) > > > > + break; > > > > + if (first_blkaddr + i - 1 != blkaddr) > > > > + return 0; > > The merge condition looks wrong, shouldn't be: > > if (first_blkaddr + i - compressed ? 1 : 0 != blkaddr) > return 0; Great. This works. I've queued up the patch with this fix. Thanks, > > Thanks, _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2021-08-05 18:36 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-04 2:23 [RFC v3] f2fs: extent cache: support unaligned extent Chao Yu 2021-08-04 2:23 ` [f2fs-dev] " Chao Yu 2021-08-04 6:52 ` kernel test robot 2021-08-04 7:27 ` Chao Yu 2021-08-04 7:28 ` kernel test robot 2021-08-04 21:37 ` Jaegeuk Kim 2021-08-04 21:37 ` [f2fs-dev] " Jaegeuk Kim 2021-08-04 23:45 ` Jaegeuk Kim 2021-08-04 23:45 ` Jaegeuk Kim 2021-08-05 0:29 ` Chao Yu 2021-08-05 0:29 ` Chao Yu 2021-08-05 2:53 ` Jaegeuk Kim 2021-08-05 2:53 ` Jaegeuk Kim 2021-08-05 3:04 ` Chao Yu 2021-08-05 3:04 ` Chao Yu 2021-08-05 3:14 ` Chao Yu 2021-08-05 3:14 ` Chao Yu 2021-08-05 13:35 ` Chao Yu 2021-08-05 13:35 ` Chao Yu 2021-08-05 18:36 ` Jaegeuk Kim [this message] 2021-08-05 18:36 ` Jaegeuk Kim
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=YQwvvEh8gzEFDH1c@google.com \ --to=jaegeuk@kernel.org \ --cc=chao.yu@linux.dev \ --cc=chao@kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-kernel@vger.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.