From: Chao Yu <chao@kernel.org>
To: 常凤楠 <changfengnan@vivo.com>, "jaegeuk@kernel.org" <jaegeuk@kernel.org>
Cc: "linux-f2fs-devel@lists.sourceforge.net"
<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [f2fs-dev] [PATCH v2 2/2] f2fs: fix missing inplace count in overwrite with direct io
Date: Fri, 17 Sep 2021 20:59:32 +0800 [thread overview]
Message-ID: <2acea857-c3e7-902a-a016-bcd5aeedc407@kernel.org> (raw)
In-Reply-To: <KL1PR0601MB40030D121F65550D690A776DBBDD9@KL1PR0601MB4003.apcprd06.prod.outlook.com>
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
next prev parent reply other threads:[~2021-09-17 12:59 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
[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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2acea857-c3e7-902a-a016-bcd5aeedc407@kernel.org \
--to=chao@kernel.org \
--cc=changfengnan@vivo.com \
--cc=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).