On 2021/11/10 9:41, 常凤楠 wrote:
-----Original Message-----
From: changfeng...@vivo.com <changfeng...@vivo.com> On Behalf Of
Chao Yu
Sent: Tuesday, November 9, 2021 9:46 PM
To: 常凤楠 <changfeng...@vivo.com>; jaeg...@kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Subject: Re: Do we need serial io for compress file?

On 2021/11/9 11:18, 常凤楠 wrote:


-----Original Message-----
From: changfeng...@vivo.com <changfeng...@vivo.com> On Behalf
Of Chao
Yu
Sent: Monday, November 8, 2021 10:30 PM
To: 常凤楠 <changfeng...@vivo.com>; jaeg...@kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Subject: Re: Do we need serial io for compress file?

On 2021/11/8 16:56, 常凤楠 wrote:
Anyway, I did some modify to verify my idea, and did some test, not
found problem for now.

Could you please consider:
1. pin file
2. fallocate file w/ filesize keeped
    - it will preallocate physical blocks aligned to segments 3. unpin file
4.
overwrite compressed file

So you means after step 1-3, it will make compressed file fragmentation
worse ?

Oh, I'm trying to find a way to avoid write performance regression due to
race condition on writepages lock meanwhile avoiding fragmentation of
compressed file.

Yep, that's what I want to do, looking forward your idea.
And how about the modify as below ?

We will still suffer fragmentation in race condition in between compressed file 
write and
normal file write?

Thanks,


Thanks.
But I'm out of my mind, above case wouldn't help, please ignore this.

Thanks,


Thanks.

Thanks,


The modify as follows:

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index
f4fd6c246c9a..0ed677efe820 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -3165,8 +3165,6 @@ static inline bool
__should_serialize_io(struct
inode *inode,
        if (IS_NOQUOTA(inode))
                return false;

-       if (f2fs_need_compress_data(inode))
-               return true;
        if (wbc->sync_mode != WB_SYNC_ALL)
                return true;
        if (get_dirty_pages(inode) >=
SM_I(F2FS_I_SB(inode))->min_seq_blocks)
@@ -3218,11 +3216,16 @@ static int __f2fs_write_data_pages(struct
address_space *mapping,
                mutex_lock(&sbi->writepages);
                locked = true;
        }
+       if (f2fs_need_compress_data(inode))
+               mutex_lock(&(F2FS_I(inode)->compress_lock));

        blk_start_plug(&plug);
        ret = f2fs_write_cache_pages(mapping, wbc, io_type);
        blk_finish_plug(&plug);

+       if (f2fs_need_compress_data(inode))
+               mutex_unlock(&(F2FS_I(inode)->compress_lock));
+
        if (locked)
                mutex_unlock(&sbi->writepages);

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index
039a229e11c9..3a6587f13d2f 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -763,6 +763,7 @@ struct f2fs_inode_info {
        struct list_head inmem_pages;   /* inmemory pages managed by
f2fs */
        struct task_struct *inmem_task; /* store inmemory task */
        struct mutex inmem_lock;        /* lock for inmemory pages */
+       struct mutex compress_lock;     /* lock for compress file */
        struct extent_tree *extent_tree;        /* cached extent_tree entry */

        /* avoid racing between foreground op and gc */ diff --git
a/fs/f2fs/super.c b/fs/f2fs/super.c index a133932333c5..8566e9c34540
100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1323,6 +1323,7 @@ static struct inode *f2fs_alloc_inode(struct
super_block *sb)
        INIT_LIST_HEAD(&fi->inmem_ilist);
        INIT_LIST_HEAD(&fi->inmem_pages);
        mutex_init(&fi->inmem_lock);
+       mutex_init(&fi->compress_lock);
        init_rwsem(&fi->i_gc_rwsem[READ]);
        init_rwsem(&fi->i_gc_rwsem[WRITE]);
        init_rwsem(&fi->i_xattr_sem);
--

-----Original Message-----
From: 常凤楠
Sent: Monday, November 8, 2021 11:55 AM
To: jaeg...@kernel.org; c...@kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Subject: Do we need serial io for compress file?

In my test, serial io for compress file will make multithread small
write performance drop a lot.

I'm try to fingure out why we need __should_serialize_io, IMO, we
use __should_serialize_io to avoid deadlock or try to improve
sequential performance, but I don't understand why we should do
this for compressed file. In my test, if we just remove this, write
same file in multithread will have problem, but parallel write
different files in multithread is ok. So I think maybe we should
use another lock to allow write different files in multithread.


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to