All of lore.kernel.org
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH] f2fs: fix missing inplace count in overwrite with direct io
@ 2021-07-29 12:25 Fengnan Chang
  2021-08-06  1:00 ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Fengnan Chang @ 2021-07-29 12:25 UTC (permalink / raw)
  To: jaegeuk, chao, linux-f2fs-devel; +Cc: Fengnan Chang

For now, overwrite file with direct io use inplace policy, but not
counted, fix it.

Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
---
 fs/f2fs/data.c | 6 ++++++
 fs/f2fs/f2fs.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index d2cf48c5a2e4..60510acf91ec 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1477,6 +1477,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);
 		goto out;
 	}
 
@@ -1526,6 +1529,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 			blkaddr = dn.data_blkaddr;
 			set_inode_flag(inode, FI_APPEND_WRITE);
 		}
+		if (!create && !f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
+				map->m_may_create)
+			stat_inc_inplace_blocks(sbi);
 	} else {
 		if (create) {
 			if (unlikely(f2fs_cp_error(sbi))) {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 867f2c5d9559..3a9df28e6fd7 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3804,6 +3804,8 @@ static inline struct f2fs_stat_info *F2FS_STAT(struct f2fs_sb_info *sbi)
 		((sbi)->block_count[(curseg)->alloc_type]++)
 #define stat_inc_inplace_blocks(sbi)					\
 		(atomic_inc(&(sbi)->inplace_count))
+#define stat_add_inplace_blocks(sbi, count)				\
+		(atomic_add(count, &(sbi)->inplace_count))
 #define stat_update_max_atomic_write(inode)				\
 	do {								\
 		int cur = F2FS_I_SB(inode)->atomic_files;	\
-- 
2.29.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] f2fs: fix missing inplace count in overwrite with direct io
  2021-07-29 12:25 [f2fs-dev] [PATCH] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang
@ 2021-08-06  1:00 ` Chao Yu
  2021-08-12 21:15   ` Jaegeuk Kim
  0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2021-08-06  1:00 UTC (permalink / raw)
  To: Fengnan Chang, jaegeuk, linux-f2fs-devel

On 2021/7/29 20:25, Fengnan Chang wrote:
> For now, overwrite file with direct io use inplace policy, but not
> counted, fix it.

IMO, LFS/SSR/IPU stats in debugfs was for buffered write, maybe we
need to add separated one for DIO.

Jaegeuk, thoughts?

Thanks,

> 
> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
> ---
>   fs/f2fs/data.c | 6 ++++++
>   fs/f2fs/f2fs.h | 2 ++
>   2 files changed, 8 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index d2cf48c5a2e4..60510acf91ec 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1477,6 +1477,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);
>   		goto out;
>   	}
>   
> @@ -1526,6 +1529,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>   			blkaddr = dn.data_blkaddr;
>   			set_inode_flag(inode, FI_APPEND_WRITE);
>   		}
> +		if (!create && !f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
> +				map->m_may_create)
> +			stat_inc_inplace_blocks(sbi);
>   	} else {
>   		if (create) {
>   			if (unlikely(f2fs_cp_error(sbi))) {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 867f2c5d9559..3a9df28e6fd7 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3804,6 +3804,8 @@ static inline struct f2fs_stat_info *F2FS_STAT(struct f2fs_sb_info *sbi)
>   		((sbi)->block_count[(curseg)->alloc_type]++)
>   #define stat_inc_inplace_blocks(sbi)					\
>   		(atomic_inc(&(sbi)->inplace_count))
> +#define stat_add_inplace_blocks(sbi, count)				\
> +		(atomic_add(count, &(sbi)->inplace_count))
>   #define stat_update_max_atomic_write(inode)				\
>   	do {								\
>   		int cur = F2FS_I_SB(inode)->atomic_files;	\
> 


_______________________________________________
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] f2fs: fix missing inplace count in overwrite with direct io
  2021-08-06  1:00 ` Chao Yu
@ 2021-08-12 21:15   ` Jaegeuk Kim
  2021-08-13  1:36     ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2021-08-12 21:15 UTC (permalink / raw)
  To: Chao Yu; +Cc: Fengnan Chang, linux-f2fs-devel

On 08/06, Chao Yu wrote:
> On 2021/7/29 20:25, Fengnan Chang wrote:
> > For now, overwrite file with direct io use inplace policy, but not
> > counted, fix it.
> 
> IMO, LFS/SSR/IPU stats in debugfs was for buffered write, maybe we
> need to add separated one for DIO.

Do we really need to monitor DIO stats?

> 
> Jaegeuk, thoughts?
> 
> Thanks,
> 
> > 
> > Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
> > ---
> >   fs/f2fs/data.c | 6 ++++++
> >   fs/f2fs/f2fs.h | 2 ++
> >   2 files changed, 8 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index d2cf48c5a2e4..60510acf91ec 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1477,6 +1477,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);
> >   		goto out;
> >   	}
> > @@ -1526,6 +1529,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
> >   			blkaddr = dn.data_blkaddr;
> >   			set_inode_flag(inode, FI_APPEND_WRITE);
> >   		}
> > +		if (!create && !f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
> > +				map->m_may_create)
> > +			stat_inc_inplace_blocks(sbi);
> >   	} else {
> >   		if (create) {
> >   			if (unlikely(f2fs_cp_error(sbi))) {
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 867f2c5d9559..3a9df28e6fd7 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -3804,6 +3804,8 @@ static inline struct f2fs_stat_info *F2FS_STAT(struct f2fs_sb_info *sbi)
> >   		((sbi)->block_count[(curseg)->alloc_type]++)
> >   #define stat_inc_inplace_blocks(sbi)					\
> >   		(atomic_inc(&(sbi)->inplace_count))
> > +#define stat_add_inplace_blocks(sbi, count)				\
> > +		(atomic_add(count, &(sbi)->inplace_count))
> >   #define stat_update_max_atomic_write(inode)				\
> >   	do {								\
> >   		int cur = F2FS_I_SB(inode)->atomic_files;	\
> > 


_______________________________________________
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] f2fs: fix missing inplace count in overwrite with direct io
  2021-08-12 21:15   ` Jaegeuk Kim
@ 2021-08-13  1:36     ` Chao Yu
  2021-08-18  3:49       ` Fengnan Chang
  0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2021-08-13  1:36 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Fengnan Chang, linux-f2fs-devel

On 2021/8/13 5:15, Jaegeuk Kim wrote:
> On 08/06, Chao Yu wrote:
>> On 2021/7/29 20:25, Fengnan Chang wrote:
>>> For now, overwrite file with direct io use inplace policy, but not
>>> counted, fix it.
>>
>> IMO, LFS/SSR/IPU stats in debugfs was for buffered write, maybe we
>> need to add separated one for DIO.
> 
> Do we really need to monitor DIO stats?

Similar reason as we did for buffered IO?

Thanks,

> 
>>
>> Jaegeuk, thoughts?
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
>>> ---
>>>    fs/f2fs/data.c | 6 ++++++
>>>    fs/f2fs/f2fs.h | 2 ++
>>>    2 files changed, 8 insertions(+)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index d2cf48c5a2e4..60510acf91ec 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1477,6 +1477,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);
>>>    		goto out;
>>>    	}
>>> @@ -1526,6 +1529,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
>>>    			blkaddr = dn.data_blkaddr;
>>>    			set_inode_flag(inode, FI_APPEND_WRITE);
>>>    		}
>>> +		if (!create && !f2fs_lfs_mode(sbi) && flag == F2FS_GET_BLOCK_DIO &&
>>> +				map->m_may_create)
>>> +			stat_inc_inplace_blocks(sbi);
>>>    	} else {
>>>    		if (create) {
>>>    			if (unlikely(f2fs_cp_error(sbi))) {
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 867f2c5d9559..3a9df28e6fd7 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -3804,6 +3804,8 @@ static inline struct f2fs_stat_info *F2FS_STAT(struct f2fs_sb_info *sbi)
>>>    		((sbi)->block_count[(curseg)->alloc_type]++)
>>>    #define stat_inc_inplace_blocks(sbi)					\
>>>    		(atomic_inc(&(sbi)->inplace_count))
>>> +#define stat_add_inplace_blocks(sbi, count)				\
>>> +		(atomic_add(count, &(sbi)->inplace_count))
>>>    #define stat_update_max_atomic_write(inode)				\
>>>    	do {								\
>>>    		int cur = F2FS_I_SB(inode)->atomic_files;	\
>>>


_______________________________________________
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] f2fs: fix missing inplace count in overwrite with direct io
  2021-08-13  1:36     ` Chao Yu
@ 2021-08-18  3:49       ` Fengnan Chang
  2021-08-20  9:41         ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Fengnan Chang @ 2021-08-18  3:49 UTC (permalink / raw)
  To: Chao Yu, Jaegeuk Kim; +Cc: linux-f2fs-devel



On 2021/8/13 9:36, Chao Yu wrote:
> On 2021/8/13 5:15, Jaegeuk Kim wrote:
>> On 08/06, Chao Yu wrote:
>>> On 2021/7/29 20:25, Fengnan Chang wrote:
>>>> For now, overwrite file with direct io use inplace policy, but not
>>>> counted, fix it.
>>>
>>> IMO, LFS/SSR/IPU stats in debugfs was for buffered write, maybe we
>>> need to add separated one for DIO.
>>
>> Do we really need to monitor DIO stats?
> 
> Similar reason as we did for buffered IO?

For now, LFS & SSR are count in DIO, but not count IPU,  I think we 
should keep consistency.

> 
> Thanks,
> 
>>
>>>
>>> Jaegeuk, thoughts?
>>>
>>> Thanks,
>>>
>>>>
>>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
>>>> ---
>>>>    fs/f2fs/data.c | 6 ++++++
>>>>    fs/f2fs/f2fs.h | 2 ++
>>>>    2 files changed, 8 insertions(+)
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index d2cf48c5a2e4..60510acf91ec 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1477,6 +1477,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);
>>>>            goto out;
>>>>        }
>>>> @@ -1526,6 +1529,9 @@ int f2fs_map_blocks(struct inode *inode, 
>>>> struct f2fs_map_blocks *map,
>>>>                blkaddr = dn.data_blkaddr;
>>>>                set_inode_flag(inode, FI_APPEND_WRITE);
>>>>            }
>>>> +        if (!create && !f2fs_lfs_mode(sbi) && flag == 
>>>> F2FS_GET_BLOCK_DIO &&
>>>> +                map->m_may_create)
>>>> +            stat_inc_inplace_blocks(sbi);
>>>>        } else {
>>>>            if (create) {
>>>>                if (unlikely(f2fs_cp_error(sbi))) {
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index 867f2c5d9559..3a9df28e6fd7 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -3804,6 +3804,8 @@ static inline struct f2fs_stat_info 
>>>> *F2FS_STAT(struct f2fs_sb_info *sbi)
>>>>            ((sbi)->block_count[(curseg)->alloc_type]++)
>>>>    #define stat_inc_inplace_blocks(sbi)                    \
>>>>            (atomic_inc(&(sbi)->inplace_count))
>>>> +#define stat_add_inplace_blocks(sbi, count)                \
>>>> +        (atomic_add(count, &(sbi)->inplace_count))
>>>>    #define stat_update_max_atomic_write(inode)                \
>>>>        do {                                \
>>>>            int cur = F2FS_I_SB(inode)->atomic_files;    \
>>>>
> 


_______________________________________________
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] f2fs: fix missing inplace count in overwrite with direct io
  2021-08-18  3:49       ` Fengnan Chang
@ 2021-08-20  9:41         ` Chao Yu
  2021-08-23 12:07           ` Fengnan Chang
  0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2021-08-20  9:41 UTC (permalink / raw)
  To: Fengnan Chang, Jaegeuk Kim; +Cc: linux-f2fs-devel

On 2021/8/18 11:49, Fengnan Chang wrote:
> 
> 
> On 2021/8/13 9:36, Chao Yu wrote:
>> On 2021/8/13 5:15, Jaegeuk Kim wrote:
>>> On 08/06, Chao Yu wrote:
>>>> On 2021/7/29 20:25, Fengnan Chang wrote:
>>>>> For now, overwrite file with direct io use inplace policy, but not
>>>>> counted, fix it.
>>>>
>>>> IMO, LFS/SSR/IPU stats in debugfs was for buffered write, maybe we
>>>> need to add separated one for DIO.
>>>
>>> Do we really need to monitor DIO stats?
>>
>> Similar reason as we did for buffered IO?
> 
> For now, LFS & SSR are count in DIO, but not count IPU,  I think we

I guess it will account IOs which are fallbacking from DIO to buffered IO,
so all DIOs are not accounted...

Thanks,

> should keep consistency.
> 
>>
>> Thanks,
>>
>>>
>>>>
>>>> Jaegeuk, thoughts?
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
>>>>> ---
>>>>>     fs/f2fs/data.c | 6 ++++++
>>>>>     fs/f2fs/f2fs.h | 2 ++
>>>>>     2 files changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>> index d2cf48c5a2e4..60510acf91ec 100644
>>>>> --- a/fs/f2fs/data.c
>>>>> +++ b/fs/f2fs/data.c
>>>>> @@ -1477,6 +1477,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);
>>>>>             goto out;
>>>>>         }
>>>>> @@ -1526,6 +1529,9 @@ int f2fs_map_blocks(struct inode *inode,
>>>>> struct f2fs_map_blocks *map,
>>>>>                 blkaddr = dn.data_blkaddr;
>>>>>                 set_inode_flag(inode, FI_APPEND_WRITE);
>>>>>             }
>>>>> +        if (!create && !f2fs_lfs_mode(sbi) && flag ==
>>>>> F2FS_GET_BLOCK_DIO &&
>>>>> +                map->m_may_create)
>>>>> +            stat_inc_inplace_blocks(sbi);
>>>>>         } else {
>>>>>             if (create) {
>>>>>                 if (unlikely(f2fs_cp_error(sbi))) {
>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>> index 867f2c5d9559..3a9df28e6fd7 100644
>>>>> --- a/fs/f2fs/f2fs.h
>>>>> +++ b/fs/f2fs/f2fs.h
>>>>> @@ -3804,6 +3804,8 @@ static inline struct f2fs_stat_info
>>>>> *F2FS_STAT(struct f2fs_sb_info *sbi)
>>>>>             ((sbi)->block_count[(curseg)->alloc_type]++)
>>>>>     #define stat_inc_inplace_blocks(sbi)                    \
>>>>>             (atomic_inc(&(sbi)->inplace_count))
>>>>> +#define stat_add_inplace_blocks(sbi, count)                \
>>>>> +        (atomic_add(count, &(sbi)->inplace_count))
>>>>>     #define stat_update_max_atomic_write(inode)                \
>>>>>         do {                                \
>>>>>             int cur = F2FS_I_SB(inode)->atomic_files;    \
>>>>>
>>


_______________________________________________
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] f2fs: fix missing inplace count in overwrite with direct io
  2021-08-20  9:41         ` Chao Yu
@ 2021-08-23 12:07           ` Fengnan Chang
  2021-08-24  0:09             ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Fengnan Chang @ 2021-08-23 12:07 UTC (permalink / raw)
  To: Chao Yu, Jaegeuk Kim; +Cc: linux-f2fs-devel



On 2021/8/20 17:41, Chao Yu wrote:
> On 2021/8/18 11:49, Fengnan Chang wrote:
>>
>>
>> On 2021/8/13 9:36, Chao Yu wrote:
>>> On 2021/8/13 5:15, Jaegeuk Kim wrote:
>>>> On 08/06, Chao Yu wrote:
>>>>> On 2021/7/29 20:25, Fengnan Chang wrote:
>>>>>> For now, overwrite file with direct io use inplace policy, but not
>>>>>> counted, fix it.
>>>>>
>>>>> IMO, LFS/SSR/IPU stats in debugfs was for buffered write, maybe we
>>>>> need to add separated one for DIO.
>>>>
>>>> Do we really need to monitor DIO stats?
>>>
>>> Similar reason as we did for buffered IO?
>>
>> For now, LFS & SSR are count in DIO, but not count IPU,  I think we
> 
> I guess it will account IOs which are fallbacking from DIO to buffered IO,
> so all DIOs are not accounted...

It seems not, the account was done in 
f2fs_allocate_data_block->stat_inc_block_count, when direct + append 
write file, it will count DIO too, because stat_inc_block_count doesn't 
care about DIO or not.

root@kvm-xfstests:~# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3
   [---------|-|----------------------------------------]

IPU: 0 blocks
SSR: 0 blocks in 0 segments
LFS: 0 blocks in 0 segments
root@kvm-xfstests:/mnt# dd if=/dev/zero of=./new oflag=direct bs=1M count=1
1+0 records in
1+0 records out
1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.016008 s, 65.5 MB/s
root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3
   [---------|-|----------------------------------------]

IPU: 0 blocks
SSR: 0 blocks in 0 segments
LFS: 256 blocks in 1 segments

BDF: 99, avg. vblocks: 226

> 
> Thanks,
> 
>> should keep consistency.
>>
>>>
>>> Thanks,
>>>
>>>>
>>>>>
>>>>> Jaegeuk, thoughts?
>>>>>
>>>>> Thanks,
>>>>>
>>>>>>
>>>>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
>>>>>> ---
>>>>>>     fs/f2fs/data.c | 6 ++++++
>>>>>>     fs/f2fs/f2fs.h | 2 ++
>>>>>>     2 files changed, 8 insertions(+)
>>>>>>
>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>> index d2cf48c5a2e4..60510acf91ec 100644
>>>>>> --- a/fs/f2fs/data.c
>>>>>> +++ b/fs/f2fs/data.c
>>>>>> @@ -1477,6 +1477,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);
>>>>>>             goto out;
>>>>>>         }
>>>>>> @@ -1526,6 +1529,9 @@ int f2fs_map_blocks(struct inode *inode,
>>>>>> struct f2fs_map_blocks *map,
>>>>>>                 blkaddr = dn.data_blkaddr;
>>>>>>                 set_inode_flag(inode, FI_APPEND_WRITE);
>>>>>>             }
>>>>>> +        if (!create && !f2fs_lfs_mode(sbi) && flag ==
>>>>>> F2FS_GET_BLOCK_DIO &&
>>>>>> +                map->m_may_create)
>>>>>> +            stat_inc_inplace_blocks(sbi);
>>>>>>         } else {
>>>>>>             if (create) {
>>>>>>                 if (unlikely(f2fs_cp_error(sbi))) {
>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>>> index 867f2c5d9559..3a9df28e6fd7 100644
>>>>>> --- a/fs/f2fs/f2fs.h
>>>>>> +++ b/fs/f2fs/f2fs.h
>>>>>> @@ -3804,6 +3804,8 @@ static inline struct f2fs_stat_info
>>>>>> *F2FS_STAT(struct f2fs_sb_info *sbi)
>>>>>>             ((sbi)->block_count[(curseg)->alloc_type]++)
>>>>>>     #define stat_inc_inplace_blocks(sbi)                    \
>>>>>>             (atomic_inc(&(sbi)->inplace_count))
>>>>>> +#define stat_add_inplace_blocks(sbi, count)                \
>>>>>> +        (atomic_add(count, &(sbi)->inplace_count))
>>>>>>     #define stat_update_max_atomic_write(inode)                \
>>>>>>         do {                                \
>>>>>>             int cur = F2FS_I_SB(inode)->atomic_files;    \
>>>>>>
>>>
> 


_______________________________________________
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] f2fs: fix missing inplace count in overwrite with direct io
  2021-08-23 12:07           ` Fengnan Chang
@ 2021-08-24  0:09             ` Chao Yu
  2021-08-24  3:01               ` Fengnan Chang
  0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2021-08-24  0:09 UTC (permalink / raw)
  To: Fengnan Chang, Jaegeuk Kim; +Cc: linux-f2fs-devel

On 2021/8/23 20:07, Fengnan Chang wrote:
> 
> 
> On 2021/8/20 17:41, Chao Yu wrote:
>> On 2021/8/18 11:49, Fengnan Chang wrote:
>>>
>>>
>>> On 2021/8/13 9:36, Chao Yu wrote:
>>>> On 2021/8/13 5:15, Jaegeuk Kim wrote:
>>>>> On 08/06, Chao Yu wrote:
>>>>>> On 2021/7/29 20:25, Fengnan Chang wrote:
>>>>>>> For now, overwrite file with direct io use inplace policy, but not
>>>>>>> counted, fix it.
>>>>>>
>>>>>> IMO, LFS/SSR/IPU stats in debugfs was for buffered write, maybe we
>>>>>> need to add separated one for DIO.
>>>>>
>>>>> Do we really need to monitor DIO stats?
>>>>
>>>> Similar reason as we did for buffered IO?
>>>
>>> For now, LFS & SSR are count in DIO, but not count IPU,  I think we
>>
>> I guess it will account IOs which are fallbacking from DIO to buffered IO,
>> so all DIOs are not accounted...
> 
> It seems not, the account was done in
> f2fs_allocate_data_block->stat_inc_block_count, when direct + append
> write file, it will count DIO too, because stat_inc_block_count doesn't
> care about DIO or not.

Correct.

> 
> root@kvm-xfstests:~# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3
>     [---------|-|----------------------------------------]
> 
> IPU: 0 blocks
> SSR: 0 blocks in 0 segments
> LFS: 0 blocks in 0 segments

Output like this?
	buffer		direct		segments
IPU: 					N/A
SSR:
LFS:

Thanks,

> root@kvm-xfstests:/mnt# dd if=/dev/zero of=./new oflag=direct bs=1M count=1
> 1+0 records in
> 1+0 records out
> 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.016008 s, 65.5 MB/s
> root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3
>     [---------|-|----------------------------------------]
> 
> IPU: 0 blocks
> SSR: 0 blocks in 0 segments
> LFS: 256 blocks in 1 segments
> 
> BDF: 99, avg. vblocks: 226
> 
>>
>> Thanks,
>>
>>> should keep consistency.
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>>>
>>>>>> Jaegeuk, thoughts?
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
>>>>>>> ---
>>>>>>>      fs/f2fs/data.c | 6 ++++++
>>>>>>>      fs/f2fs/f2fs.h | 2 ++
>>>>>>>      2 files changed, 8 insertions(+)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>> index d2cf48c5a2e4..60510acf91ec 100644
>>>>>>> --- a/fs/f2fs/data.c
>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>> @@ -1477,6 +1477,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);
>>>>>>>              goto out;
>>>>>>>          }
>>>>>>> @@ -1526,6 +1529,9 @@ int f2fs_map_blocks(struct inode *inode,
>>>>>>> struct f2fs_map_blocks *map,
>>>>>>>                  blkaddr = dn.data_blkaddr;
>>>>>>>                  set_inode_flag(inode, FI_APPEND_WRITE);
>>>>>>>              }
>>>>>>> +        if (!create && !f2fs_lfs_mode(sbi) && flag ==
>>>>>>> F2FS_GET_BLOCK_DIO &&
>>>>>>> +                map->m_may_create)
>>>>>>> +            stat_inc_inplace_blocks(sbi);
>>>>>>>          } else {
>>>>>>>              if (create) {
>>>>>>>                  if (unlikely(f2fs_cp_error(sbi))) {
>>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>>>> index 867f2c5d9559..3a9df28e6fd7 100644
>>>>>>> --- a/fs/f2fs/f2fs.h
>>>>>>> +++ b/fs/f2fs/f2fs.h
>>>>>>> @@ -3804,6 +3804,8 @@ static inline struct f2fs_stat_info
>>>>>>> *F2FS_STAT(struct f2fs_sb_info *sbi)
>>>>>>>              ((sbi)->block_count[(curseg)->alloc_type]++)
>>>>>>>      #define stat_inc_inplace_blocks(sbi)                    \
>>>>>>>              (atomic_inc(&(sbi)->inplace_count))
>>>>>>> +#define stat_add_inplace_blocks(sbi, count)                \
>>>>>>> +        (atomic_add(count, &(sbi)->inplace_count))
>>>>>>>      #define stat_update_max_atomic_write(inode)                \
>>>>>>>          do {                                \
>>>>>>>              int cur = F2FS_I_SB(inode)->atomic_files;    \
>>>>>>>
>>>>
>>


_______________________________________________
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] f2fs: fix missing inplace count in overwrite with direct io
  2021-08-24  0:09             ` Chao Yu
@ 2021-08-24  3:01               ` Fengnan Chang
  0 siblings, 0 replies; 9+ messages in thread
From: Fengnan Chang @ 2021-08-24  3:01 UTC (permalink / raw)
  To: Chao Yu, Jaegeuk Kim; +Cc: linux-f2fs-devel



On 2021/8/24 8:09, Chao Yu wrote:
> On 2021/8/23 20:07, Fengnan Chang wrote:
>>
>>
>> On 2021/8/20 17:41, Chao Yu wrote:
>>> On 2021/8/18 11:49, Fengnan Chang wrote:
>>>>
>>>>
>>>> On 2021/8/13 9:36, Chao Yu wrote:
>>>>> On 2021/8/13 5:15, Jaegeuk Kim wrote:
>>>>>> On 08/06, Chao Yu wrote:
>>>>>>> On 2021/7/29 20:25, Fengnan Chang wrote:
>>>>>>>> For now, overwrite file with direct io use inplace policy, but not
>>>>>>>> counted, fix it.
>>>>>>>
>>>>>>> IMO, LFS/SSR/IPU stats in debugfs was for buffered write, maybe we
>>>>>>> need to add separated one for DIO.
>>>>>>
>>>>>> Do we really need to monitor DIO stats?
>>>>>
>>>>> Similar reason as we did for buffered IO?
>>>>
>>>> For now, LFS & SSR are count in DIO, but not count IPU,  I think we
>>>
>>> I guess it will account IOs which are fallbacking from DIO to 
>>> buffered IO,
>>> so all DIOs are not accounted...
>>
>> It seems not, the account was done in
>> f2fs_allocate_data_block->stat_inc_block_count, when direct + append
>> write file, it will count DIO too, because stat_inc_block_count doesn't
>> care about DIO or not.
> 
> Correct.
> 
>>
>> root@kvm-xfstests:~# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3
>>     [---------|-|----------------------------------------]
>>
>> IPU: 0 blocks
>> SSR: 0 blocks in 0 segments
>> LFS: 0 blocks in 0 segments
> 
> Output like this?
>      buffer        direct        segments
> IPU:                     N/A
> SSR:
> LFS:
> 
> Thanks,

I like this, I will send a new patch later.

Thanks.
> 
>> root@kvm-xfstests:/mnt# dd if=/dev/zero of=./new oflag=direct bs=1M 
>> count=1
>> 1+0 records in
>> 1+0 records out
>> 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.016008 s, 65.5 MB/s
>> root@kvm-xfstests:/mnt# cat /sys/kernel/debug/f2fs/status |grep SSR -C 3
>>     [---------|-|----------------------------------------]
>>
>> IPU: 0 blocks
>> SSR: 0 blocks in 0 segments
>> LFS: 256 blocks in 1 segments
>>
>> BDF: 99, avg. vblocks: 226
>>
>>>
>>> Thanks,
>>>
>>>> should keep consistency.
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Jaegeuk, thoughts?
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
>>>>>>>> ---
>>>>>>>>      fs/f2fs/data.c | 6 ++++++
>>>>>>>>      fs/f2fs/f2fs.h | 2 ++
>>>>>>>>      2 files changed, 8 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>>> index d2cf48c5a2e4..60510acf91ec 100644
>>>>>>>> --- a/fs/f2fs/data.c
>>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>>> @@ -1477,6 +1477,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);
>>>>>>>>              goto out;
>>>>>>>>          }
>>>>>>>> @@ -1526,6 +1529,9 @@ int f2fs_map_blocks(struct inode *inode,
>>>>>>>> struct f2fs_map_blocks *map,
>>>>>>>>                  blkaddr = dn.data_blkaddr;
>>>>>>>>                  set_inode_flag(inode, FI_APPEND_WRITE);
>>>>>>>>              }
>>>>>>>> +        if (!create && !f2fs_lfs_mode(sbi) && flag ==
>>>>>>>> F2FS_GET_BLOCK_DIO &&
>>>>>>>> +                map->m_may_create)
>>>>>>>> +            stat_inc_inplace_blocks(sbi);
>>>>>>>>          } else {
>>>>>>>>              if (create) {
>>>>>>>>                  if (unlikely(f2fs_cp_error(sbi))) {
>>>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>>>>> index 867f2c5d9559..3a9df28e6fd7 100644
>>>>>>>> --- a/fs/f2fs/f2fs.h
>>>>>>>> +++ b/fs/f2fs/f2fs.h
>>>>>>>> @@ -3804,6 +3804,8 @@ static inline struct f2fs_stat_info
>>>>>>>> *F2FS_STAT(struct f2fs_sb_info *sbi)
>>>>>>>>              ((sbi)->block_count[(curseg)->alloc_type]++)
>>>>>>>>      #define stat_inc_inplace_blocks(sbi)                    \
>>>>>>>>              (atomic_inc(&(sbi)->inplace_count))
>>>>>>>> +#define stat_add_inplace_blocks(sbi, count)                \
>>>>>>>> +        (atomic_add(count, &(sbi)->inplace_count))
>>>>>>>>      #define stat_update_max_atomic_write(inode)                \
>>>>>>>>          do {                                \
>>>>>>>>              int cur = F2FS_I_SB(inode)->atomic_files;    \
>>>>>>>>
>>>>>
>>>
> 


_______________________________________________
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-08-24  3:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 12:25 [f2fs-dev] [PATCH] f2fs: fix missing inplace count in overwrite with direct io Fengnan Chang
2021-08-06  1:00 ` Chao Yu
2021-08-12 21:15   ` Jaegeuk Kim
2021-08-13  1:36     ` Chao Yu
2021-08-18  3:49       ` Fengnan Chang
2021-08-20  9:41         ` Chao Yu
2021-08-23 12:07           ` Fengnan Chang
2021-08-24  0:09             ` Chao Yu
2021-08-24  3:01               ` Fengnan Chang

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.