From: zhangqilong via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
To: Chao Yu <chao@kernel.org>, "jaegeuk@kernel.org" <jaegeuk@kernel.org>
Cc: "linux-f2fs-devel@lists.sourceforge.net"
<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [f2fs-dev] Reply: Reply: Reply: [PATCH -next 2/4] f2fs: extent cache: support extent for no-compressed file
Date: Thu, 22 Sep 2022 04:54:39 +0000 [thread overview]
Message-ID: <da6ce7cd67a64ec5a0d20d7a668d1377@huawei.com> (raw)
> On 2022/9/22 10:07, zhangqilong wrote:
> >> On 2022/9/22 0:00, zhangqilong wrote:
> >>>> On 2022/9/21 21:57, zhangqilong wrote:
> >>>>>> On 2022/9/21 20:14, zhangqilong wrote:
> >>>>>>>> On 2022/9/21 15:57, Zhang Qilong wrote:
> >>>>>>>>> No-compressed file may suffer read performance issue due to
> it
> >>>>>>>>> can't use extent cache or the largest extent in inode can't
> >>>>>>>>> covered other parts of continuous blocks in readonly format
> >>>>>>>>> f2fs
> >>>> image.
> >>>>>>>>>
> >>>>>>>>> Now it won't build extent cacge tree for no-compressed file in
> >>>>>>>>> readonly format f2fs image.
> >>>>>>>>>
> >>>>>>>>> For readonly format f2fs image, maybe the no-compressed file
> >>>> don't
> >>>>>>>>> have the largest extent, or it have more than one part which
> >>>>>>>>> have
> >>>>>>>>
> >>>>>>>> Why it can not have largest extent in f2fs_inode?
> >>>>>>>
> >>>>>>> The following several situations may occur:
> >>>>>>> 1) Wrote w/o the extent when the filesystem is read-write
> fs.
> >>>>>>>
> >>>>>>> 2) Largest extent have been drop after being re-wrote,
> >>>>>>> or it have
> >>>>>> been split to smaller parts.
> >>>>>>>
> >>>>>>> 3) The largest extent only covered one part of
> >>>>>>> continuous blocks,
> >>>>>> like:
> >>>>>>> |------parts 1(continuous blocks)-----|----not
> >>>>>> continuous---|---------------------parts 2 (continuous
> >>>>>> continuous---|blocks)-----------|---------|
> >>>>>>> The largest extent is part 2, but other parts (like
> >>>>>>> part1, ) can't be
> >>>>>> mapped in readonly format f2fs image which should have been
> >>>> mapped.
> >>>>>>
> >>>>>> largest extent of non-compressed file should be updated during
> >>>>>> sload in a ro f2fs image?
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> I am sorry, I don't fully understand what you mean. I want to show
> >>>>> that the extent of file in readonly format f2fs image could not
> >>>>> existed or
> >>>> can't covered other parts that contain continuous blocks.
> >>>>
> >>>> Well, I mean the extent should be updated due to below flow?
> during
> >>>> the file was loaded into a formated f2fs image w/ ro feature.
> >>>>
> >>>> - f2fs_sload
> >>>> - build_directory
> >>>> - f2fs_build_file
> >>>>
> >>>> if (!c.compress.enabled || (c.feature &
> >>>> cpu_to_le32(F2FS_FEATURE_RO)))
> >>>> update_largest_extent(sbi, de->ino);
> >>>>
> >>>
> >>> Hi,
> >>>
> >>> I get it. I think we could consider this flow.
> >>> But it will change the metadata of the file, is it was a little
> inappropriate?
> >>> Maybe users does not want to do that during being loaded and will
> >> refuse this change.
> >>
> >> I don't get it.
> >>
> >> IIUC, we can only load files into ro f2fs image w/ sload, and sload
> >> has updated largest extent for file, or am I missing something?
> >
> > Ah, maybe I misunderstood your idea just now. We could updated
> largest
> > extent for file when loading into ro f2fs image w/ sload, but it will
> > not solve situations 3) issue for the no-compressed file that be
> mentioned previously.
>
> Why case 3) can happen? The data blocks should be allocated
> contiguously while sload loads file into image?
That's what I thought before, it should allocated contiguously while
sload loads file into image. But I found that it may not be completely
continuous for the big file recently. It may contain several discontinuous
parts.
Thanks,
>
> Thanks,
>
> >
> > Thanks,
> >
> >>
> >> Thanks,
> >>
> >>>
> >>> Thanks,
> >>>
> >>>> Thanks,
> >>>>
> >>>>> Whether it exists or not, we will not change or update largest
> >>>>> extent
> >>>> during sload in a ro f2fs image.
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>>>
> >>>>>> Thanks,
> >>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>>
> >>>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>>
> >>>>>>>>> internally continuous blocks. So we add extent cache tree for
> >>>>>>>>> the no-compressed file in readonly format f2fs image.
> >>>>>>>>>
> >>>>>>>>> The cache policy is almost same with compressed file. The
> >>>>>>>>> difference is that, the no-compressed file part will set
> >>>>>>>>> min-number of continuous blocks F2FS_MIN_EXTENT_LEN in
> >> order
> >>>> to
> >>>>>>>>> reduce cache
> >>>>>> fragmentation.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
> >>>>>>>>> ---
> >>>>>>>>> fs/f2fs/extent_cache.c | 52
> >>>>>>>> ++++++++++++++++++++++++++++++++++--------
> >>>>>>>>> 1 file changed, 42 insertions(+), 10 deletions(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> >>>>>>>>> index
> >>>>>>>>> 387d53a61270..7e39381edca0 100644
> >>>>>>>>> --- a/fs/f2fs/extent_cache.c
> >>>>>>>>> +++ b/fs/f2fs/extent_cache.c
> >>>>>>>>> @@ -695,9 +695,12 @@ static void
> >>>>>>>> f2fs_update_extent_tree_range_compressed(struct inode
> >> *inode,
> >>>>>>>>> set_extent_info(&ei, fofs, blkaddr, llen);
> >>>>>>>>> ei.c_len = c_len;
> >>>>>>>>>
> >>>>>>>>> - if (!__try_merge_extent_node(sbi, et, &ei, prev_en,
> >> next_en))
> >>>>>>>>> + if (!__try_merge_extent_node(sbi, et, &ei, prev_en,
> >> next_en)) {
> >>>>>>>>> + if (!c_len && llen < F2FS_MIN_EXTENT_LEN)
> >>>>>>>>> + goto unlock_out;
> >>>>>>>>> __insert_extent_tree(sbi, et, &ei,
> >>>>>>>>> insert_p, insert_parent, leftmost);
> >>>>>>>>> + }
> >>>>>>>>> unlock_out:
> >>>>>>>>> write_unlock(&et->lock);
> >>>>>>>>> }
> >>>>>>>>> @@ -726,24 +729,53 @@ static unsigned int
> >>>>>>>> f2fs_cluster_blocks_are_contiguous(struct dnode_of_data
> *dn)
> >>>>>>>>> return compressed ? i - 1 : i;
> >>>>>>>>> }
> >>>>>>>>>
> >>>>>>>>> +/*
> >>>>>>>>> + * check whether normal file blocks are contiguous, and add
> >>>>>>>>> +extent cache
> >>>>>>>>> + * entry only if remained blocks are logically and physically
> >>>>>> contiguous.
> >>>>>>>>> + */
> >>>>>>>>> +static unsigned int
> f2fs_normal_blocks_are_contiguous(struct
> >>>>>>>>> +dnode_of_data *dn) {
> >>>>>>>>> + int i = 0;
> >>>>>>>>> + struct inode *inode = dn->inode;
> >>>>>>>>> + block_t first_blkaddr = data_blkaddr(inode, dn->node_page,
> >>>>>>>>> + dn->ofs_in_node);
> >>>>>>>>> + unsigned int max_blocks =
> >> ADDRS_PER_PAGE(dn->node_page,
> >>>>>> inode)
> >>>>>>>>> + - dn->ofs_in_node;
> >>>>>>>>> +
> >>>>>>>>> + for (i = 1; i < max_blocks; i++) {
> >>>>>>>>> + block_t blkaddr = data_blkaddr(inode, dn->node_page,
> >>>>>>>>> + dn->ofs_in_node + i);
> >>>>>>>>> +
> >>>>>>>>> + if (!__is_valid_data_blkaddr(blkaddr) ||
> >>>>>>>>> + first_blkaddr + i != blkaddr)
> >>>>>>>>> + return i;
> >>>>>>>>> + }
> >>>>>>>>> +
> >>>>>>>>> + return i;
> >>>>>>>>> +}
> >>>>>>>>> +
> >>>>>>>>> void f2fs_readonly_update_extent_cache(struct
> >>>> dnode_of_data
> >>>>>> *dn,
> >>>>>>>>> pgoff_t index)
> >>>>>>>>> {
> >>>>>>>>> - unsigned int c_len =
> >> f2fs_cluster_blocks_are_contiguous(dn);
> >>>>>>>>> + unsigned int c_len = 0;
> >>>>>>>>> + unsigned int llen = 0;
> >>>>>>>>> block_t blkaddr;
> >>>>>>>>>
> >>>>>>>>> - if (!c_len)
> >>>>>>>>> - return;
> >>>>>>>>> -
> >>>>>>>>> blkaddr = f2fs_data_blkaddr(dn);
> >>>>>>>>> - if (blkaddr == COMPRESS_ADDR)
> >>>>>>>>> - blkaddr = data_blkaddr(dn->inode, dn->node_page,
> >>>>>>>>> + if (f2fs_compressed_file(dn->inode)) {
> >>>>>>>>> + c_len = f2fs_cluster_blocks_are_contiguous(dn);
> >>>>>>>>> + if (!c_len)
> >>>>>>>>> + return;
> >>>>>>>>> + llen = F2FS_I(dn->inode)->i_cluster_size;
> >>>>>>>>> + if (blkaddr == COMPRESS_ADDR)
> >>>>>>>>> + blkaddr = data_blkaddr(dn->inode,
> >> dn->node_page,
> >>>>>>>>> dn->ofs_in_node + 1);
> >>>>>>>>> + } else {
> >>>>>>>>> + llen = f2fs_normal_blocks_are_contiguous(dn);
> >>>>>>>>> + }
> >>>>>>>>>
> >>>>>>>>>
> >> f2fs_update_extent_tree_range_compressed(dn->inode,
> >>>>>>>>> - index, blkaddr,
> >>>>>>>>> - F2FS_I(dn->inode)->i_cluster_size,
> >>>>>>>>> - c_len);
> >>>>>>>>> + index, blkaddr, llen, c_len);
> >>>>>>>>> }
> >>>>>>>>> #endif
> >>>>>>>>>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next reply other threads:[~2022-09-22 4:55 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-22 4:54 zhangqilong via Linux-f2fs-devel [this message]
2022-09-22 6:58 ` [f2fs-dev] Reply: Reply: Reply: [PATCH -next 2/4] f2fs: extent cache: support extent for no-compressed file Chao Yu
-- strict thread matches above, loose matches on Subject: below --
2022-09-22 9:32 zhangqilong via Linux-f2fs-devel
2022-09-22 10:14 ` Chao Yu
2022-09-22 2:07 zhangqilong via Linux-f2fs-devel
2022-09-22 2:23 ` Chao Yu
2022-09-21 16:00 zhangqilong via Linux-f2fs-devel
2022-09-22 1:51 ` Chao Yu
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=da6ce7cd67a64ec5a0d20d7a668d1377@huawei.com \
--to=linux-f2fs-devel@lists.sourceforge.net \
--cc=chao@kernel.org \
--cc=jaegeuk@kernel.org \
--cc=zhangqilong3@huawei.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).