Re: [PATCH v3] f2fs: add bio cache for IPU

2018-12-26 Thread Jaegeuk Kim 
On 12/24, Chao Yu wrote:
> Jaegeuk,
> 
> Will kernel still hang with this v3?

I'll consider this later, since it blocked local tests before.

> 
> Thanks,
> 
> On 2018/12/19 17:29, 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 
> > ---
> > v3:
> > - introduce f2fs_submit_ipu_bio() to check page Writeback status.
> >  fs/f2fs/data.c| 85 ++-
> >  fs/f2fs/f2fs.h|  3 ++
> >  fs/f2fs/segment.c |  5 ++-
> >  3 files changed, 84 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index cdfe9a7b856e..e5cd3fd9e215 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -341,20 +341,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;
> > @@ -405,7 +405,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_rwsem);
> > -   ret = __has_merged_page(io, inode, page, ino);
> > +   ret = __has_merged_page(io->bio, inode, page, ino);
> > up_read(>io_rwsem);
> > }
> > if (ret)
> > @@ -474,6 +474,59 @@ 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);
> > +

Re: [PATCH v3] f2fs: add bio cache for IPU

2018-12-23 Thread Chao Yu 
Jaegeuk,

Will kernel still hang with this v3?

Thanks,

On 2018/12/19 17:29, 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 
> ---
> v3:
> - introduce f2fs_submit_ipu_bio() to check page Writeback status.
>  fs/f2fs/data.c| 85 ++-
>  fs/f2fs/f2fs.h|  3 ++
>  fs/f2fs/segment.c |  5 ++-
>  3 files changed, 84 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index cdfe9a7b856e..e5cd3fd9e215 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -341,20 +341,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;
> @@ -405,7 +405,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_rwsem);
> - ret = __has_merged_page(io, inode, page, ino);
> + ret = __has_merged_page(io->bio, inode, page, ino);
>   up_read(>io_rwsem);
>   }
>   if (ret)
> @@ -474,6 +474,59 @@ 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) {

[PATCH v3] f2fs: add bio cache for IPU

2018-12-19 Thread 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 
---
v3:
- introduce f2fs_submit_ipu_bio() to check page Writeback status.
 fs/f2fs/data.c| 85 ++-
 fs/f2fs/f2fs.h|  3 ++
 fs/f2fs/segment.c |  5 ++-
 3 files changed, 84 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cdfe9a7b856e..e5cd3fd9e215 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -341,20 +341,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;
@@ -405,7 +405,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_rwsem);
-   ret = __has_merged_page(io, inode, page, ino);
+   ret = __has_merged_page(io->bio, inode, page, ino);
up_read(>io_rwsem);
}
if (ret)
@@ -474,6 +474,59 @@ 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);
+   *fio->last_block = fio->new_blkaddr;
+   bio_set_op_attrs(bio, fio->op, fio->op_flags);
+