[RFC PATCH 15/17] fs: use helper bio_add_page() instead of open coding on bi_io_vec

[RFC PATCH 15/17] fs: use helper bio_add_page() instead of open coding on bi_io_vec

[Dongsu Park]

From: Kent Overstreet <kmo@daterainc.com>

Call pre-defined helper bio_add_page() instead of open coding for
iterating through bi_io_vec[]. Doing that, it's possible to make some
parts in filesystems and mm/page_io.c simpler than before.

Signed-off-by: Kent Overstreet <kmo@daterainc.com>
[dpark: add more description in commit message]
Signed-off-by: Dongsu Park <dongsu.park@profitbricks.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Dave Kleikamp <shaggy@kernel.org>
Cc: linux-fsdevel@vger.kernel.org
---
 fs/buffer.c         |  7 ++-----
 fs/jfs/jfs_logmgr.c | 14 ++++----------
 mm/page_io.c        |  8 +++-----
 3 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 20805db..35ac0ec 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -3022,12 +3022,9 @@ int _submit_bh(int rw, struct buffer_head *bh, unsigned long bio_flags)
 
 	bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 	bio->bi_bdev = bh->b_bdev;
-	bio->bi_io_vec[0].bv_page = bh->b_page;
-	bio->bi_io_vec[0].bv_len = bh->b_size;
-	bio->bi_io_vec[0].bv_offset = bh_offset(bh);
 
-	bio->bi_vcnt = 1;
-	bio->bi_iter.bi_size = bh->b_size;
+	bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh));
+	BUG_ON(bio->bi_iter.bi_size != bh->b_size);
 
 	bio->bi_end_io = end_bio_bh_io_sync;
 	bio->bi_private = bh;
diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index bc462dc..46fae06 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -1999,12 +1999,9 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp)
 
 	bio->bi_iter.bi_sector = bp->l_blkno << (log->l2bsize - 9);
 	bio->bi_bdev = log->bdev;
-	bio->bi_io_vec[0].bv_page = bp->l_page;
-	bio->bi_io_vec[0].bv_len = LOGPSIZE;
-	bio->bi_io_vec[0].bv_offset = bp->l_offset;
 
-	bio->bi_vcnt = 1;
-	bio->bi_iter.bi_size = LOGPSIZE;
+	bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset);
+	BUG_ON(bio->bi_iter.bi_size != LOGPSIZE);
 
 	bio->bi_end_io = lbmIODone;
 	bio->bi_private = bp;
@@ -2145,12 +2142,9 @@ static void lbmStartIO(struct lbuf * bp)
 	bio = bio_alloc(GFP_NOFS, 1);
 	bio->bi_iter.bi_sector = bp->l_blkno << (log->l2bsize - 9);
 	bio->bi_bdev = log->bdev;
-	bio->bi_io_vec[0].bv_page = bp->l_page;
-	bio->bi_io_vec[0].bv_len = LOGPSIZE;
-	bio->bi_io_vec[0].bv_offset = bp->l_offset;
 
-	bio->bi_vcnt = 1;
-	bio->bi_iter.bi_size = LOGPSIZE;
+	bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset);
+	BUG_ON(bio->bi_iter.bi_size != LOGPSIZE);
 
 	bio->bi_end_io = lbmIODone;
 	bio->bi_private = bp;
diff --git a/mm/page_io.c b/mm/page_io.c
index 955db8b..8c878c7 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -33,12 +33,10 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
 	if (bio) {
 		bio->bi_iter.bi_sector = map_swap_page(page, &bio->bi_bdev);
 		bio->bi_iter.bi_sector <<= PAGE_SHIFT - 9;
-		bio->bi_io_vec[0].bv_page = page;
-		bio->bi_io_vec[0].bv_len = PAGE_SIZE;
-		bio->bi_io_vec[0].bv_offset = 0;
-		bio->bi_vcnt = 1;
-		bio->bi_iter.bi_size = PAGE_SIZE;
 		bio->bi_end_io = end_io;
+
+		bio_add_page(bio, page, PAGE_SIZE, 0);
+		BUG_ON(bio->bi_iter.bi_size != PAGE_SIZE);
 	}
 	return bio;
 }
-- 
2.1.0

[RFC PATCH 16/17] fs: convert buffer head etc. to use immutable biovecs API.

[Dongsu Park]

From: Kent Overstreet <kmo@daterainc.com>

Increase bio->bi_remaining instead of calling bio_get(),
and call bio_end() instead of bio_put() upon buffer_head submission.
Also make bio submission in kernel/power/block_io.c to properly submit
bios by checking whether bio_chain is available or not.

Doing that, some codes that have been still using the older API
can be converted in order to use the immutable biovecs API.

Signed-off-by: Kent Overstreet <kmo@daterainc.com>
[dpark: add more description in commit message]
Signed-off-by: Dongsu Park <dongsu.park@profitbricks.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
---
 fs/buffer.c             |  4 ++--
 kernel/power/block_io.c | 23 ++++++++++++++++++-----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 35ac0ec..78e63e3 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -3038,13 +3038,13 @@ int _submit_bh(int rw, struct buffer_head *bh, unsigned long bio_flags)
 	if (buffer_prio(bh))
 		rw |= REQ_PRIO;
 
-	bio_get(bio);
+	atomic_inc(&bio->bi_remaining);
 	submit_bio(rw, bio);
 
 	if (bio_flagged(bio, BIO_EOPNOTSUPP))
 		ret = -EOPNOTSUPP;
 
-	bio_put(bio);
+	bio_endio(bio, 0);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(_submit_bh);
diff --git a/kernel/power/block_io.c b/kernel/power/block_io.c
index 9a58bc2..7206408 100644
--- a/kernel/power/block_io.c
+++ b/kernel/power/block_io.c
@@ -34,7 +34,6 @@ static int submit(int rw, struct block_device *bdev, sector_t sector,
 	bio = bio_alloc(__GFP_WAIT | __GFP_HIGH, 1);
 	bio->bi_iter.bi_sector = sector;
 	bio->bi_bdev = bdev;
-	bio->bi_end_io = end_swap_bio_read;
 
 	if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
 		printk(KERN_ERR "PM: Adding page to bio failed at %llu\n",
@@ -44,15 +43,29 @@ static int submit(int rw, struct block_device *bdev, sector_t sector,
 	}
 
 	lock_page(page);
-	bio_get(bio);
 
 	if (bio_chain == NULL) {
-		submit_bio(bio_rw, bio);
-		wait_on_page_locked(page);
+		int err = submit_bio_wait(bio_rw, bio);
+
+		if (err) {
+			SetPageError(page);
+			ClearPageUptodate(page);
+			pr_alert("Read-error on swap-device (%u:%u:%llu)\n",
+				 imajor(bio->bi_bdev->bd_inode),
+				 iminor(bio->bi_bdev->bd_inode),
+				 (unsigned long long)bio->bi_iter.bi_sector);
+		} else {
+			SetPageUptodate(page);
+		}
+
 		if (rw == READ)
-			bio_set_pages_dirty(bio);
+			set_page_dirty_lock(page);
+		unlock_page(page);
 		bio_put(bio);
 	} else {
+		bio->bi_end_io = end_swap_bio_read;
+		bio_get(bio);
+
 		if (rw == READ)
 			get_page(page);	/* These pages are freed later */
 		bio->bi_private = *bio_chain;
-- 
2.1.0

Re: [RFC PATCH 15/17] fs: use helper bio_add_page() instead of open coding on bi_io_vec

[Dave Kleikamp]

On 12/22/2014 05:48 AM, Dongsu Park wrote:
> From: Kent Overstreet <kmo@daterainc.com>
> 
> Call pre-defined helper bio_add_page() instead of open coding for
> iterating through bi_io_vec[]. Doing that, it's possible to make some
> parts in filesystems and mm/page_io.c simpler than before.
> 
> Signed-off-by: Kent Overstreet <kmo@daterainc.com>
> [dpark: add more description in commit message]
> Signed-off-by: Dongsu Park <dongsu.park@profitbricks.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Dave Kleikamp <shaggy@kernel.org>

Acked-by: Dave Kleikamp <dave.kleikamp@oracle.com>

> Cc: linux-fsdevel@vger.kernel.org
> ---
>  fs/buffer.c         |  7 ++-----
>  fs/jfs/jfs_logmgr.c | 14 ++++----------
>  mm/page_io.c        |  8 +++-----
>  3 files changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 20805db..35ac0ec 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -3022,12 +3022,9 @@ int _submit_bh(int rw, struct buffer_head *bh, unsigned long bio_flags)
>  
>  	bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9);
>  	bio->bi_bdev = bh->b_bdev;
> -	bio->bi_io_vec[0].bv_page = bh->b_page;
> -	bio->bi_io_vec[0].bv_len = bh->b_size;
> -	bio->bi_io_vec[0].bv_offset = bh_offset(bh);
>  
> -	bio->bi_vcnt = 1;
> -	bio->bi_iter.bi_size = bh->b_size;
> +	bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh));
> +	BUG_ON(bio->bi_iter.bi_size != bh->b_size);
>  
>  	bio->bi_end_io = end_bio_bh_io_sync;
>  	bio->bi_private = bh;
> diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
> index bc462dc..46fae06 100644
> --- a/fs/jfs/jfs_logmgr.c
> +++ b/fs/jfs/jfs_logmgr.c
> @@ -1999,12 +1999,9 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp)
>  
>  	bio->bi_iter.bi_sector = bp->l_blkno << (log->l2bsize - 9);
>  	bio->bi_bdev = log->bdev;
> -	bio->bi_io_vec[0].bv_page = bp->l_page;
> -	bio->bi_io_vec[0].bv_len = LOGPSIZE;
> -	bio->bi_io_vec[0].bv_offset = bp->l_offset;
>  
> -	bio->bi_vcnt = 1;
> -	bio->bi_iter.bi_size = LOGPSIZE;
> +	bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset);
> +	BUG_ON(bio->bi_iter.bi_size != LOGPSIZE);
>  
>  	bio->bi_end_io = lbmIODone;
>  	bio->bi_private = bp;
> @@ -2145,12 +2142,9 @@ static void lbmStartIO(struct lbuf * bp)
>  	bio = bio_alloc(GFP_NOFS, 1);
>  	bio->bi_iter.bi_sector = bp->l_blkno << (log->l2bsize - 9);
>  	bio->bi_bdev = log->bdev;
> -	bio->bi_io_vec[0].bv_page = bp->l_page;
> -	bio->bi_io_vec[0].bv_len = LOGPSIZE;
> -	bio->bi_io_vec[0].bv_offset = bp->l_offset;
>  
> -	bio->bi_vcnt = 1;
> -	bio->bi_iter.bi_size = LOGPSIZE;
> +	bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset);
> +	BUG_ON(bio->bi_iter.bi_size != LOGPSIZE);
>  
>  	bio->bi_end_io = lbmIODone;
>  	bio->bi_private = bp;
> diff --git a/mm/page_io.c b/mm/page_io.c
> index 955db8b..8c878c7 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -33,12 +33,10 @@ static struct bio *get_swap_bio(gfp_t gfp_flags,
>  	if (bio) {
>  		bio->bi_iter.bi_sector = map_swap_page(page, &bio->bi_bdev);
>  		bio->bi_iter.bi_sector <<= PAGE_SHIFT - 9;
> -		bio->bi_io_vec[0].bv_page = page;
> -		bio->bi_io_vec[0].bv_len = PAGE_SIZE;
> -		bio->bi_io_vec[0].bv_offset = 0;
> -		bio->bi_vcnt = 1;
> -		bio->bi_iter.bi_size = PAGE_SIZE;
>  		bio->bi_end_io = end_io;
> +
> +		bio_add_page(bio, page, PAGE_SIZE, 0);
> +		BUG_ON(bio->bi_iter.bi_size != PAGE_SIZE);
>  	}
>  	return bio;
>  }
> 

Re: [RFC PATCH 16/17] fs: convert buffer head etc. to use immutable biovecs API.

[Christoph Hellwig]

On Mon, Dec 22, 2014 at 12:48:43PM +0100, Dongsu Park wrote:
> From: Kent Overstreet <kmo@daterainc.com>
> 
> Increase bio->bi_remaining instead of calling bio_get(),
> and call bio_end() instead of bio_put() upon buffer_head submission.

Nees an explanation on why this is done.

> Also make bio submission in kernel/power/block_io.c to properly submit
> bios by checking whether bio_chain is available or not.

Should be a separate patch.

And of course both should go into the preparation series of cleanups.

Re: [RFC PATCH 16/17] fs: convert buffer head etc. to use immutable biovecs API.

[Dongsu Park]

On 23.12.2014 02:51, Christoph Hellwig wrote:
> On Mon, Dec 22, 2014 at 12:48:43PM +0100, Dongsu Park wrote:
> > From: Kent Overstreet <kmo@daterainc.com>
> > 
> > Increase bio->bi_remaining instead of calling bio_get(),
> > and call bio_end() instead of bio_put() upon buffer_head submission.
> 
> Nees an explanation on why this is done.

Right.

> > Also make bio submission in kernel/power/block_io.c to properly submit
> > bios by checking whether bio_chain is available or not.
> 
> Should be a separate patch.
> 
> And of course both should go into the preparation series of cleanups.

Yep, agreed.
I'll split it up and also move them to the prep series.

Thanks,
Dongsu