Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io

["常凤楠 via Linux-f2fs-devel" <linux-f2fs-devel@lists.sourceforge.net>]

> -----Original Message-----
> From: 常凤楠
> Sent: Thursday, September 16, 2021 8:46 PM
> To: Chao Yu <chao@kernel.org>; jaegeuk@kernel.org
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: RE: [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with
> direct io
> 
> 
> 
> > -----Original Message-----
> > From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of
> Chao
> > Yu
> > Sent: Thursday, September 16, 2021 8:10 PM
> > To: 常凤楠 <changfengnan@vivo.com>; jaegeuk@kernel.org
> > Cc: linux-f2fs-devel@lists.sourceforge.net
> > 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 <changfengnan@vivo.com>
> > > ---
> > >   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?

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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel