* [PATCH v4] f2fs: add bio cache for IPU
@ 2019-02-19 8:15 Chao Yu
2019-02-28 3:49 ` Chao Yu
2019-08-31 7:23 ` Chao Yu
0 siblings, 2 replies; 10+ messages in thread
From: Chao Yu @ 2019-02-19 8:15 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
SQLite in Wal mode may trigger sequential IPU write in db-wal file, after
commit d1b3e72d5490 ("f2fs: submit bio of in-place-update pages"), we
lost the chance of merging page in inner managed bio cache, result in
submitting more small-sized IO.
So let's add temporary bio in writepages() to cache mergeable write IO as
much as possible.
Test case:
1. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
2. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
Before:
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65552, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65560, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65568, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65576, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65584, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65592, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65600, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65608, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65616, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65624, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65632, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65640, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65648, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65656, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65664, size = 4096
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57352, size = 4096
After:
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 65536
f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57368, size = 4096
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
v4:
- fix to set *bio to NULL in f2fs_submit_ipu_bio()
- spread f2fs_submit_ipu_bio()
- fix to count dirty encrypted page correctly in f2fs_merge_page_bio()
- remove redundant assignment of fio->last_block
fs/f2fs/data.c | 88 ++++++++++++++++++++++++++++++++++++++++++-----
fs/f2fs/f2fs.h | 3 ++
fs/f2fs/segment.c | 5 ++-
3 files changed, 86 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 35910ff23582..e4c183e85de8 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -296,20 +296,20 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
io->bio = NULL;
}
-static bool __has_merged_page(struct f2fs_bio_info *io, struct inode *inode,
+static bool __has_merged_page(struct bio *bio, struct inode *inode,
struct page *page, nid_t ino)
{
struct bio_vec *bvec;
struct page *target;
int i;
- if (!io->bio)
+ if (!bio)
return false;
if (!inode && !page && !ino)
return true;
- bio_for_each_segment_all(bvec, io->bio, i) {
+ bio_for_each_segment_all(bvec, bio, i) {
if (bvec->bv_page->mapping)
target = bvec->bv_page;
@@ -360,7 +360,7 @@ static void __submit_merged_write_cond(struct f2fs_sb_info *sbi,
struct f2fs_bio_info *io = sbi->write_io[btype] + temp;
down_read(&io->io_rwsem);
- ret = __has_merged_page(io, inode, page, ino);
+ ret = __has_merged_page(io->bio, inode, page, ino);
up_read(&io->io_rwsem);
}
if (ret)
@@ -429,6 +429,61 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
return 0;
}
+int f2fs_merge_page_bio(struct f2fs_io_info *fio)
+{
+ struct bio *bio = *fio->bio;
+ struct page *page = fio->encrypted_page ?
+ fio->encrypted_page : fio->page;
+
+ if (!f2fs_is_valid_blkaddr(fio->sbi, fio->new_blkaddr,
+ __is_meta_io(fio) ? META_GENERIC : DATA_GENERIC))
+ return -EFAULT;
+
+ trace_f2fs_submit_page_bio(page, fio);
+ f2fs_trace_ios(fio, 0);
+
+ if (bio && (*fio->last_block + 1 != fio->new_blkaddr ||
+ !__same_bdev(fio->sbi, fio->new_blkaddr, bio))) {
+ __submit_bio(fio->sbi, bio, fio->type);
+ bio = NULL;
+ }
+alloc_new:
+ if (!bio) {
+ bio = __bio_alloc(fio->sbi, fio->new_blkaddr, fio->io_wbc,
+ BIO_MAX_PAGES, false, fio->type, fio->temp);
+ bio_set_op_attrs(bio, fio->op, fio->op_flags);
+ }
+
+ if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
+ __submit_bio(fio->sbi, bio, fio->type);
+ bio = NULL;
+ goto alloc_new;
+ }
+
+ if (fio->io_wbc)
+ wbc_account_io(fio->io_wbc, page, PAGE_SIZE);
+
+ inc_page_count(fio->sbi, WB_DATA_TYPE(page));
+
+ *fio->last_block = fio->new_blkaddr;
+ *fio->bio = bio;
+
+ return 0;
+}
+
+void f2fs_submit_ipu_bio(struct f2fs_sb_info *sbi, struct bio **bio,
+ struct page *page)
+{
+ if (!bio)
+ return;
+
+ if (!__has_merged_page(*bio, NULL, page, 0))
+ return;
+
+ __submit_bio(sbi, *bio, DATA);
+ *bio = NULL;
+}
+
void f2fs_submit_page_write(struct f2fs_io_info *fio)
{
struct f2fs_sb_info *sbi = fio->sbi;
@@ -1854,6 +1909,8 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
}
static int __write_data_page(struct page *page, bool *submitted,
+ struct bio **bio,
+ sector_t *last_block,
struct writeback_control *wbc,
enum iostat_type io_type)
{
@@ -1879,6 +1936,8 @@ static int __write_data_page(struct page *page, bool *submitted,
.need_lock = LOCK_RETRY,
.io_type = io_type,
.io_wbc = wbc,
+ .bio = bio,
+ .last_block = last_block,
};
trace_f2fs_writepage(page, DATA);
@@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
}
unlock_page(page);
- if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
+ if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
+ f2fs_submit_ipu_bio(sbi, bio, page);
f2fs_balance_fs(sbi, need_balance_fs);
+ }
if (unlikely(f2fs_cp_error(sbi))) {
+ f2fs_submit_ipu_bio(sbi, bio, page);
f2fs_submit_merged_write(sbi, DATA);
submitted = NULL;
}
@@ -2006,7 +2068,7 @@ static int __write_data_page(struct page *page, bool *submitted,
static int f2fs_write_data_page(struct page *page,
struct writeback_control *wbc)
{
- return __write_data_page(page, NULL, wbc, FS_DATA_IO);
+ return __write_data_page(page, NULL, NULL, NULL, wbc, FS_DATA_IO);
}
/*
@@ -2022,6 +2084,8 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
int done = 0;
struct pagevec pvec;
struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
+ struct bio *bio = NULL;
+ sector_t last_block;
int nr_pages;
pgoff_t uninitialized_var(writeback_index);
pgoff_t index;
@@ -2098,17 +2162,20 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
}
if (PageWriteback(page)) {
- if (wbc->sync_mode != WB_SYNC_NONE)
+ if (wbc->sync_mode != WB_SYNC_NONE) {
f2fs_wait_on_page_writeback(page,
DATA, true, true);
- else
+ f2fs_submit_ipu_bio(sbi, &bio, page);
+ } else {
goto continue_unlock;
+ }
}
if (!clear_page_dirty_for_io(page))
goto continue_unlock;
- ret = __write_data_page(page, &submitted, wbc, io_type);
+ ret = __write_data_page(page, &submitted, &bio,
+ &last_block, wbc, io_type);
if (unlikely(ret)) {
/*
* keep nr_to_write, since vfs uses this to
@@ -2157,6 +2224,9 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
if (nwritten)
f2fs_submit_merged_write_cond(F2FS_M_SB(mapping), mapping->host,
NULL, 0, DATA);
+ /* submit cached bio of IPU write */
+ if (bio)
+ __submit_bio(sbi, bio, DATA);
return ret;
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 46db3ed87c84..7c4355927d28 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1048,6 +1048,8 @@ struct f2fs_io_info {
bool retry; /* need to reallocate block address */
enum iostat_type io_type; /* io type */
struct writeback_control *io_wbc; /* writeback control */
+ struct bio **bio; /* bio for ipu */
+ sector_t *last_block; /* last block number in bio */
unsigned char version; /* version of the node */
};
@@ -3105,6 +3107,7 @@ void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
nid_t ino, enum page_type type);
void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi);
int f2fs_submit_page_bio(struct f2fs_io_info *fio);
+int f2fs_merge_page_bio(struct f2fs_io_info *fio);
void f2fs_submit_page_write(struct f2fs_io_info *fio);
struct block_device *f2fs_target_device(struct f2fs_sb_info *sbi,
block_t blk_addr, struct bio *bio);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index b2167d53fb9d..66838ab2f21e 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3196,7 +3196,10 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
stat_inc_inplace_blocks(fio->sbi);
- err = f2fs_submit_page_bio(fio);
+ if (fio->bio)
+ err = f2fs_merge_page_bio(fio);
+ else
+ err = f2fs_submit_page_bio(fio);
if (!err)
update_device_state(fio);
--
2.18.0.rc1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add bio cache for IPU
2019-02-19 8:15 [PATCH v4] f2fs: add bio cache for IPU Chao Yu
@ 2019-02-28 3:49 ` Chao Yu
2019-03-01 17:55 ` Jaegeuk Kim
2019-08-31 7:23 ` Chao Yu
1 sibling, 1 reply; 10+ messages in thread
From: Chao Yu @ 2019-02-28 3:49 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao
Ping,
On 2019/2/19 16:15, Chao Yu wrote:
> SQLite in Wal mode may trigger sequential IPU write in db-wal file, after
> commit d1b3e72d5490 ("f2fs: submit bio of in-place-update pages"), we
> lost the chance of merging page in inner managed bio cache, result in
> submitting more small-sized IO.
>
> So let's add temporary bio in writepages() to cache mergeable write IO as
> much as possible.
>
> Test case:
> 1. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
> 2. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
>
> Before:
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65552, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65560, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65568, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65576, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65584, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65592, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65600, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65608, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65616, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65624, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65632, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65640, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65648, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65656, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65664, size = 4096
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57352, size = 4096
>
> After:
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 65536
> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57368, size = 4096
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> v4:
> - fix to set *bio to NULL in f2fs_submit_ipu_bio()
> - spread f2fs_submit_ipu_bio()
> - fix to count dirty encrypted page correctly in f2fs_merge_page_bio()
> - remove redundant assignment of fio->last_block
> fs/f2fs/data.c | 88 ++++++++++++++++++++++++++++++++++++++++++-----
> fs/f2fs/f2fs.h | 3 ++
> fs/f2fs/segment.c | 5 ++-
> 3 files changed, 86 insertions(+), 10 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 35910ff23582..e4c183e85de8 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -296,20 +296,20 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
> io->bio = NULL;
> }
>
> -static bool __has_merged_page(struct f2fs_bio_info *io, struct inode *inode,
> +static bool __has_merged_page(struct bio *bio, struct inode *inode,
> struct page *page, nid_t ino)
> {
> struct bio_vec *bvec;
> struct page *target;
> int i;
>
> - if (!io->bio)
> + if (!bio)
> return false;
>
> if (!inode && !page && !ino)
> return true;
>
> - bio_for_each_segment_all(bvec, io->bio, i) {
> + bio_for_each_segment_all(bvec, bio, i) {
>
> if (bvec->bv_page->mapping)
> target = bvec->bv_page;
> @@ -360,7 +360,7 @@ static void __submit_merged_write_cond(struct f2fs_sb_info *sbi,
> struct f2fs_bio_info *io = sbi->write_io[btype] + temp;
>
> down_read(&io->io_rwsem);
> - ret = __has_merged_page(io, inode, page, ino);
> + ret = __has_merged_page(io->bio, inode, page, ino);
> up_read(&io->io_rwsem);
> }
> if (ret)
> @@ -429,6 +429,61 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> return 0;
> }
>
> +int f2fs_merge_page_bio(struct f2fs_io_info *fio)
> +{
> + struct bio *bio = *fio->bio;
> + struct page *page = fio->encrypted_page ?
> + fio->encrypted_page : fio->page;
> +
> + if (!f2fs_is_valid_blkaddr(fio->sbi, fio->new_blkaddr,
> + __is_meta_io(fio) ? META_GENERIC : DATA_GENERIC))
> + return -EFAULT;
> +
> + trace_f2fs_submit_page_bio(page, fio);
> + f2fs_trace_ios(fio, 0);
> +
> + if (bio && (*fio->last_block + 1 != fio->new_blkaddr ||
> + !__same_bdev(fio->sbi, fio->new_blkaddr, bio))) {
> + __submit_bio(fio->sbi, bio, fio->type);
> + bio = NULL;
> + }
> +alloc_new:
> + if (!bio) {
> + bio = __bio_alloc(fio->sbi, fio->new_blkaddr, fio->io_wbc,
> + BIO_MAX_PAGES, false, fio->type, fio->temp);
> + bio_set_op_attrs(bio, fio->op, fio->op_flags);
> + }
> +
> + if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
> + __submit_bio(fio->sbi, bio, fio->type);
> + bio = NULL;
> + goto alloc_new;
> + }
> +
> + if (fio->io_wbc)
> + wbc_account_io(fio->io_wbc, page, PAGE_SIZE);
> +
> + inc_page_count(fio->sbi, WB_DATA_TYPE(page));
> +
> + *fio->last_block = fio->new_blkaddr;
> + *fio->bio = bio;
> +
> + return 0;
> +}
> +
> +void f2fs_submit_ipu_bio(struct f2fs_sb_info *sbi, struct bio **bio,
> + struct page *page)
> +{
> + if (!bio)
> + return;
> +
> + if (!__has_merged_page(*bio, NULL, page, 0))
> + return;
> +
> + __submit_bio(sbi, *bio, DATA);
> + *bio = NULL;
> +}
> +
> void f2fs_submit_page_write(struct f2fs_io_info *fio)
> {
> struct f2fs_sb_info *sbi = fio->sbi;
> @@ -1854,6 +1909,8 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
> }
>
> static int __write_data_page(struct page *page, bool *submitted,
> + struct bio **bio,
> + sector_t *last_block,
> struct writeback_control *wbc,
> enum iostat_type io_type)
> {
> @@ -1879,6 +1936,8 @@ static int __write_data_page(struct page *page, bool *submitted,
> .need_lock = LOCK_RETRY,
> .io_type = io_type,
> .io_wbc = wbc,
> + .bio = bio,
> + .last_block = last_block,
> };
>
> trace_f2fs_writepage(page, DATA);
> @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
> }
>
> unlock_page(page);
> - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
> + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
> + f2fs_submit_ipu_bio(sbi, bio, page);
> f2fs_balance_fs(sbi, need_balance_fs);
> + }
>
> if (unlikely(f2fs_cp_error(sbi))) {
> + f2fs_submit_ipu_bio(sbi, bio, page);
> f2fs_submit_merged_write(sbi, DATA);
> submitted = NULL;
> }
> @@ -2006,7 +2068,7 @@ static int __write_data_page(struct page *page, bool *submitted,
> static int f2fs_write_data_page(struct page *page,
> struct writeback_control *wbc)
> {
> - return __write_data_page(page, NULL, wbc, FS_DATA_IO);
> + return __write_data_page(page, NULL, NULL, NULL, wbc, FS_DATA_IO);
> }
>
> /*
> @@ -2022,6 +2084,8 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> int done = 0;
> struct pagevec pvec;
> struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
> + struct bio *bio = NULL;
> + sector_t last_block;
> int nr_pages;
> pgoff_t uninitialized_var(writeback_index);
> pgoff_t index;
> @@ -2098,17 +2162,20 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> }
>
> if (PageWriteback(page)) {
> - if (wbc->sync_mode != WB_SYNC_NONE)
> + if (wbc->sync_mode != WB_SYNC_NONE) {
> f2fs_wait_on_page_writeback(page,
> DATA, true, true);
> - else
> + f2fs_submit_ipu_bio(sbi, &bio, page);
> + } else {
> goto continue_unlock;
> + }
> }
>
> if (!clear_page_dirty_for_io(page))
> goto continue_unlock;
>
> - ret = __write_data_page(page, &submitted, wbc, io_type);
> + ret = __write_data_page(page, &submitted, &bio,
> + &last_block, wbc, io_type);
> if (unlikely(ret)) {
> /*
> * keep nr_to_write, since vfs uses this to
> @@ -2157,6 +2224,9 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> if (nwritten)
> f2fs_submit_merged_write_cond(F2FS_M_SB(mapping), mapping->host,
> NULL, 0, DATA);
> + /* submit cached bio of IPU write */
> + if (bio)
> + __submit_bio(sbi, bio, DATA);
>
> return ret;
> }
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 46db3ed87c84..7c4355927d28 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1048,6 +1048,8 @@ struct f2fs_io_info {
> bool retry; /* need to reallocate block address */
> enum iostat_type io_type; /* io type */
> struct writeback_control *io_wbc; /* writeback control */
> + struct bio **bio; /* bio for ipu */
> + sector_t *last_block; /* last block number in bio */
> unsigned char version; /* version of the node */
> };
>
> @@ -3105,6 +3107,7 @@ void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
> nid_t ino, enum page_type type);
> void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi);
> int f2fs_submit_page_bio(struct f2fs_io_info *fio);
> +int f2fs_merge_page_bio(struct f2fs_io_info *fio);
> void f2fs_submit_page_write(struct f2fs_io_info *fio);
> struct block_device *f2fs_target_device(struct f2fs_sb_info *sbi,
> block_t blk_addr, struct bio *bio);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index b2167d53fb9d..66838ab2f21e 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -3196,7 +3196,10 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
>
> stat_inc_inplace_blocks(fio->sbi);
>
> - err = f2fs_submit_page_bio(fio);
> + if (fio->bio)
> + err = f2fs_merge_page_bio(fio);
> + else
> + err = f2fs_submit_page_bio(fio);
> if (!err)
> update_device_state(fio);
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add bio cache for IPU
2019-02-28 3:49 ` Chao Yu
@ 2019-03-01 17:55 ` Jaegeuk Kim
2019-03-04 6:27 ` Chao Yu
2019-05-14 4:55 ` [f2fs-dev] " Ju Hyung Park
0 siblings, 2 replies; 10+ messages in thread
From: Jaegeuk Kim @ 2019-03-01 17:55 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
On 02/28, Chao Yu wrote:
> Ping,
Ditto.
>
> On 2019/2/19 16:15, Chao Yu wrote:
> > SQLite in Wal mode may trigger sequential IPU write in db-wal file, after
> > commit d1b3e72d5490 ("f2fs: submit bio of in-place-update pages"), we
> > lost the chance of merging page in inner managed bio cache, result in
> > submitting more small-sized IO.
> >
> > So let's add temporary bio in writepages() to cache mergeable write IO as
> > much as possible.
> >
> > Test case:
> > 1. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
> > 2. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
> >
> > Before:
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65552, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65560, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65568, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65576, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65584, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65592, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65600, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65608, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65616, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65624, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65632, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65640, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65648, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65656, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65664, size = 4096
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57352, size = 4096
> >
> > After:
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 65536
> > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57368, size = 4096
> >
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > ---
> > v4:
> > - fix to set *bio to NULL in f2fs_submit_ipu_bio()
> > - spread f2fs_submit_ipu_bio()
> > - fix to count dirty encrypted page correctly in f2fs_merge_page_bio()
> > - remove redundant assignment of fio->last_block
> > fs/f2fs/data.c | 88 ++++++++++++++++++++++++++++++++++++++++++-----
> > fs/f2fs/f2fs.h | 3 ++
> > fs/f2fs/segment.c | 5 ++-
> > 3 files changed, 86 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 35910ff23582..e4c183e85de8 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -296,20 +296,20 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
> > io->bio = NULL;
> > }
> >
> > -static bool __has_merged_page(struct f2fs_bio_info *io, struct inode *inode,
> > +static bool __has_merged_page(struct bio *bio, struct inode *inode,
> > struct page *page, nid_t ino)
> > {
> > struct bio_vec *bvec;
> > struct page *target;
> > int i;
> >
> > - if (!io->bio)
> > + if (!bio)
> > return false;
> >
> > if (!inode && !page && !ino)
> > return true;
> >
> > - bio_for_each_segment_all(bvec, io->bio, i) {
> > + bio_for_each_segment_all(bvec, bio, i) {
> >
> > if (bvec->bv_page->mapping)
> > target = bvec->bv_page;
> > @@ -360,7 +360,7 @@ static void __submit_merged_write_cond(struct f2fs_sb_info *sbi,
> > struct f2fs_bio_info *io = sbi->write_io[btype] + temp;
> >
> > down_read(&io->io_rwsem);
> > - ret = __has_merged_page(io, inode, page, ino);
> > + ret = __has_merged_page(io->bio, inode, page, ino);
> > up_read(&io->io_rwsem);
> > }
> > if (ret)
> > @@ -429,6 +429,61 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> > return 0;
> > }
> >
> > +int f2fs_merge_page_bio(struct f2fs_io_info *fio)
> > +{
> > + struct bio *bio = *fio->bio;
> > + struct page *page = fio->encrypted_page ?
> > + fio->encrypted_page : fio->page;
> > +
> > + if (!f2fs_is_valid_blkaddr(fio->sbi, fio->new_blkaddr,
> > + __is_meta_io(fio) ? META_GENERIC : DATA_GENERIC))
> > + return -EFAULT;
> > +
> > + trace_f2fs_submit_page_bio(page, fio);
> > + f2fs_trace_ios(fio, 0);
> > +
> > + if (bio && (*fio->last_block + 1 != fio->new_blkaddr ||
> > + !__same_bdev(fio->sbi, fio->new_blkaddr, bio))) {
> > + __submit_bio(fio->sbi, bio, fio->type);
> > + bio = NULL;
> > + }
> > +alloc_new:
> > + if (!bio) {
> > + bio = __bio_alloc(fio->sbi, fio->new_blkaddr, fio->io_wbc,
> > + BIO_MAX_PAGES, false, fio->type, fio->temp);
> > + bio_set_op_attrs(bio, fio->op, fio->op_flags);
> > + }
> > +
> > + if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
> > + __submit_bio(fio->sbi, bio, fio->type);
> > + bio = NULL;
> > + goto alloc_new;
> > + }
> > +
> > + if (fio->io_wbc)
> > + wbc_account_io(fio->io_wbc, page, PAGE_SIZE);
> > +
> > + inc_page_count(fio->sbi, WB_DATA_TYPE(page));
> > +
> > + *fio->last_block = fio->new_blkaddr;
> > + *fio->bio = bio;
> > +
> > + return 0;
> > +}
> > +
> > +void f2fs_submit_ipu_bio(struct f2fs_sb_info *sbi, struct bio **bio,
> > + struct page *page)
> > +{
> > + if (!bio)
> > + return;
> > +
> > + if (!__has_merged_page(*bio, NULL, page, 0))
> > + return;
> > +
> > + __submit_bio(sbi, *bio, DATA);
> > + *bio = NULL;
> > +}
> > +
> > void f2fs_submit_page_write(struct f2fs_io_info *fio)
> > {
> > struct f2fs_sb_info *sbi = fio->sbi;
> > @@ -1854,6 +1909,8 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
> > }
> >
> > static int __write_data_page(struct page *page, bool *submitted,
> > + struct bio **bio,
> > + sector_t *last_block,
> > struct writeback_control *wbc,
> > enum iostat_type io_type)
> > {
> > @@ -1879,6 +1936,8 @@ static int __write_data_page(struct page *page, bool *submitted,
> > .need_lock = LOCK_RETRY,
> > .io_type = io_type,
> > .io_wbc = wbc,
> > + .bio = bio,
> > + .last_block = last_block,
> > };
> >
> > trace_f2fs_writepage(page, DATA);
> > @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
> > }
> >
> > unlock_page(page);
> > - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
> > + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
> > + f2fs_submit_ipu_bio(sbi, bio, page);
> > f2fs_balance_fs(sbi, need_balance_fs);
> > + }
> >
> > if (unlikely(f2fs_cp_error(sbi))) {
> > + f2fs_submit_ipu_bio(sbi, bio, page);
> > f2fs_submit_merged_write(sbi, DATA);
> > submitted = NULL;
> > }
> > @@ -2006,7 +2068,7 @@ static int __write_data_page(struct page *page, bool *submitted,
> > static int f2fs_write_data_page(struct page *page,
> > struct writeback_control *wbc)
> > {
> > - return __write_data_page(page, NULL, wbc, FS_DATA_IO);
> > + return __write_data_page(page, NULL, NULL, NULL, wbc, FS_DATA_IO);
> > }
> >
> > /*
> > @@ -2022,6 +2084,8 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> > int done = 0;
> > struct pagevec pvec;
> > struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
> > + struct bio *bio = NULL;
> > + sector_t last_block;
> > int nr_pages;
> > pgoff_t uninitialized_var(writeback_index);
> > pgoff_t index;
> > @@ -2098,17 +2162,20 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> > }
> >
> > if (PageWriteback(page)) {
> > - if (wbc->sync_mode != WB_SYNC_NONE)
> > + if (wbc->sync_mode != WB_SYNC_NONE) {
> > f2fs_wait_on_page_writeback(page,
> > DATA, true, true);
> > - else
> > + f2fs_submit_ipu_bio(sbi, &bio, page);
> > + } else {
> > goto continue_unlock;
> > + }
> > }
> >
> > if (!clear_page_dirty_for_io(page))
> > goto continue_unlock;
> >
> > - ret = __write_data_page(page, &submitted, wbc, io_type);
> > + ret = __write_data_page(page, &submitted, &bio,
> > + &last_block, wbc, io_type);
> > if (unlikely(ret)) {
> > /*
> > * keep nr_to_write, since vfs uses this to
> > @@ -2157,6 +2224,9 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> > if (nwritten)
> > f2fs_submit_merged_write_cond(F2FS_M_SB(mapping), mapping->host,
> > NULL, 0, DATA);
> > + /* submit cached bio of IPU write */
> > + if (bio)
> > + __submit_bio(sbi, bio, DATA);
> >
> > return ret;
> > }
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 46db3ed87c84..7c4355927d28 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1048,6 +1048,8 @@ struct f2fs_io_info {
> > bool retry; /* need to reallocate block address */
> > enum iostat_type io_type; /* io type */
> > struct writeback_control *io_wbc; /* writeback control */
> > + struct bio **bio; /* bio for ipu */
> > + sector_t *last_block; /* last block number in bio */
> > unsigned char version; /* version of the node */
> > };
> >
> > @@ -3105,6 +3107,7 @@ void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
> > nid_t ino, enum page_type type);
> > void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi);
> > int f2fs_submit_page_bio(struct f2fs_io_info *fio);
> > +int f2fs_merge_page_bio(struct f2fs_io_info *fio);
> > void f2fs_submit_page_write(struct f2fs_io_info *fio);
> > struct block_device *f2fs_target_device(struct f2fs_sb_info *sbi,
> > block_t blk_addr, struct bio *bio);
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index b2167d53fb9d..66838ab2f21e 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -3196,7 +3196,10 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
> >
> > stat_inc_inplace_blocks(fio->sbi);
> >
> > - err = f2fs_submit_page_bio(fio);
> > + if (fio->bio)
> > + err = f2fs_merge_page_bio(fio);
> > + else
> > + err = f2fs_submit_page_bio(fio);
> > if (!err)
> > update_device_state(fio);
> >
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add bio cache for IPU
2019-03-01 17:55 ` Jaegeuk Kim
@ 2019-03-04 6:27 ` Chao Yu
2019-05-14 4:55 ` [f2fs-dev] " Ju Hyung Park
1 sibling, 0 replies; 10+ messages in thread
From: Chao Yu @ 2019-03-04 6:27 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao
On 2019/3/2 1:55, Jaegeuk Kim wrote:
> On 02/28, Chao Yu wrote:
>> Ping,
>
> Ditto.
Copied.
Thanks,
>
>>
>> On 2019/2/19 16:15, Chao Yu wrote:
>>> SQLite in Wal mode may trigger sequential IPU write in db-wal file, after
>>> commit d1b3e72d5490 ("f2fs: submit bio of in-place-update pages"), we
>>> lost the chance of merging page in inner managed bio cache, result in
>>> submitting more small-sized IO.
>>>
>>> So let's add temporary bio in writepages() to cache mergeable write IO as
>>> much as possible.
>>>
>>> Test case:
>>> 1. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
>>> 2. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
>>>
>>> Before:
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65552, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65560, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65568, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65576, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65584, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65592, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65600, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65608, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65616, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65624, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65632, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65640, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65648, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65656, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65664, size = 4096
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57352, size = 4096
>>>
>>> After:
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 65536
>>> f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57368, size = 4096
>>>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>> ---
>>> v4:
>>> - fix to set *bio to NULL in f2fs_submit_ipu_bio()
>>> - spread f2fs_submit_ipu_bio()
>>> - fix to count dirty encrypted page correctly in f2fs_merge_page_bio()
>>> - remove redundant assignment of fio->last_block
>>> fs/f2fs/data.c | 88 ++++++++++++++++++++++++++++++++++++++++++-----
>>> fs/f2fs/f2fs.h | 3 ++
>>> fs/f2fs/segment.c | 5 ++-
>>> 3 files changed, 86 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 35910ff23582..e4c183e85de8 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -296,20 +296,20 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
>>> io->bio = NULL;
>>> }
>>>
>>> -static bool __has_merged_page(struct f2fs_bio_info *io, struct inode *inode,
>>> +static bool __has_merged_page(struct bio *bio, struct inode *inode,
>>> struct page *page, nid_t ino)
>>> {
>>> struct bio_vec *bvec;
>>> struct page *target;
>>> int i;
>>>
>>> - if (!io->bio)
>>> + if (!bio)
>>> return false;
>>>
>>> if (!inode && !page && !ino)
>>> return true;
>>>
>>> - bio_for_each_segment_all(bvec, io->bio, i) {
>>> + bio_for_each_segment_all(bvec, bio, i) {
>>>
>>> if (bvec->bv_page->mapping)
>>> target = bvec->bv_page;
>>> @@ -360,7 +360,7 @@ static void __submit_merged_write_cond(struct f2fs_sb_info *sbi,
>>> struct f2fs_bio_info *io = sbi->write_io[btype] + temp;
>>>
>>> down_read(&io->io_rwsem);
>>> - ret = __has_merged_page(io, inode, page, ino);
>>> + ret = __has_merged_page(io->bio, inode, page, ino);
>>> up_read(&io->io_rwsem);
>>> }
>>> if (ret)
>>> @@ -429,6 +429,61 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
>>> return 0;
>>> }
>>>
>>> +int f2fs_merge_page_bio(struct f2fs_io_info *fio)
>>> +{
>>> + struct bio *bio = *fio->bio;
>>> + struct page *page = fio->encrypted_page ?
>>> + fio->encrypted_page : fio->page;
>>> +
>>> + if (!f2fs_is_valid_blkaddr(fio->sbi, fio->new_blkaddr,
>>> + __is_meta_io(fio) ? META_GENERIC : DATA_GENERIC))
>>> + return -EFAULT;
>>> +
>>> + trace_f2fs_submit_page_bio(page, fio);
>>> + f2fs_trace_ios(fio, 0);
>>> +
>>> + if (bio && (*fio->last_block + 1 != fio->new_blkaddr ||
>>> + !__same_bdev(fio->sbi, fio->new_blkaddr, bio))) {
>>> + __submit_bio(fio->sbi, bio, fio->type);
>>> + bio = NULL;
>>> + }
>>> +alloc_new:
>>> + if (!bio) {
>>> + bio = __bio_alloc(fio->sbi, fio->new_blkaddr, fio->io_wbc,
>>> + BIO_MAX_PAGES, false, fio->type, fio->temp);
>>> + bio_set_op_attrs(bio, fio->op, fio->op_flags);
>>> + }
>>> +
>>> + if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
>>> + __submit_bio(fio->sbi, bio, fio->type);
>>> + bio = NULL;
>>> + goto alloc_new;
>>> + }
>>> +
>>> + if (fio->io_wbc)
>>> + wbc_account_io(fio->io_wbc, page, PAGE_SIZE);
>>> +
>>> + inc_page_count(fio->sbi, WB_DATA_TYPE(page));
>>> +
>>> + *fio->last_block = fio->new_blkaddr;
>>> + *fio->bio = bio;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void f2fs_submit_ipu_bio(struct f2fs_sb_info *sbi, struct bio **bio,
>>> + struct page *page)
>>> +{
>>> + if (!bio)
>>> + return;
>>> +
>>> + if (!__has_merged_page(*bio, NULL, page, 0))
>>> + return;
>>> +
>>> + __submit_bio(sbi, *bio, DATA);
>>> + *bio = NULL;
>>> +}
>>> +
>>> void f2fs_submit_page_write(struct f2fs_io_info *fio)
>>> {
>>> struct f2fs_sb_info *sbi = fio->sbi;
>>> @@ -1854,6 +1909,8 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>>> }
>>>
>>> static int __write_data_page(struct page *page, bool *submitted,
>>> + struct bio **bio,
>>> + sector_t *last_block,
>>> struct writeback_control *wbc,
>>> enum iostat_type io_type)
>>> {
>>> @@ -1879,6 +1936,8 @@ static int __write_data_page(struct page *page, bool *submitted,
>>> .need_lock = LOCK_RETRY,
>>> .io_type = io_type,
>>> .io_wbc = wbc,
>>> + .bio = bio,
>>> + .last_block = last_block,
>>> };
>>>
>>> trace_f2fs_writepage(page, DATA);
>>> @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
>>> }
>>>
>>> unlock_page(page);
>>> - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
>>> + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
>>> + f2fs_submit_ipu_bio(sbi, bio, page);
>>> f2fs_balance_fs(sbi, need_balance_fs);
>>> + }
>>>
>>> if (unlikely(f2fs_cp_error(sbi))) {
>>> + f2fs_submit_ipu_bio(sbi, bio, page);
>>> f2fs_submit_merged_write(sbi, DATA);
>>> submitted = NULL;
>>> }
>>> @@ -2006,7 +2068,7 @@ static int __write_data_page(struct page *page, bool *submitted,
>>> static int f2fs_write_data_page(struct page *page,
>>> struct writeback_control *wbc)
>>> {
>>> - return __write_data_page(page, NULL, wbc, FS_DATA_IO);
>>> + return __write_data_page(page, NULL, NULL, NULL, wbc, FS_DATA_IO);
>>> }
>>>
>>> /*
>>> @@ -2022,6 +2084,8 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
>>> int done = 0;
>>> struct pagevec pvec;
>>> struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
>>> + struct bio *bio = NULL;
>>> + sector_t last_block;
>>> int nr_pages;
>>> pgoff_t uninitialized_var(writeback_index);
>>> pgoff_t index;
>>> @@ -2098,17 +2162,20 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
>>> }
>>>
>>> if (PageWriteback(page)) {
>>> - if (wbc->sync_mode != WB_SYNC_NONE)
>>> + if (wbc->sync_mode != WB_SYNC_NONE) {
>>> f2fs_wait_on_page_writeback(page,
>>> DATA, true, true);
>>> - else
>>> + f2fs_submit_ipu_bio(sbi, &bio, page);
>>> + } else {
>>> goto continue_unlock;
>>> + }
>>> }
>>>
>>> if (!clear_page_dirty_for_io(page))
>>> goto continue_unlock;
>>>
>>> - ret = __write_data_page(page, &submitted, wbc, io_type);
>>> + ret = __write_data_page(page, &submitted, &bio,
>>> + &last_block, wbc, io_type);
>>> if (unlikely(ret)) {
>>> /*
>>> * keep nr_to_write, since vfs uses this to
>>> @@ -2157,6 +2224,9 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
>>> if (nwritten)
>>> f2fs_submit_merged_write_cond(F2FS_M_SB(mapping), mapping->host,
>>> NULL, 0, DATA);
>>> + /* submit cached bio of IPU write */
>>> + if (bio)
>>> + __submit_bio(sbi, bio, DATA);
>>>
>>> return ret;
>>> }
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 46db3ed87c84..7c4355927d28 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1048,6 +1048,8 @@ struct f2fs_io_info {
>>> bool retry; /* need to reallocate block address */
>>> enum iostat_type io_type; /* io type */
>>> struct writeback_control *io_wbc; /* writeback control */
>>> + struct bio **bio; /* bio for ipu */
>>> + sector_t *last_block; /* last block number in bio */
>>> unsigned char version; /* version of the node */
>>> };
>>>
>>> @@ -3105,6 +3107,7 @@ void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
>>> nid_t ino, enum page_type type);
>>> void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi);
>>> int f2fs_submit_page_bio(struct f2fs_io_info *fio);
>>> +int f2fs_merge_page_bio(struct f2fs_io_info *fio);
>>> void f2fs_submit_page_write(struct f2fs_io_info *fio);
>>> struct block_device *f2fs_target_device(struct f2fs_sb_info *sbi,
>>> block_t blk_addr, struct bio *bio);
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index b2167d53fb9d..66838ab2f21e 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -3196,7 +3196,10 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
>>>
>>> stat_inc_inplace_blocks(fio->sbi);
>>>
>>> - err = f2fs_submit_page_bio(fio);
>>> + if (fio->bio)
>>> + err = f2fs_merge_page_bio(fio);
>>> + else
>>> + err = f2fs_submit_page_bio(fio);
>>> if (!err)
>>> update_device_state(fio);
>>>
>>>
>
> .
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v4] f2fs: add bio cache for IPU
2019-03-01 17:55 ` Jaegeuk Kim
2019-03-04 6:27 ` Chao Yu
@ 2019-05-14 4:55 ` Ju Hyung Park
1 sibling, 0 replies; 10+ messages in thread
From: Ju Hyung Park @ 2019-05-14 4:55 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: Chao Yu, linux-kernel, linux-f2fs-devel
Hi,
Is this still causing hangs?
Just wondering why this isn't merged yet.
Thanks.
On Sat, Mar 2, 2019 at 2:55 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> On 02/28, Chao Yu wrote:
> > Ping,
>
> Ditto.
>
> >
> > On 2019/2/19 16:15, Chao Yu wrote:
> > > SQLite in Wal mode may trigger sequential IPU write in db-wal file, after
> > > commit d1b3e72d5490 ("f2fs: submit bio of in-place-update pages"), we
> > > lost the chance of merging page in inner managed bio cache, result in
> > > submitting more small-sized IO.
> > >
> > > So let's add temporary bio in writepages() to cache mergeable write IO as
> > > much as possible.
> > >
> > > Test case:
> > > 1. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
> > > 2. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
> > >
> > > Before:
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65552, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65560, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65568, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65576, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65584, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65592, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65600, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65608, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65616, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65624, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65632, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65640, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65648, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65656, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65664, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57352, size = 4096
> > >
> > > After:
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector = 65544, size = 65536
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector = 57368, size = 4096
> > >
> > > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > > ---
> > > v4:
> > > - fix to set *bio to NULL in f2fs_submit_ipu_bio()
> > > - spread f2fs_submit_ipu_bio()
> > > - fix to count dirty encrypted page correctly in f2fs_merge_page_bio()
> > > - remove redundant assignment of fio->last_block
> > > fs/f2fs/data.c | 88 ++++++++++++++++++++++++++++++++++++++++++-----
> > > fs/f2fs/f2fs.h | 3 ++
> > > fs/f2fs/segment.c | 5 ++-
> > > 3 files changed, 86 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > index 35910ff23582..e4c183e85de8 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -296,20 +296,20 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
> > > io->bio = NULL;
> > > }
> > >
> > > -static bool __has_merged_page(struct f2fs_bio_info *io, struct inode *inode,
> > > +static bool __has_merged_page(struct bio *bio, struct inode *inode,
> > > struct page *page, nid_t ino)
> > > {
> > > struct bio_vec *bvec;
> > > struct page *target;
> > > int i;
> > >
> > > - if (!io->bio)
> > > + if (!bio)
> > > return false;
> > >
> > > if (!inode && !page && !ino)
> > > return true;
> > >
> > > - bio_for_each_segment_all(bvec, io->bio, i) {
> > > + bio_for_each_segment_all(bvec, bio, i) {
> > >
> > > if (bvec->bv_page->mapping)
> > > target = bvec->bv_page;
> > > @@ -360,7 +360,7 @@ static void __submit_merged_write_cond(struct f2fs_sb_info *sbi,
> > > struct f2fs_bio_info *io = sbi->write_io[btype] + temp;
> > >
> > > down_read(&io->io_rwsem);
> > > - ret = __has_merged_page(io, inode, page, ino);
> > > + ret = __has_merged_page(io->bio, inode, page, ino);
> > > up_read(&io->io_rwsem);
> > > }
> > > if (ret)
> > > @@ -429,6 +429,61 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> > > return 0;
> > > }
> > >
> > > +int f2fs_merge_page_bio(struct f2fs_io_info *fio)
> > > +{
> > > + struct bio *bio = *fio->bio;
> > > + struct page *page = fio->encrypted_page ?
> > > + fio->encrypted_page : fio->page;
> > > +
> > > + if (!f2fs_is_valid_blkaddr(fio->sbi, fio->new_blkaddr,
> > > + __is_meta_io(fio) ? META_GENERIC : DATA_GENERIC))
> > > + return -EFAULT;
> > > +
> > > + trace_f2fs_submit_page_bio(page, fio);
> > > + f2fs_trace_ios(fio, 0);
> > > +
> > > + if (bio && (*fio->last_block + 1 != fio->new_blkaddr ||
> > > + !__same_bdev(fio->sbi, fio->new_blkaddr, bio))) {
> > > + __submit_bio(fio->sbi, bio, fio->type);
> > > + bio = NULL;
> > > + }
> > > +alloc_new:
> > > + if (!bio) {
> > > + bio = __bio_alloc(fio->sbi, fio->new_blkaddr, fio->io_wbc,
> > > + BIO_MAX_PAGES, false, fio->type, fio->temp);
> > > + bio_set_op_attrs(bio, fio->op, fio->op_flags);
> > > + }
> > > +
> > > + if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
> > > + __submit_bio(fio->sbi, bio, fio->type);
> > > + bio = NULL;
> > > + goto alloc_new;
> > > + }
> > > +
> > > + if (fio->io_wbc)
> > > + wbc_account_io(fio->io_wbc, page, PAGE_SIZE);
> > > +
> > > + inc_page_count(fio->sbi, WB_DATA_TYPE(page));
> > > +
> > > + *fio->last_block = fio->new_blkaddr;
> > > + *fio->bio = bio;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +void f2fs_submit_ipu_bio(struct f2fs_sb_info *sbi, struct bio **bio,
> > > + struct page *page)
> > > +{
> > > + if (!bio)
> > > + return;
> > > +
> > > + if (!__has_merged_page(*bio, NULL, page, 0))
> > > + return;
> > > +
> > > + __submit_bio(sbi, *bio, DATA);
> > > + *bio = NULL;
> > > +}
> > > +
> > > void f2fs_submit_page_write(struct f2fs_io_info *fio)
> > > {
> > > struct f2fs_sb_info *sbi = fio->sbi;
> > > @@ -1854,6 +1909,8 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
> > > }
> > >
> > > static int __write_data_page(struct page *page, bool *submitted,
> > > + struct bio **bio,
> > > + sector_t *last_block,
> > > struct writeback_control *wbc,
> > > enum iostat_type io_type)
> > > {
> > > @@ -1879,6 +1936,8 @@ static int __write_data_page(struct page *page, bool *submitted,
> > > .need_lock = LOCK_RETRY,
> > > .io_type = io_type,
> > > .io_wbc = wbc,
> > > + .bio = bio,
> > > + .last_block = last_block,
> > > };
> > >
> > > trace_f2fs_writepage(page, DATA);
> > > @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
> > > }
> > >
> > > unlock_page(page);
> > > - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
> > > + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
> > > + f2fs_submit_ipu_bio(sbi, bio, page);
> > > f2fs_balance_fs(sbi, need_balance_fs);
> > > + }
> > >
> > > if (unlikely(f2fs_cp_error(sbi))) {
> > > + f2fs_submit_ipu_bio(sbi, bio, page);
> > > f2fs_submit_merged_write(sbi, DATA);
> > > submitted = NULL;
> > > }
> > > @@ -2006,7 +2068,7 @@ static int __write_data_page(struct page *page, bool *submitted,
> > > static int f2fs_write_data_page(struct page *page,
> > > struct writeback_control *wbc)
> > > {
> > > - return __write_data_page(page, NULL, wbc, FS_DATA_IO);
> > > + return __write_data_page(page, NULL, NULL, NULL, wbc, FS_DATA_IO);
> > > }
> > >
> > > /*
> > > @@ -2022,6 +2084,8 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> > > int done = 0;
> > > struct pagevec pvec;
> > > struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
> > > + struct bio *bio = NULL;
> > > + sector_t last_block;
> > > int nr_pages;
> > > pgoff_t uninitialized_var(writeback_index);
> > > pgoff_t index;
> > > @@ -2098,17 +2162,20 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> > > }
> > >
> > > if (PageWriteback(page)) {
> > > - if (wbc->sync_mode != WB_SYNC_NONE)
> > > + if (wbc->sync_mode != WB_SYNC_NONE) {
> > > f2fs_wait_on_page_writeback(page,
> > > DATA, true, true);
> > > - else
> > > + f2fs_submit_ipu_bio(sbi, &bio, page);
> > > + } else {
> > > goto continue_unlock;
> > > + }
> > > }
> > >
> > > if (!clear_page_dirty_for_io(page))
> > > goto continue_unlock;
> > >
> > > - ret = __write_data_page(page, &submitted, wbc, io_type);
> > > + ret = __write_data_page(page, &submitted, &bio,
> > > + &last_block, wbc, io_type);
> > > if (unlikely(ret)) {
> > > /*
> > > * keep nr_to_write, since vfs uses this to
> > > @@ -2157,6 +2224,9 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> > > if (nwritten)
> > > f2fs_submit_merged_write_cond(F2FS_M_SB(mapping), mapping->host,
> > > NULL, 0, DATA);
> > > + /* submit cached bio of IPU write */
> > > + if (bio)
> > > + __submit_bio(sbi, bio, DATA);
> > >
> > > return ret;
> > > }
> > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > index 46db3ed87c84..7c4355927d28 100644
> > > --- a/fs/f2fs/f2fs.h
> > > +++ b/fs/f2fs/f2fs.h
> > > @@ -1048,6 +1048,8 @@ struct f2fs_io_info {
> > > bool retry; /* need to reallocate block address */
> > > enum iostat_type io_type; /* io type */
> > > struct writeback_control *io_wbc; /* writeback control */
> > > + struct bio **bio; /* bio for ipu */
> > > + sector_t *last_block; /* last block number in bio */
> > > unsigned char version; /* version of the node */
> > > };
> > >
> > > @@ -3105,6 +3107,7 @@ void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
> > > nid_t ino, enum page_type type);
> > > void f2fs_flush_merged_writes(struct f2fs_sb_info *sbi);
> > > int f2fs_submit_page_bio(struct f2fs_io_info *fio);
> > > +int f2fs_merge_page_bio(struct f2fs_io_info *fio);
> > > void f2fs_submit_page_write(struct f2fs_io_info *fio);
> > > struct block_device *f2fs_target_device(struct f2fs_sb_info *sbi,
> > > block_t blk_addr, struct bio *bio);
> > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > > index b2167d53fb9d..66838ab2f21e 100644
> > > --- a/fs/f2fs/segment.c
> > > +++ b/fs/f2fs/segment.c
> > > @@ -3196,7 +3196,10 @@ int f2fs_inplace_write_data(struct f2fs_io_info *fio)
> > >
> > > stat_inc_inplace_blocks(fio->sbi);
> > >
> > > - err = f2fs_submit_page_bio(fio);
> > > + if (fio->bio)
> > > + err = f2fs_merge_page_bio(fio);
> > > + else
> > > + err = f2fs_submit_page_bio(fio);
> > > if (!err)
> > > update_device_state(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] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v4] f2fs: add bio cache for IPU
2019-02-19 8:15 [PATCH v4] f2fs: add bio cache for IPU Chao Yu
2019-02-28 3:49 ` Chao Yu
@ 2019-08-31 7:23 ` Chao Yu
2019-09-01 7:17 ` Jaegeuk Kim
1 sibling, 1 reply; 10+ messages in thread
From: Chao Yu @ 2019-08-31 7:23 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel
On 2019/2/19 16:15, Chao Yu wrote:
> @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
> }
>
> unlock_page(page);
> - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
> + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
> + f2fs_submit_ipu_bio(sbi, bio, page);
> f2fs_balance_fs(sbi, need_balance_fs);
> + }
Above bio submission was added to avoid below deadlock:
- __write_data_page
- f2fs_do_write_data_page
- set_page_writeback ---- set writeback flag
- f2fs_inplace_write_data
- f2fs_balance_fs
- f2fs_gc
- do_garbage_collect
- gc_data_segment
- move_data_page
- f2fs_wait_on_page_writeback
- wait_on_page_writeback --- wait writeback
However, it breaks the merge of IPU IOs, to solve this issue, it looks we need
to add global bio cache for such IPU merge case, then later
f2fs_wait_on_page_writeback can check whether writebacked page is cached or not,
and do the submission if necessary.
Jaegeuk, any thoughts?
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] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v4] f2fs: add bio cache for IPU
2019-08-31 7:23 ` Chao Yu
@ 2019-09-01 7:17 ` Jaegeuk Kim
2019-09-02 1:16 ` Chao Yu
0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2019-09-01 7:17 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 08/31, Chao Yu wrote:
> On 2019/2/19 16:15, Chao Yu wrote:
> > @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
> > }
> >
> > unlock_page(page);
> > - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
> > + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
> > + f2fs_submit_ipu_bio(sbi, bio, page);
> > f2fs_balance_fs(sbi, need_balance_fs);
> > + }
>
> Above bio submission was added to avoid below deadlock:
>
> - __write_data_page
> - f2fs_do_write_data_page
> - set_page_writeback ---- set writeback flag
> - f2fs_inplace_write_data
> - f2fs_balance_fs
> - f2fs_gc
> - do_garbage_collect
> - gc_data_segment
> - move_data_page
> - f2fs_wait_on_page_writeback
> - wait_on_page_writeback --- wait writeback
>
> However, it breaks the merge of IPU IOs, to solve this issue, it looks we need
> to add global bio cache for such IPU merge case, then later
> f2fs_wait_on_page_writeback can check whether writebacked page is cached or not,
> and do the submission if necessary.
>
> Jaegeuk, any thoughts?
How about calling f2fs_submit_ipu_bio() when we need to do GC in the same
context?
>
> 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] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v4] f2fs: add bio cache for IPU
2019-09-01 7:17 ` Jaegeuk Kim
@ 2019-09-02 1:16 ` Chao Yu
2019-09-02 23:04 ` Jaegeuk Kim
0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2019-09-02 1:16 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel
On 2019/9/1 15:17, Jaegeuk Kim wrote:
> On 08/31, Chao Yu wrote:
>> On 2019/2/19 16:15, Chao Yu wrote:
>>> @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
>>> }
>>>
>>> unlock_page(page);
>>> - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
>>> + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
>>> + f2fs_submit_ipu_bio(sbi, bio, page);
>>> f2fs_balance_fs(sbi, need_balance_fs);
>>> + }
>>
>> Above bio submission was added to avoid below deadlock:
>>
>> - __write_data_page
>> - f2fs_do_write_data_page
>> - set_page_writeback ---- set writeback flag
>> - f2fs_inplace_write_data
>> - f2fs_balance_fs
>> - f2fs_gc
>> - do_garbage_collect
>> - gc_data_segment
>> - move_data_page
>> - f2fs_wait_on_page_writeback
>> - wait_on_page_writeback --- wait writeback
>>
>> However, it breaks the merge of IPU IOs, to solve this issue, it looks we need
>> to add global bio cache for such IPU merge case, then later
>> f2fs_wait_on_page_writeback can check whether writebacked page is cached or not,
>> and do the submission if necessary.
>>
>> Jaegeuk, any thoughts?
>
> How about calling f2fs_submit_ipu_bio() when we need to do GC in the same
> context?
However it also could happen in race case:
Thread A Thread B
- __write_data_page (inode x, page y)
- f2fs_do_write_data_page
- set_page_writeback ---- set writeback flag in page y
- f2fs_inplace_write_data
- f2fs_balance_fs
- lock gc_mutex
- lock gc_mutex
- f2fs_gc
- do_garbage_collect
- gc_data_segment
- move_data_page
- f2fs_wait_on_page_writeback
- wait_on_page_writeback --- wait writeback of page y
So it needs a global bio cache for merged IPU pages, how about adding a list to
link all ipu bios in struct f2fs_bio_info?
struct f2fs_bio_info {
....
struct list_head ipu_bio_list; /* track all ipu bio */
spinlock_t ipu_bio_lock; /* protect ipu bio list */
}
>
>>
>> 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] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v4] f2fs: add bio cache for IPU
2019-09-02 1:16 ` Chao Yu
@ 2019-09-02 23:04 ` Jaegeuk Kim
2019-09-02 23:34 ` Chao Yu
0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2019-09-02 23:04 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 09/02, Chao Yu wrote:
> On 2019/9/1 15:17, Jaegeuk Kim wrote:
> > On 08/31, Chao Yu wrote:
> >> On 2019/2/19 16:15, Chao Yu wrote:
> >>> @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
> >>> }
> >>>
> >>> unlock_page(page);
> >>> - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
> >>> + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
> >>> + f2fs_submit_ipu_bio(sbi, bio, page);
> >>> f2fs_balance_fs(sbi, need_balance_fs);
> >>> + }
> >>
> >> Above bio submission was added to avoid below deadlock:
> >>
> >> - __write_data_page
> >> - f2fs_do_write_data_page
> >> - set_page_writeback ---- set writeback flag
> >> - f2fs_inplace_write_data
> >> - f2fs_balance_fs
> >> - f2fs_gc
> >> - do_garbage_collect
> >> - gc_data_segment
> >> - move_data_page
> >> - f2fs_wait_on_page_writeback
> >> - wait_on_page_writeback --- wait writeback
> >>
> >> However, it breaks the merge of IPU IOs, to solve this issue, it looks we need
> >> to add global bio cache for such IPU merge case, then later
> >> f2fs_wait_on_page_writeback can check whether writebacked page is cached or not,
> >> and do the submission if necessary.
> >>
> >> Jaegeuk, any thoughts?
> >
> > How about calling f2fs_submit_ipu_bio() when we need to do GC in the same
> > context?
>
> However it also could happen in race case:
>
> Thread A Thread B
> - __write_data_page (inode x, page y)
> - f2fs_do_write_data_page
> - set_page_writeback ---- set writeback flag in page y
> - f2fs_inplace_write_data
> - f2fs_balance_fs
> - lock gc_mutex
> - lock gc_mutex
> - f2fs_gc
> - do_garbage_collect
> - gc_data_segment
> - move_data_page
> - f2fs_wait_on_page_writeback
> - wait_on_page_writeback --- wait writeback of page y
>
> So it needs a global bio cache for merged IPU pages, how about adding a list to
> link all ipu bios in struct f2fs_bio_info?
Hmm, I can't think of better solution than adding a list. In this case, blk_plug
doesn't work well?
>
> struct f2fs_bio_info {
> ....
> struct list_head ipu_bio_list; /* track all ipu bio */
> spinlock_t ipu_bio_lock; /* protect ipu bio list */
> }
>
> >
> >>
> >> 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] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v4] f2fs: add bio cache for IPU
2019-09-02 23:04 ` Jaegeuk Kim
@ 2019-09-02 23:34 ` Chao Yu
0 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2019-09-02 23:34 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel
On 2019-9-3 7:04, Jaegeuk Kim wrote:
> On 09/02, Chao Yu wrote:
>> On 2019/9/1 15:17, Jaegeuk Kim wrote:
>>> On 08/31, Chao Yu wrote:
>>>> On 2019/2/19 16:15, Chao Yu wrote:
>>>>> @@ -1976,10 +2035,13 @@ static int __write_data_page(struct page *page, bool *submitted,
>>>>> }
>>>>>
>>>>> unlock_page(page);
>>>>> - if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode))
>>>>> + if (!S_ISDIR(inode->i_mode) && !IS_NOQUOTA(inode)) {
>>>>> + f2fs_submit_ipu_bio(sbi, bio, page);
>>>>> f2fs_balance_fs(sbi, need_balance_fs);
>>>>> + }
>>>>
>>>> Above bio submission was added to avoid below deadlock:
>>>>
>>>> - __write_data_page
>>>> - f2fs_do_write_data_page
>>>> - set_page_writeback ---- set writeback flag
>>>> - f2fs_inplace_write_data
>>>> - f2fs_balance_fs
>>>> - f2fs_gc
>>>> - do_garbage_collect
>>>> - gc_data_segment
>>>> - move_data_page
>>>> - f2fs_wait_on_page_writeback
>>>> - wait_on_page_writeback --- wait writeback
>>>>
>>>> However, it breaks the merge of IPU IOs, to solve this issue, it looks we need
>>>> to add global bio cache for such IPU merge case, then later
>>>> f2fs_wait_on_page_writeback can check whether writebacked page is cached or not,
>>>> and do the submission if necessary.
>>>>
>>>> Jaegeuk, any thoughts?
>>>
>>> How about calling f2fs_submit_ipu_bio() when we need to do GC in the same
>>> context?
>>
>> However it also could happen in race case:
>>
>> Thread A Thread B
>> - __write_data_page (inode x, page y)
>> - f2fs_do_write_data_page
>> - set_page_writeback ---- set writeback flag in page y
>> - f2fs_inplace_write_data
>> - f2fs_balance_fs
>> - lock gc_mutex
>> - lock gc_mutex
>> - f2fs_gc
>> - do_garbage_collect
>> - gc_data_segment
>> - move_data_page
>> - f2fs_wait_on_page_writeback
>> - wait_on_page_writeback --- wait writeback of page y
>>
>> So it needs a global bio cache for merged IPU pages, how about adding a list to
>> link all ipu bios in struct f2fs_bio_info?
>
> Hmm, I can't think of better solution than adding a list. In this case, blk_plug
> doesn't work well?
Only submitted bio will be taken care of plug, for our case, the bio is still
pending though, I guess plug won't help.
Thanks,
>
>>
>> struct f2fs_bio_info {
>> ....
>> struct list_head ipu_bio_list; /* track all ipu bio */
>> spinlock_t ipu_bio_lock; /* protect ipu bio list */
>> }
>>
>>>
>>>>
>>>> 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] 10+ messages in thread
end of thread, other threads:[~2019-09-02 23:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-19 8:15 [PATCH v4] f2fs: add bio cache for IPU Chao Yu
2019-02-28 3:49 ` Chao Yu
2019-03-01 17:55 ` Jaegeuk Kim
2019-03-04 6:27 ` Chao Yu
2019-05-14 4:55 ` [f2fs-dev] " Ju Hyung Park
2019-08-31 7:23 ` Chao Yu
2019-09-01 7:17 ` Jaegeuk Kim
2019-09-02 1:16 ` Chao Yu
2019-09-02 23:04 ` Jaegeuk Kim
2019-09-02 23:34 ` 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).