* [PATCH v3] F2FS: invalidate META_MAPPING before IPU/DIO write
[not found] <CGME20211102045952epcas1p44cb6fd41baa76a19e0924c4b6b3cf1e6@epcas1p4.samsung.com>
@ 2021-11-02 4:59 ` Hyeong-Jun Kim
2021-11-02 6:47 ` Chao Yu
0 siblings, 1 reply; 4+ messages in thread
From: Hyeong-Jun Kim @ 2021-11-02 4:59 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu
Cc: sj1557.seo, Hyeong-Jun Kim, linux-f2fs-devel, linux-kernel
Encrypted pages during GC are read and cached in META_MAPPING.
However, due to cached pages in META_MAPPING, there is an issue where
newly written pages are lost by IPU or DIO writes.
Thread A - f2fs_gc() Thread B
/* phase 3 */
down_write(i_gc_rwsem)
ra_data_block() ---- (a)
up_write(i_gc_rwsem)
f2fs_direct_IO() :
- down_read(i_gc_rwsem)
- __blockdev_direct_io()
- get_data_block_dio_write()
- f2fs_dio_submit_bio() ---- (b)
- up_read(i_gc_rwsem)
/* phase 4 */
down_write(i_gc_rwsem)
move_data_block() ---- (c)
up_write(i_gc_rwsem)
(a) In phase 3 of f2fs_gc(), up-to-date page is read from storage and
cached in META_MAPPING.
(b) In thread B, writing new data by IPU or DIO write on same blkaddr as
read in (a). cached page in META_MAPPING become out-dated.
(c) In phase 4 of f2fs_gc(), out-dated page in META_MAPPING is copied to
new blkaddr. In conclusion, the newly written data in (b) is lost.
To address this issue, invalidating pages in META_MAPPING before IPU or
DIO write.
---
v3:
- Use sbi instead of fio->sbi in f2fs_inplace_write_data()
v2:
- Update patch description with race condition
Fixes: 6aa58d8ad20a ("f2fs: readahead encrypted block during GC")
Signed-off-by: Hyeong-Jun Kim <hj514.kim@samsung.com>
---
fs/f2fs/data.c | 2 ++
fs/f2fs/segment.c | 3 +++
2 files changed, 5 insertions(+)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 74e1a350c1d8..9f754aaef558 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1708,6 +1708,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
*/
f2fs_wait_on_block_writeback_range(inode,
map->m_pblk, map->m_len);
+ invalidate_mapping_pages(META_MAPPING(sbi),
+ map->m_pblk, map->m_pblk);
if (map->m_multidev_dio) {
block_t blk_addr = map->m_pblk;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 526423fe84ce..df9ed75f0b7a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3652,6 +3652,9 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
goto drop_bio;
}
+ invalidate_mapping_pages(META_MAPPING(sbi),
+ fio->new_blkaddr, fio->new_blkaddr);
+
stat_inc_inplace_blocks(fio->sbi);
if (fio->bio && !(SM_I(sbi)->ipu_policy & (1 << F2FS_IPU_NOCACHE)))
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] F2FS: invalidate META_MAPPING before IPU/DIO write
2021-11-02 4:59 ` [PATCH v3] F2FS: invalidate META_MAPPING before IPU/DIO write Hyeong-Jun Kim
@ 2021-11-02 6:47 ` Chao Yu
2021-11-02 6:49 ` Hyeong-Jun Kim
0 siblings, 1 reply; 4+ messages in thread
From: Chao Yu @ 2021-11-02 6:47 UTC (permalink / raw)
To: Hyeong-Jun Kim, Jaegeuk Kim; +Cc: sj1557.seo, linux-f2fs-devel, linux-kernel
On 2021/11/2 12:59, Hyeong-Jun Kim wrote:
> Encrypted pages during GC are read and cached in META_MAPPING.
> However, due to cached pages in META_MAPPING, there is an issue where
> newly written pages are lost by IPU or DIO writes.
>
> Thread A - f2fs_gc() Thread B
> /* phase 3 */
> down_write(i_gc_rwsem)
> ra_data_block() ---- (a)
> up_write(i_gc_rwsem)
> f2fs_direct_IO() :
> - down_read(i_gc_rwsem)
> - __blockdev_direct_io()
> - get_data_block_dio_write()
> - f2fs_dio_submit_bio() ---- (b)
> - up_read(i_gc_rwsem)
> /* phase 4 */
> down_write(i_gc_rwsem)
> move_data_block() ---- (c)
> up_write(i_gc_rwsem)
>
> (a) In phase 3 of f2fs_gc(), up-to-date page is read from storage and
> cached in META_MAPPING.
> (b) In thread B, writing new data by IPU or DIO write on same blkaddr as
> read in (a). cached page in META_MAPPING become out-dated.
> (c) In phase 4 of f2fs_gc(), out-dated page in META_MAPPING is copied to
> new blkaddr. In conclusion, the newly written data in (b) is lost.
>
> To address this issue, invalidating pages in META_MAPPING before IPU or
> DIO write.
>
> ---
> v3:
> - Use sbi instead of fio->sbi in f2fs_inplace_write_data()
> v2:
> - Update patch description with race condition
>
> Fixes: 6aa58d8ad20a ("f2fs: readahead encrypted block during GC")
> Signed-off-by: Hyeong-Jun Kim <hj514.kim@samsung.com>
> ---
Change log should be attached here, otherwise Fixes & Signed-off-by tag will be lost
after applying this patch.
Thanks,
> fs/f2fs/data.c | 2 ++
> fs/f2fs/segment.c | 3 +++
> 2 files changed, 5 insertions(+)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 74e1a350c1d8..9f754aaef558 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1708,6 +1708,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
> */
> f2fs_wait_on_block_writeback_range(inode,
> map->m_pblk, map->m_len);
> + invalidate_mapping_pages(META_MAPPING(sbi),
> + map->m_pblk, map->m_pblk);
>
> if (map->m_multidev_dio) {
> block_t blk_addr = map->m_pblk;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 526423fe84ce..df9ed75f0b7a 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -3652,6 +3652,9 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
> goto drop_bio;
> }
>
> + invalidate_mapping_pages(META_MAPPING(sbi),
> + fio->new_blkaddr, fio->new_blkaddr);
> +
> stat_inc_inplace_blocks(fio->sbi);
>
> if (fio->bio && !(SM_I(sbi)->ipu_policy & (1 << F2FS_IPU_NOCACHE)))
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] F2FS: invalidate META_MAPPING before IPU/DIO write
2021-11-02 6:47 ` Chao Yu
@ 2021-11-02 6:49 ` Hyeong-Jun Kim
2021-11-02 6:50 ` Chao Yu
0 siblings, 1 reply; 4+ messages in thread
From: Hyeong-Jun Kim @ 2021-11-02 6:49 UTC (permalink / raw)
To: Chao Yu, Jaegeuk Kim; +Cc: sj1557.seo, linux-f2fs-devel, linux-kernel
On Tue, 2021-11-02 at 14:47 +0800, Chao Yu wrote:
> On 2021/11/2 12:59, Hyeong-Jun Kim wrote:
> > Encrypted pages during GC are read and cached in META_MAPPING.
> > However, due to cached pages in META_MAPPING, there is an issue
> > where
> > newly written pages are lost by IPU or DIO writes.
> >
> > Thread A - f2fs_gc() Thread B
> > /* phase 3 */
> > down_write(i_gc_rwsem)
> > ra_data_block() ---- (a)
> > up_write(i_gc_rwsem)
> > f2fs_direct_IO() :
> > - down_read(i_gc_rwsem)
> > - __blockdev_direct_io()
> > - get_data_block_dio_write()
> > - f2fs_dio_submit_bio() ---- (b)
> > - up_read(i_gc_rwsem)
> > /* phase 4 */
> > down_write(i_gc_rwsem)
> > move_data_block() ---- (c)
> > up_write(i_gc_rwsem)
> >
> > (a) In phase 3 of f2fs_gc(), up-to-date page is read from storage
> > and
> > cached in META_MAPPING.
> > (b) In thread B, writing new data by IPU or DIO write on same
> > blkaddr as
> > read in (a). cached page in META_MAPPING become out-dated.
> > (c) In phase 4 of f2fs_gc(), out-dated page in META_MAPPING is
> > copied to
> > new blkaddr. In conclusion, the newly written data in (b) is
> > lost.
> >
> > To address this issue, invalidating pages in META_MAPPING before
> > IPU or
> > DIO write.
> >
> > ---
> > v3:
> > - Use sbi instead of fio->sbi in f2fs_inplace_write_data()
> > v2:
> > - Update patch description with race condition
>
>
>
> > Fixes: 6aa58d8ad20a ("f2fs: readahead encrypted block during GC")
> > Signed-off-by: Hyeong-Jun Kim <
> > hj514.kim@samsung.com
> > >
> > ---
>
> Change log should be attached here, otherwise Fixes & Signed-off-by
> tag will be lost
> after applying this patch.
>
> Thanks,
>
Thanks for your comment.
Should I re-send this patch?
Thanks,
> > fs/f2fs/data.c | 2 ++
> > fs/f2fs/segment.c | 3 +++
> > 2 files changed, 5 insertions(+)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 74e1a350c1d8..9f754aaef558 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1708,6 +1708,8 @@ int f2fs_map_blocks(struct inode *inode,
> > struct f2fs_map_blocks *map,
> > */
> > f2fs_wait_on_block_writeback_range(inode,
> > map->m_pblk, map-
> > >m_len);
> > + invalidate_mapping_pages(META_MAPPING(sbi),
> > + map->m_pblk, map-
> > >m_pblk);
> >
> > if (map->m_multidev_dio) {
> > block_t blk_addr = map->m_pblk;
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 526423fe84ce..df9ed75f0b7a 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -3652,6 +3652,9 @@ int f2fs_inplace_write_data(struct
> > f2fs_io_info *fio)
> > goto drop_bio;
> > }
> >
> > + invalidate_mapping_pages(META_MAPPING(sbi),
> > + fio->new_blkaddr, fio->new_blkaddr);
> > +
> > stat_inc_inplace_blocks(fio->sbi);
> >
> > if (fio->bio && !(SM_I(sbi)->ipu_policy & (1 <<
> > F2FS_IPU_NOCACHE)))
> >
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] F2FS: invalidate META_MAPPING before IPU/DIO write
2021-11-02 6:49 ` Hyeong-Jun Kim
@ 2021-11-02 6:50 ` Chao Yu
0 siblings, 0 replies; 4+ messages in thread
From: Chao Yu @ 2021-11-02 6:50 UTC (permalink / raw)
To: Hyeong-Jun Kim, Jaegeuk Kim; +Cc: sj1557.seo, linux-f2fs-devel, linux-kernel
On 2021/11/2 14:49, Hyeong-Jun Kim wrote:
> On Tue, 2021-11-02 at 14:47 +0800, Chao Yu wrote:
>> On 2021/11/2 12:59, Hyeong-Jun Kim wrote:
>>> Encrypted pages during GC are read and cached in META_MAPPING.
>>> However, due to cached pages in META_MAPPING, there is an issue
>>> where
>>> newly written pages are lost by IPU or DIO writes.
>>>
>>> Thread A - f2fs_gc() Thread B
>>> /* phase 3 */
>>> down_write(i_gc_rwsem)
>>> ra_data_block() ---- (a)
>>> up_write(i_gc_rwsem)
>>> f2fs_direct_IO() :
>>> - down_read(i_gc_rwsem)
>>> - __blockdev_direct_io()
>>> - get_data_block_dio_write()
>>> - f2fs_dio_submit_bio() ---- (b)
>>> - up_read(i_gc_rwsem)
>>> /* phase 4 */
>>> down_write(i_gc_rwsem)
>>> move_data_block() ---- (c)
>>> up_write(i_gc_rwsem)
>>>
>>> (a) In phase 3 of f2fs_gc(), up-to-date page is read from storage
>>> and
>>> cached in META_MAPPING.
>>> (b) In thread B, writing new data by IPU or DIO write on same
>>> blkaddr as
>>> read in (a). cached page in META_MAPPING become out-dated.
>>> (c) In phase 4 of f2fs_gc(), out-dated page in META_MAPPING is
>>> copied to
>>> new blkaddr. In conclusion, the newly written data in (b) is
>>> lost.
>>>
>>> To address this issue, invalidating pages in META_MAPPING before
>>> IPU or
>>> DIO write.
>>>
>>> ---
>>> v3:
>>> - Use sbi instead of fio->sbi in f2fs_inplace_write_data()
>>> v2:
>>> - Update patch description with race condition
>>
>>
>>
>>> Fixes: 6aa58d8ad20a ("f2fs: readahead encrypted block during GC")
>>> Signed-off-by: Hyeong-Jun Kim <
>>> hj514.kim@samsung.com
>>>>
>>> ---
>>
>> Change log should be attached here, otherwise Fixes & Signed-off-by
>> tag will be lost
>> after applying this patch.
>>
>> Thanks,
>>
> Thanks for your comment.
> Should I re-send this patch?
Yes, please. :)
Thanks,
>
> Thanks,
>>> fs/f2fs/data.c | 2 ++
>>> fs/f2fs/segment.c | 3 +++
>>> 2 files changed, 5 insertions(+)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 74e1a350c1d8..9f754aaef558 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1708,6 +1708,8 @@ int f2fs_map_blocks(struct inode *inode,
>>> struct f2fs_map_blocks *map,
>>> */
>>> f2fs_wait_on_block_writeback_range(inode,
>>> map->m_pblk, map-
>>>> m_len);
>>> + invalidate_mapping_pages(META_MAPPING(sbi),
>>> + map->m_pblk, map-
>>>> m_pblk);
>>>
>>> if (map->m_multidev_dio) {
>>> block_t blk_addr = map->m_pblk;
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 526423fe84ce..df9ed75f0b7a 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -3652,6 +3652,9 @@ int f2fs_inplace_write_data(struct
>>> f2fs_io_info *fio)
>>> goto drop_bio;
>>> }
>>>
>>> + invalidate_mapping_pages(META_MAPPING(sbi),
>>> + fio->new_blkaddr, fio->new_blkaddr);
>>> +
>>> stat_inc_inplace_blocks(fio->sbi);
>>>
>>> if (fio->bio && !(SM_I(sbi)->ipu_policy & (1 <<
>>> F2FS_IPU_NOCACHE)))
>>>
>>
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-02 6:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20211102045952epcas1p44cb6fd41baa76a19e0924c4b6b3cf1e6@epcas1p4.samsung.com>
2021-11-02 4:59 ` [PATCH v3] F2FS: invalidate META_MAPPING before IPU/DIO write Hyeong-Jun Kim
2021-11-02 6:47 ` Chao Yu
2021-11-02 6:49 ` Hyeong-Jun Kim
2021-11-02 6:50 ` Chao Yu
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).