| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CALpufv3fCZXM4aD9cUkOQVk6Er-GxjVXhu-ceFnPAC4Mvnnbzw@mail.gmail.com>
Date: Thu, 24 Oct 2024 17:54:31 +0800
From: yi sun <sunyibuaa@...il.com>
To: Chao Yu <chao@...nel.org>
Cc: Yi Sun <yi.sun@...soc.com>, jaegeuk@...nel.org,
linux-f2fs-devel@...ts.sourceforge.net, yi sun <sunyibuaa@...il.com>,
linux-kernel@...r.kernel.org, niuzhiguo84@...il.com, hao_hao.wang@...soc.com,
ke.wang@...soc.com
Subject: Re: [RFC PATCH 2/2] f2fs: introduce f2fs_invalidate_consecutive_blocks()
On Thu, Oct 17, 2024 at 9:40 AM Chao Yu <chao@...nel.org> wrote:
>
> On 2024/10/16 13:27, Yi Sun wrote:
> > When doing truncate, consecutive blocks in the same segment
> > can be processed at the same time. So that the efficiency of
> > doing truncate can be improved.
> >
> > Add f2fs_invalidate_compress_pages_range() only for doing truncate.
> > Add check_f2fs_invalidate_consecutive_blocks() only for doing
> > truncate and to determine whether the blocks are continuous and
> > belong to the same segment.
> >
> > Signed-off-by: Yi Sun <yi.sun@...soc.com>
> > ---
> > fs/f2fs/compress.c | 14 ++++++++++++++
> > fs/f2fs/f2fs.h | 5 +++++
> > fs/f2fs/file.c | 34 +++++++++++++++++++++++++++++++++-
> > fs/f2fs/segment.c | 25 +++++++++++++++++++++++++
> > 4 files changed, 77 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > index 7f26440e8595..70929a87e9bf 100644
> > --- a/fs/f2fs/compress.c
> > +++ b/fs/f2fs/compress.c
> > @@ -2014,6 +2014,20 @@ void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
> > } while (index < end);
> > }
> >
> > +void f2fs_invalidate_compress_pages_range(struct f2fs_sb_info *sbi,
> > + block_t blkaddr, int cnt)
> > +{
> > + if (!sbi->compress_inode)
> > + return;
> > +
> > + if (cnt < 1) {
> > + f2fs_bug_on(sbi, 1);
> > + cnt = 1;
> > + }
> > +
> > + invalidate_mapping_pages(COMPRESS_MAPPING(sbi), blkaddr, blkaddr + cnt - 1);
> > +}
> > +
> > int f2fs_init_compress_inode(struct f2fs_sb_info *sbi)
> > {
> > struct inode *inode;
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index ce00cb546f4a..99767f35678f 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -3716,6 +3716,7 @@ int f2fs_create_flush_cmd_control(struct f2fs_sb_info *sbi);
> > int f2fs_flush_device_cache(struct f2fs_sb_info *sbi);
> > void f2fs_destroy_flush_cmd_control(struct f2fs_sb_info *sbi, bool free);
> > void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr);
> > +void f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi, block_t addr, int cnt);
> > bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr);
> > int f2fs_start_discard_thread(struct f2fs_sb_info *sbi);
> > void f2fs_drop_discard_cmd(struct f2fs_sb_info *sbi);
> > @@ -4375,6 +4376,8 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > block_t blkaddr);
> > void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
> > +void f2fs_invalidate_compress_pages_range(struct f2fs_sb_info *sbi,
> > + block_t blkaddr, int cnt);
> > #define inc_compr_inode_stat(inode) \
> > do { \
> > struct f2fs_sb_info *sbi = F2FS_I_SB(inode); \
> > @@ -4432,6 +4435,8 @@ static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> > struct page *page, block_t blkaddr) { return false; }
> > static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
> > nid_t ino) { }
> > +static inline void f2fs_invalidate_compress_pages_range(struct f2fs_sb_info *sbi,
> > + block_t blkaddr, int cnt) { }
> > #define inc_compr_inode_stat(inode) do { } while (0)
> > static inline int f2fs_is_compressed_cluster(
> > struct inode *inode,
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 7057efa8ec17..634691e3b5f1 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -612,6 +612,18 @@ static int f2fs_file_open(struct inode *inode, struct file *filp)
> > return finish_preallocate_blocks(inode);
> > }
> >
> > +static bool check_f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi,
> > + block_t blkaddr1, block_t blkaddr2)
> > +{
> > + if (blkaddr2 - blkaddr1 != 1)
> > + return false;
> > +
> > + if (GET_SEGNO(sbi, blkaddr1) != GET_SEGNO(sbi, blkaddr2))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
> > {
> > struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
> > @@ -621,6 +633,9 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
> > int cluster_index = 0, valid_blocks = 0;
> > int cluster_size = F2FS_I(dn->inode)->i_cluster_size;
> > bool released = !atomic_read(&F2FS_I(dn->inode)->i_compr_blocks);
> > + block_t con_start;
> > + bool run_invalid = true;
> > + int con_cnt = 1;
> >
> > addr = get_dnode_addr(dn->inode, dn->node_page) + ofs;
> >
> > @@ -652,7 +667,24 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
> > valid_blocks++;
> > }
> >
> > - f2fs_invalidate_blocks(sbi, blkaddr);
> > + if (run_invalid)
> > + con_start = blkaddr;
> > +
> > + if (count > 1 &&
> > + check_f2fs_invalidate_consecutive_blocks(sbi, blkaddr,
> > + le32_to_cpu(*(addr + 1)))) {
> > + run_invalid = false;
> > +
> > + if (con_cnt++ == 1)
> > + con_start = blkaddr;
> > + } else {
> > + run_invalid = true;
> > + }
> > +
> > + if (run_invalid) {
> > + f2fs_invalidate_consecutive_blocks(sbi, con_start, con_cnt);
> > + con_cnt = 1;
> > + }
> >
> > if (!released || blkaddr != COMPRESS_ADDR)
> > nr_free++;
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index f118faf36d35..edb8a78985ba 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -2570,6 +2570,31 @@ void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr)
> > up_write(&sit_i->sentry_lock);
> > }
> >
> > +void f2fs_invalidate_consecutive_blocks(struct f2fs_sb_info *sbi, block_t addr, int cnt)
> > +{
> > + unsigned int segno = GET_SEGNO(sbi, addr);
> > + unsigned int segno2 = GET_SEGNO(sbi, addr + cnt - 1);
> > + struct sit_info *sit_i = SIT_I(sbi);
> > +
> > + f2fs_bug_on(sbi, addr == NULL_ADDR || segno != segno2);
> > + if (addr == NEW_ADDR || addr == COMPRESS_ADDR)
> > + return;
> > +
> > + f2fs_truncate_meta_inode_pages(sbi, addr, cnt);
> > + f2fs_invalidate_compress_pages_range(sbi, addr, cnt);
> > +
> > + /* add it into sit main buffer */
> > + down_write(&sit_i->sentry_lock);
> > +
> > + update_segment_mtime(sbi, addr, 0);
> > + update_sit_entry(sbi, addr, -cnt);
> > +
> > + /* add it into dirty seglist */
> > + locate_dirty_segment(sbi, segno);
> > +
> > + up_write(&sit_i->sentry_lock);
>
> I think it needs to clean up this patchset, what about expanding
> f2fs_invalidate_blocks() to support invalidating block address extent?
>
> Something like this:
>
> void f2fs_invalidate_blocks(struct f2fs_sb_info *sbi, block_t blkaddr,
> unsigned int len)
> {
> struct sit_info *sit_i = SIT_I(sbi);
> int i;
>
> /* TODO: do sanity check on blkaddr extent */
>
> down_write(&sit_i->sentry_lock);
>
> /* TODO: expand f2fs_invalidate_internal_cache() to invalidate blkaddr extent */
> f2fs_invalidate_internal_cache(sbi, blkaddr, len);
>
> for (i = 0; i < len; i++) {
> update_segment_mtime(sbi, blkaddr, 0);
> update_sit_entry(sbi, blkaddr, -1);
>
> /* add it into dirty seglist */
> locate_dirty_segment(sbi, GET_SEGNO(sbi, blkaddr));
> }
>
> up_write(&sit_i->sentry_lock);
> }
>
> Thanks,
>
Hi Chao,
The code structure you proposed is very good and very clear.
I retested using this code structure and found that the speed
of doing truncate also improved, but the improvement was smaller.
So it might be better to use the following code structure.
void f2fs_invalidate_blocks(... , len)
{
down_write();
// Process in segments instead of blocks.
for (i = 0; i < segment_num; i++) {
update_segment_mtime();
update_sit_entry();
/* add it into dirty seglist */
locate_dirty_segment();
}
up_write();
}
Time Comparison of rm:
original optimization(segment unit) ratio(segment unit)
7.17s 3.27s 54.39%
optimization(block unit) ratio(block unit)
5.12s 28.6%
New patches will be sent out by email after they are sorted out.
Thank you for your valuable suggestions.
> > +}
> > +
> > bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr)
> > {
> > struct sit_info *sit_i = SIT_I(sbi);
>
Powered by blists - more mailing lists