> -----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
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
