On 2021/9/18 14:46, 常凤楠 wrote:


-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of
Chao Yu
Sent: Friday, September 17, 2021 9:00 PM
To: 常凤楠 <[email protected]>; [email protected]
Cc: [email protected]
Subject: Re: [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with
direct io

On 2021/9/17 18:19, 常凤楠 wrote:


-----Original Message-----
From: 常凤楠
Sent: Thursday, September 16, 2021 8:46 PM
To: Chao Yu <[email protected]>; [email protected]
Cc: [email protected]
Subject: RE: [PATCH v2 2/2] f2fs: fix missing inplace count in
overwrite with direct io



-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of
Chao
Yu
Sent: Thursday, September 16, 2021 8:10 PM
To: 常凤楠 <[email protected]>; [email protected]
Cc: [email protected]
Subject: Re: [PATCH v2 2/2] f2fs: fix missing inplace count in
overwrite with direct io

On 2021/9/16 19:30, Fengnan Chang wrote:
For now, overwrite file with direct io use inplace policy, but not
counted, fix it. And use stat_add_inplace_blocks(sbi, 1, ) instead
of stat_inc_inplace_blocks(sb, ).

Signed-off-by: Fengnan Chang <[email protected]>
---
    fs/f2fs/data.c    | 7 ++++++-
    fs/f2fs/f2fs.h    | 8 ++++----
    fs/f2fs/segment.c | 2 +-
    3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index
c1490b9a1345..0c5728d63c33 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1491,6 +1491,9 @@ int f2fs_map_blocks(struct inode *inode,
struct
f2fs_map_blocks *map,
                if (flag == F2FS_GET_BLOCK_DIO)
                        f2fs_wait_on_block_writeback_range(inode,
                                                map->m_pblk, map->m_len);
+               if (!f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
+                               map->m_may_create)
+                       stat_add_inplace_blocks(sbi, map->m_len, true);
                goto out;
        }

@@ -1553,7 +1556,9 @@ int f2fs_map_blocks(struct inode *inode,
struct
f2fs_map_blocks *map,
                                goto sync_out;
                        blkaddr = dn.data_blkaddr;
                        set_inode_flag(inode, FI_APPEND_WRITE);
-               }
+               } else if (!create && !f2fs_lfs_mode(sbi) && flag ==
F2FS_GET_BLOCK_DIO &&
+                               map->m_may_create)

Why not

} else if {!f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
                                        map->m_may_create)


You are right, no need to check create .

There is a problem here, if remove create check, when create file and
write with direct io, It will count in LFS and IPU both, because
preallocate block addr first. So, We still need check create.
Am I right?

Could you please check below case w/ your original patch:

xfs_io -f file -c "pwrite 0 8k" -c "fsync"
xfs_io file -c "fpunch 0 4k"
xfs_io  -c "open -d file" -c "pwrite -b 4k 0 8k"

It accounts on both IPU and LFS stats.

My origin patch is need check create:
@@ -1553,7 +1556,9 @@ int f2fs_map_blocks(struct inode *inode, struct 
f2fs_map_blocks *map,
                                goto sync_out;
                        blkaddr = dn.data_blkaddr;
                        set_inode_flag(inode, FI_APPEND_WRITE);
-               }
+               } else if (!create && !f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO 
&&
+                               map->m_may_create)
+                       stat_add_inplace_blocks(sbi, 1, true);

And below case looks correct, So I think check create is necessary.

root@kvm-xfstests:/mnt/test# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3

            buffer     direct   segments
IPU:            0          0        N/A
SSR:            0          0          0
LFS:          542          0          1

BDF: 99, avg. vblocks: 488
root@kvm-xfstests:/mnt/test# xfs_io -f file -c "pwrite 0 8k" -c "fsync"
wrote 8192/8192 bytes at offset 0
8 KiB, 2 ops; 0.0078 sec (1014.070 KiB/sec and 253.5176 ops/sec)
root@kvm-xfstests:/mnt/test# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3

            buffer     direct   segments
IPU:            0          0        N/A
SSR:            0          0          0
LFS:          545          0          1

BDF: 99, avg. vblocks: 488
root@kvm-xfstests:/mnt/test# xfs_io file -c "fpunch 0 4k"
root@kvm-xfstests:/mnt/test# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3

            buffer     direct   segments
IPU:            0          0        N/A
SSR:            0          0          0
LFS:          545          0          1

BDF: 99, avg. vblocks: 488
root@kvm-xfstests:/mnt/test# xfs_io  -c "open -d file" -c "pwrite -b 4k 0 8k"
wrote 8192/8192 bytes at offset 0
8 KiB, 2 ops; 0.0322 sec (248.185 KiB/sec and 62.0463 ops/sec)
root@kvm-xfstests:/mnt/test# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3

            buffer     direct   segments
IPU:            0          2        N/A
SSR:            0          0          0
LFS:          545          1          1

Shouldn't this be IPU: 1 and LFS: 1? due to [0, 4k] was a hole, and [4k, 8k] 
mapped
to a valid blkaddr.

Thanks,


BDF: 99, avg. vblocks: 488



Thanks,


Thanks.

Thanks.
Thanks,

+                       stat_add_inplace_blocks(sbi, 1, true);
        } else {
                if (create) {
                        if (unlikely(f2fs_cp_error(sbi))) { diff --git
a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 3d4ee444db27..2d81e9f0a0ee
100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3785,12 +3785,12 @@ static inline struct f2fs_stat_info
*F2FS_STAT(struct f2fs_sb_info *sbi)
                else                                                            
\
                        ((sbi)->block_count[1][(curseg)->alloc_type]++);  \
        } while (0)
-#define stat_inc_inplace_blocks(sbi, direct_io)                                
        \
+#define stat_add_inplace_blocks(sbi, count, direct_io)                 \
        do {                                                            \
                if (direct_io)                                          \
-                       (atomic_inc(&(sbi)->inplace_count[0]));          \
+                       (atomic_add(count, &(sbi)->inplace_count[0]));  \
                else                                                            
\
-                       (atomic_inc(&(sbi)->inplace_count[1]));          \
+                       (atomic_add(count, &(sbi)->inplace_count[1]));   \
        } while (0)
    #define stat_update_max_atomic_write(inode)                         \
        do {                                                            \
@@ -3877,7 +3877,7 @@ void f2fs_update_sit_info(struct f2fs_sb_info
*sbi);
    #define stat_inc_meta_count(sbi, blkaddr)           do { } while (0)
    #define stat_inc_seg_type(sbi, curseg)                      do { } while (0)
    #define stat_inc_block_count(sbi, curseg)           do { } while (0)
-#define stat_inc_inplace_blocks(sbi)                   do { } while (0)
+#define stat_add_inplace_blocks(sbi, count, direct_io)         do { }
while
(0)
    #define stat_inc_seg_count(sbi, type, gc_type)              do { } while
(0)
    #define stat_inc_tot_blk_count(si, blks)            do { } while (0)
    #define stat_inc_data_blk_count(sbi, blks, gc_type) do { } while
(0)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index
ded744e880d0..c542c4b687ca 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3611,7 +3611,7 @@ int f2fs_inplace_write_data(struct
f2fs_io_info
*fio)
                goto drop_bio;
        }

-       stat_inc_inplace_blocks(fio->sbi, false);
+       stat_add_inplace_blocks(sbi, 1, false);

        if (fio->bio && !(SM_I(sbi)->ipu_policy & (1 <<
F2FS_IPU_NOCACHE)))
                err = f2fs_merge_page_bio(fio);



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to