All of lore.kernel.org
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH v2 1/2] f2fs: separate buffer and direct io in block allocation statistics
@ 2021-09-16 11:30 Fengnan Chang via Linux-f2fs-devel
  2021-09-16 11:30 ` [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang via Linux-f2fs-devel
  0 siblings, 1 reply; 9+ messages in thread
From: Fengnan Chang via Linux-f2fs-devel @ 2021-09-16 11:30 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: Fengnan Chang, linux-f2fs-devel

separate buffer and direct io in block allocation statistics.

New output will like this:
           buffer     direct   segments
IPU:            0          0        N/A
SSR:            0          0          0
LFS:            0          0          0

Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
Reviewed-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/data.c    | 10 ++++++----
 fs/f2fs/debug.c   | 24 +++++++++++++++---------
 fs/f2fs/f2fs.h    | 28 +++++++++++++++++++---------
 fs/f2fs/gc.c      |  2 +-
 fs/f2fs/segment.c |  8 ++++----
 5 files changed, 45 insertions(+), 27 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index f4fd6c246c9a..c1490b9a1345 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1342,7 +1342,7 @@ struct page *f2fs_get_new_data_page(struct inode *inode,
 	return page;
 }
 
-static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
+static int __allocate_data_block(struct dnode_of_data *dn, int seg_type, bool direct_io)
 {
 	struct f2fs_sb_info *sbi = F2FS_I_SB(dn->inode);
 	struct f2fs_summary sum;
@@ -1369,7 +1369,7 @@ static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
 	set_summary(&sum, dn->nid, dn->ofs_in_node, ni.version);
 	old_blkaddr = dn->data_blkaddr;
 	f2fs_allocate_data_block(sbi, NULL, old_blkaddr, &dn->data_blkaddr,
-				&sum, seg_type, NULL);
+				&sum, seg_type, NULL, direct_io);
 	if (GET_SEGNO(sbi, old_blkaddr) != NULL_SEGNO) {
 		invalidate_mapping_pages(META_MAPPING(sbi),
 					old_blkaddr, old_blkaddr);
@@ -1548,7 +1548,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		/* use out-place-update for driect IO under LFS mode */
 		if (f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
 							map->m_may_create) {
-			err = __allocate_data_block(&dn, map->m_seg_type);
+			err = __allocate_data_block(&dn, map->m_seg_type, true);
 			if (err)
 				goto sync_out;
 			blkaddr = dn.data_blkaddr;
@@ -1569,7 +1569,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 				WARN_ON(flag != F2FS_GET_BLOCK_PRE_DIO &&
 					flag != F2FS_GET_BLOCK_DIO);
 				err = __allocate_data_block(&dn,
-							map->m_seg_type);
+					map->m_seg_type,
+					flag == F2FS_GET_BLOCK_PRE_DIO ||
+					flag == F2FS_GET_BLOCK_DIO);
 				if (!err)
 					set_inode_flag(inode, FI_APPEND_WRITE);
 			}
diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index 8c50518475a9..e1aa843b067c 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -64,7 +64,7 @@ static void update_general_status(struct f2fs_sb_info *sbi)
 {
 	struct f2fs_stat_info *si = F2FS_STAT(sbi);
 	struct f2fs_super_block *raw_super = F2FS_RAW_SUPER(sbi);
-	int i;
+	int i, j;
 
 	/* these will be changed if online resize is done */
 	si->main_area_segs = le32_to_cpu(raw_super->segment_count_main);
@@ -210,10 +210,12 @@ static void update_general_status(struct f2fs_sb_info *sbi)
 
 	for (i = 0; i < 2; i++) {
 		si->segment_count[i] = sbi->segment_count[i];
-		si->block_count[i] = sbi->block_count[i];
+		for (j = 0; j < 2; j++)
+			si->block_count[i][j] = sbi->block_count[i][j];
 	}
 
-	si->inplace_count = atomic_read(&sbi->inplace_count);
+	for (i = 0; i < 2; i++)
+		si->inplace_count[i] = atomic_read(&sbi->inplace_count[i]);
 }
 
 /*
@@ -551,11 +553,14 @@ static int stat_show(struct seq_file *s, void *v)
 		for (j = 0; j < si->util_free; j++)
 			seq_putc(s, '-');
 		seq_puts(s, "]\n\n");
-		seq_printf(s, "IPU: %u blocks\n", si->inplace_count);
-		seq_printf(s, "SSR: %u blocks in %u segments\n",
-			   si->block_count[SSR], si->segment_count[SSR]);
-		seq_printf(s, "LFS: %u blocks in %u segments\n",
-			   si->block_count[LFS], si->segment_count[LFS]);
+
+		seq_printf(s, "       %10s %10s %10s\n", "buffer", "direct", "segments");
+		seq_printf(s,   "IPU:   %10d %10d        N/A\n", si->inplace_count[1],
+				si->inplace_count[0]);
+		seq_printf(s,   "SSR:   %10d %10d %10d\n", si->block_count[1][SSR],
+				si->block_count[0][SSR], si->segment_count[SSR]);
+		seq_printf(s,   "LFS:   %10d %10d %10d\n", si->block_count[1][LFS],
+				si->block_count[0][LFS], si->segment_count[LFS]);
 
 		/* segment usage info */
 		f2fs_update_sit_info(si->sbi);
@@ -611,7 +616,8 @@ int f2fs_build_stats(struct f2fs_sb_info *sbi)
 	atomic_set(&sbi->inline_dir, 0);
 	atomic_set(&sbi->compr_inode, 0);
 	atomic64_set(&sbi->compr_blocks, 0);
-	atomic_set(&sbi->inplace_count, 0);
+	for (i = 0; i < 2; i++)
+		atomic_set(&sbi->inplace_count[i], 0);
 	for (i = META_CP; i < META_MAX; i++)
 		atomic_set(&sbi->meta_count[i], 0);
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index b339ae89c1ad..3d4ee444db27 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1692,8 +1692,8 @@ struct f2fs_sb_info {
 	struct f2fs_stat_info *stat_info;	/* FS status information */
 	atomic_t meta_count[META_MAX];		/* # of meta blocks */
 	unsigned int segment_count[2];		/* # of allocated segments */
-	unsigned int block_count[2];		/* # of allocated blocks */
-	atomic_t inplace_count;		/* # of inplace update */
+	unsigned int block_count[2][2];		/* # of allocated blocks */
+	atomic_t inplace_count[2];		/* # of inplace update */
 	atomic64_t total_hit_ext;		/* # of lookup extent cache */
 	atomic64_t read_hit_rbtree;		/* # of hit rbtree extent node */
 	atomic64_t read_hit_largest;		/* # of hit largest extent node */
@@ -3491,7 +3491,7 @@ void f2fs_replace_block(struct f2fs_sb_info *sbi, struct dnode_of_data *dn,
 void f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 			block_t old_blkaddr, block_t *new_blkaddr,
 			struct f2fs_summary *sum, int type,
-			struct f2fs_io_info *fio);
+			struct f2fs_io_info *fio, bool direct_io);
 void f2fs_wait_on_page_writeback(struct page *page,
 			enum page_type type, bool ordered, bool locked);
 void f2fs_wait_on_block_writeback(struct inode *inode, block_t blkaddr);
@@ -3699,8 +3699,8 @@ struct f2fs_stat_info {
 
 	unsigned int meta_count[META_MAX];
 	unsigned int segment_count[2];
-	unsigned int block_count[2];
-	unsigned int inplace_count;
+	unsigned int block_count[2][2];
+	unsigned int inplace_count[2];
 	unsigned long long base_mem, cache_mem, page_mem;
 };
 
@@ -3778,10 +3778,20 @@ static inline struct f2fs_stat_info *F2FS_STAT(struct f2fs_sb_info *sbi)
 	} while (0)
 #define stat_inc_seg_type(sbi, curseg)					\
 		((sbi)->segment_count[(curseg)->alloc_type]++)
-#define stat_inc_block_count(sbi, curseg)				\
-		((sbi)->block_count[(curseg)->alloc_type]++)
-#define stat_inc_inplace_blocks(sbi)					\
-		(atomic_inc(&(sbi)->inplace_count))
+#define stat_inc_block_count(sbi, curseg, direct_io)			\
+	do {								\
+		if (direct_io)						\
+			((sbi)->block_count[0][(curseg)->alloc_type]++);	\
+		else								\
+			((sbi)->block_count[1][(curseg)->alloc_type]++);	\
+	} while (0)
+#define stat_inc_inplace_blocks(sbi, direct_io)					\
+	do {								\
+		if (direct_io)						\
+			(atomic_inc(&(sbi)->inplace_count[0]));		\
+		else								\
+			(atomic_inc(&(sbi)->inplace_count[1]));		\
+	} while (0)
 #define stat_update_max_atomic_write(inode)				\
 	do {								\
 		int cur = F2FS_I_SB(inode)->atomic_files;	\
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 77391e3b7d68..7c47082f73cc 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1247,7 +1247,7 @@ static int move_data_block(struct inode *inode, block_t bidx,
 
 	/* allocate block address */
 	f2fs_allocate_data_block(fio.sbi, NULL, fio.old_blkaddr, &newaddr,
-				&sum, type, NULL);
+				&sum, type, NULL, false);
 
 	fio.encrypted_page = f2fs_pagecache_get_page(META_MAPPING(fio.sbi),
 				newaddr, FGP_LOCK | FGP_CREAT, GFP_NOFS);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a135d2247415..ded744e880d0 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3391,7 +3391,7 @@ static int __get_segment_type(struct f2fs_io_info *fio)
 void f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 		block_t old_blkaddr, block_t *new_blkaddr,
 		struct f2fs_summary *sum, int type,
-		struct f2fs_io_info *fio)
+		struct f2fs_io_info *fio, bool direct_io)
 {
 	struct sit_info *sit_i = SIT_I(sbi);
 	struct curseg_info *curseg = CURSEG_I(sbi, type);
@@ -3425,7 +3425,7 @@ void f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 
 	__refresh_next_blkoff(sbi, curseg);
 
-	stat_inc_block_count(sbi, curseg);
+	stat_inc_block_count(sbi, curseg, direct_io);
 
 	if (from_gc) {
 		old_mtime = get_segment_mtime(sbi, old_blkaddr);
@@ -3515,7 +3515,7 @@ static void do_write_page(struct f2fs_summary *sum, struct f2fs_io_info *fio)
 		down_read(&fio->sbi->io_order_lock);
 reallocate:
 	f2fs_allocate_data_block(fio->sbi, fio->page, fio->old_blkaddr,
-			&fio->new_blkaddr, sum, type, fio);
+			&fio->new_blkaddr, sum, type, fio, false);
 	if (GET_SEGNO(fio->sbi, fio->old_blkaddr) != NULL_SEGNO) {
 		invalidate_mapping_pages(META_MAPPING(fio->sbi),
 					fio->old_blkaddr, fio->old_blkaddr);
@@ -3611,7 +3611,7 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
 		goto drop_bio;
 	}
 
-	stat_inc_inplace_blocks(fio->sbi);
+	stat_inc_inplace_blocks(fio->sbi, false);
 
 	if (fio->bio && !(SM_I(sbi)->ipu_policy & (1 << F2FS_IPU_NOCACHE)))
 		err = f2fs_merge_page_bio(fio);
-- 
2.32.0



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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io
  2021-09-16 11:30 [f2fs-dev] [PATCH v2 1/2] f2fs: separate buffer and direct io in block allocation statistics Fengnan Chang via Linux-f2fs-devel
@ 2021-09-16 11:30 ` Fengnan Chang via Linux-f2fs-devel
  2021-09-16 12:09   ` Chao Yu
       [not found]   ` <AFQA*QDoEjqpHYJlWwMYT4qj.9.1631794201010.Hmail.changfengnan@vivo.com>
  0 siblings, 2 replies; 9+ messages in thread
From: Fengnan Chang via Linux-f2fs-devel @ 2021-09-16 11:30 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: Fengnan Chang, linux-f2fs-devel

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)
+			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);
-- 
2.32.0



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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io
  2021-09-16 11:30 ` [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang via Linux-f2fs-devel
@ 2021-09-16 12:09   ` Chao Yu
       [not found]   ` <AFQA*QDoEjqpHYJlWwMYT4qj.9.1631794201010.Hmail.changfengnan@vivo.com>
  1 sibling, 0 replies; 9+ messages in thread
From: Chao Yu @ 2021-09-16 12:09 UTC (permalink / raw)
  To: Fengnan Chang, jaegeuk; +Cc: linux-f2fs-devel

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)

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io
       [not found]   ` <AFQA*QDoEjqpHYJlWwMYT4qj.9.1631794201010.Hmail.changfengnan@vivo.com>
@ 2021-09-16 12:45     ` 常凤楠 via Linux-f2fs-devel
  2021-09-17 10:19       ` 常凤楠 via Linux-f2fs-devel
  0 siblings, 1 reply; 9+ messages in thread
From: 常凤楠 via Linux-f2fs-devel @ 2021-09-16 12:45 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel



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

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io
  2021-09-16 12:45     ` 常凤楠 via Linux-f2fs-devel
@ 2021-09-17 10:19       ` 常凤楠 via Linux-f2fs-devel
  2021-09-17 12:59         ` Chao Yu
       [not found]         ` <ANgA6QClEsWrpLTMx5-VNqof.9.1631883577159.Hmail.changfengnan@vivo.com>
  0 siblings, 2 replies; 9+ messages in thread
From: 常凤楠 via Linux-f2fs-devel @ 2021-09-17 10:19 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io
  2021-09-17 10:19       ` 常凤楠 via Linux-f2fs-devel
@ 2021-09-17 12:59         ` Chao Yu
       [not found]         ` <ANgA6QClEsWrpLTMx5-VNqof.9.1631883577159.Hmail.changfengnan@vivo.com>
  1 sibling, 0 replies; 9+ messages in thread
From: Chao Yu @ 2021-09-17 12:59 UTC (permalink / raw)
  To: 常凤楠, jaegeuk; +Cc: linux-f2fs-devel

On 2021/9/17 18:19, 常凤楠 wrote:
> 
> 
>> -----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?

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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io
       [not found]         ` <ANgA6QClEsWrpLTMx5-VNqof.9.1631883577159.Hmail.changfengnan@vivo.com>
@ 2021-09-18  6:46           ` 常凤楠 via Linux-f2fs-devel
  2021-09-18 22:34             ` Chao Yu
       [not found]             ` <AJQA4AArEnKurlijneb9sapU.9.1632004451993.Hmail.changfengnan@vivo.com>
  0 siblings, 2 replies; 9+ messages in thread
From: 常凤楠 via Linux-f2fs-devel @ 2021-09-18  6:46 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel



> -----Original Message-----
> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of
> Chao Yu
> Sent: Friday, September 17, 2021 9:00 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/17 18:19, 常凤楠 wrote:
> >
> >
> >> -----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?
> 
> 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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io
  2021-09-18  6:46           ` 常凤楠 via Linux-f2fs-devel
@ 2021-09-18 22:34             ` Chao Yu
       [not found]             ` <AJQA4AArEnKurlijneb9sapU.9.1632004451993.Hmail.changfengnan@vivo.com>
  1 sibling, 0 replies; 9+ messages in thread
From: Chao Yu @ 2021-09-18 22:34 UTC (permalink / raw)
  To: 常凤楠, jaegeuk; +Cc: linux-f2fs-devel

On 2021/9/18 14:46, 常凤楠 wrote:
> 
> 
>> -----Original Message-----
>> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of
>> Chao Yu
>> Sent: Friday, September 17, 2021 9:00 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/17 18:19, 常凤楠 wrote:
>>>
>>>
>>>> -----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?
>>
>> 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
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io
       [not found]             ` <AJQA4AArEnKurlijneb9sapU.9.1632004451993.Hmail.changfengnan@vivo.com>
@ 2021-09-22  3:01               ` 常凤楠 via Linux-f2fs-devel
  0 siblings, 0 replies; 9+ messages in thread
From: 常凤楠 via Linux-f2fs-devel @ 2021-09-22  3:01 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel


> -----Original Message-----
> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of
> Chao Yu
> Sent: Sunday, September 19, 2021 6:34 AM
> 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/18 14:46, 常凤楠 wrote:
> >
> >
> >> -----Original Message-----
> >> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of
> Chao
> >> Yu
> >> Sent: Friday, September 17, 2021 9:00 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/17 18:19, 常凤楠 wrote:
> >>>
> >>>
> >>>> -----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?
> >>
> >> 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.
> 

Yes, you are right, we should check blkaddr is valid when physical block can found in extent cache.

Thanks.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-09-22  3:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 11:30 [f2fs-dev] [PATCH v2 1/2] f2fs: separate buffer and direct io in block allocation statistics Fengnan Chang via Linux-f2fs-devel
2021-09-16 11:30 ` [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang via Linux-f2fs-devel
2021-09-16 12:09   ` Chao Yu
     [not found]   ` <AFQA*QDoEjqpHYJlWwMYT4qj.9.1631794201010.Hmail.changfengnan@vivo.com>
2021-09-16 12:45     ` 常凤楠 via Linux-f2fs-devel
2021-09-17 10:19       ` 常凤楠 via Linux-f2fs-devel
2021-09-17 12:59         ` Chao Yu
     [not found]         ` <ANgA6QClEsWrpLTMx5-VNqof.9.1631883577159.Hmail.changfengnan@vivo.com>
2021-09-18  6:46           ` 常凤楠 via Linux-f2fs-devel
2021-09-18 22:34             ` Chao Yu
     [not found]             ` <AJQA4AArEnKurlijneb9sapU.9.1632004451993.Hmail.changfengnan@vivo.com>
2021-09-22  3:01               ` 常凤楠 via Linux-f2fs-devel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.