All of lore.kernel.org
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics
@ 2021-10-09 11:27 Fengnan Chang via Linux-f2fs-devel
  2021-10-09 11:27 ` [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang via Linux-f2fs-devel
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Fengnan Chang via Linux-f2fs-devel @ 2021-10-09 11:27 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>
---
 fs/f2fs/data.c    | 10 ++++++----
 fs/f2fs/debug.c   | 24 +++++++++++++++---------
 fs/f2fs/f2fs.h    | 32 +++++++++++++++++++++-----------
 fs/f2fs/gc.c      |  2 +-
 fs/f2fs/segment.c |  8 ++++----
 5 files changed, 47 insertions(+), 29 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..bf2e73e46304 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;	\
@@ -3866,8 +3876,8 @@ void f2fs_update_sit_info(struct f2fs_sb_info *sbi);
 #define stat_update_max_volatile_write(inode)		do { } while (0)
 #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_inc_block_count(sbi, curseg, direct_io)	do { } while (0)
+#define stat_inc_inplace_blocks(sbi, 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/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] 8+ messages in thread

* [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io
  2021-10-09 11:27 [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics Fengnan Chang via Linux-f2fs-devel
@ 2021-10-09 11:27 ` Fengnan Chang via Linux-f2fs-devel
  2021-10-13 15:18   ` Chao Yu
  2021-10-13 14:53 ` [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics Chao Yu
  2021-10-20  0:09 ` Jaegeuk Kim
  2 siblings, 1 reply; 8+ messages in thread
From: Fengnan Chang via Linux-f2fs-devel @ 2021-10-09 11:27 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    | 4 +++-
 fs/f2fs/f2fs.h    | 8 ++++----
 fs/f2fs/segment.c | 2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index c1490b9a1345..7798f7236376 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1553,7 +1553,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 (!f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_PRE_DIO &&
+				map->m_may_create && 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 bf2e73e46304..a7da836ca64f 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, direct_io)	do { } while (0)
-#define stat_inc_inplace_blocks(sbi, direct_io)		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] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics
  2021-10-09 11:27 [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics Fengnan Chang via Linux-f2fs-devel
  2021-10-09 11:27 ` [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang via Linux-f2fs-devel
@ 2021-10-13 14:53 ` Chao Yu
  2021-10-20  0:09 ` Jaegeuk Kim
  2 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-10-13 14:53 UTC (permalink / raw)
  To: Fengnan Chang, jaegeuk; +Cc: linux-f2fs-devel

On 2021/10/9 19:27, Fengnan Chang wrote:
> 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>

Thanks,


_______________________________________________
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] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io
  2021-10-09 11:27 ` [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang via Linux-f2fs-devel
@ 2021-10-13 15:18   ` Chao Yu
  2021-10-16  8:01     ` fengnan chang
  0 siblings, 1 reply; 8+ messages in thread
From: Chao Yu @ 2021-10-13 15:18 UTC (permalink / raw)
  To: Fengnan Chang, jaegeuk; +Cc: linux-f2fs-devel

On 2021/10/9 19:27, 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    | 4 +++-
>   fs/f2fs/f2fs.h    | 8 ++++----
>   fs/f2fs/segment.c | 2 +-
>   3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index c1490b9a1345..7798f7236376 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1553,7 +1553,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 (!f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_PRE_DIO &&
> +				map->m_may_create && create)
> +			stat_add_inplace_blocks(sbi, 1, true);

What about this case?

- f2fs_preallocate_blocks
  - f2fs_map_blocks
   - stat_add_inplace_blocks
   map.m_len > 0 && err == -ENOSPC
   err = 0;
- __generic_file_write_iter
  - generic_file_direct_write
   - f2fs_direct_IO
    - get_data_block_dio_write
     - __allocate_data_block
      - stat_inc_block_count

DIO blocks will be accounted into different type? IIUC.

>   	} else {
>   		if (create) {
>   			if (unlikely(f2fs_cp_error(sbi))) {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index bf2e73e46304..a7da836ca64f 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]));	\

If count always be one, we can just keep to use atomic_inc() here?

Thanks,

>   	} 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, direct_io)	do { } while (0)
> -#define stat_inc_inplace_blocks(sbi, direct_io)		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] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io
  2021-10-13 15:18   ` Chao Yu
@ 2021-10-16  8:01     ` fengnan chang
  2021-10-27  3:30       ` Chao Yu
       [not found]       ` <AOYAygA-EjrsqgsUALfVQapx.9.1635305442578.Hmail.changfengnan@vivo.com>
  0 siblings, 2 replies; 8+ messages in thread
From: fengnan chang @ 2021-10-16  8:01 UTC (permalink / raw)
  To: Chao Yu; +Cc: Jaegeuk Kim, Fengnan Chang, linux-f2fs-devel

Chao Yu <chao@kernel.org> 于2021年10月13日周三 下午11:19写道:
>
> On 2021/10/9 19:27, 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    | 4 +++-
> >   fs/f2fs/f2fs.h    | 8 ++++----
> >   fs/f2fs/segment.c | 2 +-
> >   3 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index c1490b9a1345..7798f7236376 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1553,7 +1553,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 (!f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_PRE_DIO &&
> > +                             map->m_may_create && create)
> > +                     stat_add_inplace_blocks(sbi, 1, true);
>
> What about this case?
>
> - f2fs_preallocate_blocks
>   - f2fs_map_blocks
>    - stat_add_inplace_blocks
>    map.m_len > 0 && err == -ENOSPC
>    err = 0;
> - __generic_file_write_iter
>   - generic_file_direct_write
>    - f2fs_direct_IO
>     - get_data_block_dio_write
>      - __allocate_data_block
>       - stat_inc_block_count
>
> DIO blocks will be accounted into different type? IIUC.
Yes, it will be accounted into different type,  IPU and LFS, but it
will not accounted into both in same time for one block.

root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 2
           buffer     direct   segments
IPU:           16         32        N/A
SSR:            0          0          0
LFS:           38         48          0
root@kvm-xfstests:/mnt# dd if=/dev/zero of=./1 bs=32K count=1 oflag=direct
root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 2
           buffer     direct   segments
IPU:           16         32        N/A
SSR:            0          0          0
LFS:           38         56          0

root@kvm-xfstests:/mnt# dd if=/dev/zero of=./1 bs=32K count=1
oflag=direct conv=notrunc
root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 2
           buffer     direct   segments
IPU:           16         40        N/A
SSR:            0          0          0
LFS:           38         56          0

root@kvm-xfstests:/mnt# dd if=/dev/zero of=./1 bs=32K count=2
oflag=direct conv=notrunc
root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 2
           buffer     direct   segments
IPU:           16         48        N/A
SSR:            0          0          0
LFS:           41         64          0


>
> >       } else {
> >               if (create) {
> >                       if (unlikely(f2fs_cp_error(sbi))) {
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index bf2e73e46304..a7da836ca64f 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]));  \
>
> If count always be one, we can just keep to use atomic_inc() here?
>
I suggest not, we may use this in later patch, not ready for now.

> Thanks,
>
> >       } 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, direct_io)        do { } while (0)
> > -#define stat_inc_inplace_blocks(sbi, direct_io)              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


_______________________________________________
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] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics
  2021-10-09 11:27 [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics Fengnan Chang via Linux-f2fs-devel
  2021-10-09 11:27 ` [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang via Linux-f2fs-devel
  2021-10-13 14:53 ` [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics Chao Yu
@ 2021-10-20  0:09 ` Jaegeuk Kim
  2 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2021-10-20  0:09 UTC (permalink / raw)
  To: Fengnan Chang; +Cc: linux-f2fs-devel

On 10/09, Fengnan Chang wrote:
> 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>
> ---
>  fs/f2fs/data.c    | 10 ++++++----
>  fs/f2fs/debug.c   | 24 +++++++++++++++---------
>  fs/f2fs/f2fs.h    | 32 +++++++++++++++++++++-----------
>  fs/f2fs/gc.c      |  2 +-
>  fs/f2fs/segment.c |  8 ++++----
>  5 files changed, 47 insertions(+), 29 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]);

Please clean up unnecessary spaces.

>  
>  		/* 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..bf2e73e46304 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 */

Can we clean up w/ macros instead of 2?

>  	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;	\
> @@ -3866,8 +3876,8 @@ void f2fs_update_sit_info(struct f2fs_sb_info *sbi);
>  #define stat_update_max_volatile_write(inode)		do { } while (0)
>  #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_inc_block_count(sbi, curseg, direct_io)	do { } while (0)
> +#define stat_inc_inplace_blocks(sbi, 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/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	[flat|nested] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io
  2021-10-16  8:01     ` fengnan chang
@ 2021-10-27  3:30       ` Chao Yu
       [not found]       ` <AOYAygA-EjrsqgsUALfVQapx.9.1635305442578.Hmail.changfengnan@vivo.com>
  1 sibling, 0 replies; 8+ messages in thread
From: Chao Yu @ 2021-10-27  3:30 UTC (permalink / raw)
  To: fengnan chang; +Cc: Jaegeuk Kim, Fengnan Chang, linux-f2fs-devel

On 2021/10/16 16:01, fengnan chang wrote:
> Chao Yu <chao@kernel.org> 于2021年10月13日周三 下午11:19写道:
>>
>> On 2021/10/9 19:27, 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    | 4 +++-
>>>    fs/f2fs/f2fs.h    | 8 ++++----
>>>    fs/f2fs/segment.c | 2 +-
>>>    3 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index c1490b9a1345..7798f7236376 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1553,7 +1553,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 (!f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_PRE_DIO &&
>>> +                             map->m_may_create && create)
>>> +                     stat_add_inplace_blocks(sbi, 1, true);
>>
>> What about this case?
>>
>> - f2fs_preallocate_blocks
>>    - f2fs_map_blocks
>>     - stat_add_inplace_blocks
>>     map.m_len > 0 && err == -ENOSPC
>>     err = 0;
>> - __generic_file_write_iter
>>    - generic_file_direct_write
>>     - f2fs_direct_IO
>>      - get_data_block_dio_write
>>       - __allocate_data_block
>>        - stat_inc_block_count
>>
>> DIO blocks will be accounted into different type? IIUC.
> Yes, it will be accounted into different type,  IPU and LFS, but it
> will not accounted into both in same time for one block.

Not sure this is right, since all writes should be accounted into LFS.

> 
> root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 2
>             buffer     direct   segments
> IPU:           16         32        N/A
> SSR:            0          0          0
> LFS:           38         48          0
> root@kvm-xfstests:/mnt# dd if=/dev/zero of=./1 bs=32K count=1 oflag=direct
> root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 2
>             buffer     direct   segments
> IPU:           16         32        N/A
> SSR:            0          0          0
> LFS:           38         56          0
> 
> root@kvm-xfstests:/mnt# dd if=/dev/zero of=./1 bs=32K count=1
> oflag=direct conv=notrunc
> root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 2
>             buffer     direct   segments
> IPU:           16         40        N/A
> SSR:            0          0          0
> LFS:           38         56          0
> 
> root@kvm-xfstests:/mnt# dd if=/dev/zero of=./1 bs=32K count=2
> oflag=direct conv=notrunc
> root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 2
>             buffer     direct   segments
> IPU:           16         48        N/A
> SSR:            0          0          0
> LFS:           41         64          0
> 
> 
>>
>>>        } else {
>>>                if (create) {
>>>                        if (unlikely(f2fs_cp_error(sbi))) {
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index bf2e73e46304..a7da836ca64f 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]));  \
>>
>> If count always be one, we can just keep to use atomic_inc() here?
>>
> I suggest not, we may use this in later patch, not ready for now.

I don't thinks this is the right way, why not including above change in your later patch?

Thanks,

> 
>> Thanks,
>>
>>>        } 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, direct_io)        do { } while (0)
>>> -#define stat_inc_inplace_blocks(sbi, direct_io)              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
> 


_______________________________________________
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] 8+ messages in thread

* Re: [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io
       [not found]       ` <AOYAygA-EjrsqgsUALfVQapx.9.1635305442578.Hmail.changfengnan@vivo.com>
@ 2021-10-27  9:24         ` 常凤楠
  0 siblings, 0 replies; 8+ messages in thread
From: 常凤楠 @ 2021-10-27  9:24 UTC (permalink / raw)
  To: Chao Yu, fengnan chang; +Cc: Jaegeuk Kim, linux-f2fs-devel



> -----Original Message-----
> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of
> Chao Yu
> Sent: Wednesday, October 27, 2021 11:31 AM
> To: fengnan chang <fengnanchang@gmail.com>
> Cc: 常凤楠 <changfengnan@vivo.com>; Jaegeuk Kim <jaegeuk@kernel.org>;
> linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in
> overwrite with direct io
> 
> On 2021/10/16 16:01, fengnan chang wrote:
> > Chao Yu <chao@kernel.org> 于2021年10月13日周三 下午11:19写道:
> >>
> >> On 2021/10/9 19:27, 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    | 4 +++-
> >>>    fs/f2fs/f2fs.h    | 8 ++++----
> >>>    fs/f2fs/segment.c | 2 +-
> >>>    3 files changed, 8 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index
> >>> c1490b9a1345..7798f7236376 100644
> >>> --- a/fs/f2fs/data.c
> >>> +++ b/fs/f2fs/data.c
> >>> @@ -1553,7 +1553,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 (!f2fs_lfs_mode(sbi) && flag ==
> F2FS_GET_BLOCK_PRE_DIO &&
> >>> +                             map->m_may_create && create)
> >>> +                     stat_add_inplace_blocks(sbi, 1, true);
> >>
> >> What about this case?
> >>
> >> - f2fs_preallocate_blocks
> >>    - f2fs_map_blocks
> >>     - stat_add_inplace_blocks
> >>     map.m_len > 0 && err == -ENOSPC
> >>     err = 0;
> >> - __generic_file_write_iter
> >>    - generic_file_direct_write
> >>     - f2fs_direct_IO
> >>      - get_data_block_dio_write
> >>       - __allocate_data_block
> >>        - stat_inc_block_count
> >>
> >> DIO blocks will be accounted into different type? IIUC.
> > Yes, it will be accounted into different type,  IPU and LFS, but it
> > will not accounted into both in same time for one block.
> 
> Not sure this is right, since all writes should be accounted into LFS.

Sorry, I didn't get your point, why all writes should be accounted into LFS ? even overwrite with direct io ?

> 
> >
> > root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 2
> >             buffer     direct   segments
> > IPU:           16         32        N/A
> > SSR:            0          0          0
> > LFS:           38         48          0
> > root@kvm-xfstests:/mnt# dd if=/dev/zero of=./1 bs=32K count=1
> > oflag=direct root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status
> |grep SSR -C 2
> >             buffer     direct   segments
> > IPU:           16         32        N/A
> > SSR:            0          0          0
> > LFS:           38         56          0
> >
> > root@kvm-xfstests:/mnt# dd if=/dev/zero of=./1 bs=32K count=1
> > oflag=direct conv=notrunc root@kvm-xfstests:/mnt# cat
> > /sys/kernel/debug/f2fs/status |grep SSR -C 2
> >             buffer     direct   segments
> > IPU:           16         40        N/A
> > SSR:            0          0          0
> > LFS:           38         56          0
> >
> > root@kvm-xfstests:/mnt# dd if=/dev/zero of=./1 bs=32K count=2
> > oflag=direct conv=notrunc root@kvm-xfstests:/mnt# cat
> > /sys/kernel/debug/f2fs/status |grep SSR -C 2
> >             buffer     direct   segments
> > IPU:           16         48        N/A
> > SSR:            0          0          0
> > LFS:           41         64          0
> >
> >
> >>
> >>>        } else {
> >>>                if (create) {
> >>>                        if (unlikely(f2fs_cp_error(sbi))) { diff
> >>> --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index
> >>> bf2e73e46304..a7da836ca64f 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]));
> >>> + \
> >>
> >> If count always be one, we can just keep to use atomic_inc() here?
> >>
> > I suggest not, we may use this in later patch, not ready for now.
> 
> I don't thinks this is the right way, why not including above change in your
> later patch?
> 
> Thanks,
> 
> >
> >> Thanks,
> >>
> >>>        } 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, direct_io)        do { }
> while (0)
> >>> -#define stat_inc_inplace_blocks(sbi, direct_io)              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
> >

_______________________________________________
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] 8+ messages in thread

end of thread, other threads:[~2021-10-27  9:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-09 11:27 [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics Fengnan Chang via Linux-f2fs-devel
2021-10-09 11:27 ` [f2fs-dev] [PATCH v5 2/2] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang via Linux-f2fs-devel
2021-10-13 15:18   ` Chao Yu
2021-10-16  8:01     ` fengnan chang
2021-10-27  3:30       ` Chao Yu
     [not found]       ` <AOYAygA-EjrsqgsUALfVQapx.9.1635305442578.Hmail.changfengnan@vivo.com>
2021-10-27  9:24         ` 常凤楠
2021-10-13 14:53 ` [f2fs-dev] [PATCH v5 1/2] f2fs: separate buffer and direct io in block allocation statistics Chao Yu
2021-10-20  0:09 ` Jaegeuk Kim

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.