2015-05-19 09:40:00

by Chao Yu

[permalink] [raw]
Subject: [PATCH 1/2] f2fs: avoid duplicated code by reusing f2fs_read_end_io

This patch tries to clean up code as part code of f2fs_read_end_io
and mpage_end_io are the same, so it's better to merge and reuse them.

Signed-off-by: Chao Yu <[email protected]>
---
fs/f2fs/data.c | 35 +++++------------------------------
1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8d04e24..5173d0f 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -34,40 +34,16 @@ static void f2fs_read_end_io(struct bio *bio, int err)
struct bio_vec *bvec;
int i;

- bio_for_each_segment_all(bvec, bio, i) {
- struct page *page = bvec->bv_page;
-
- if (!err) {
- SetPageUptodate(page);
- } else {
- ClearPageUptodate(page);
- SetPageError(page);
- }
- unlock_page(page);
- }
- bio_put(bio);
-}
-
-/*
- * I/O completion handler for multipage BIOs.
- * copied from fs/mpage.c
- */
-static void mpage_end_io(struct bio *bio, int err)
-{
- struct bio_vec *bv;
- int i;
-
if (f2fs_bio_encrypted(bio)) {
if (err) {
f2fs_release_crypto_ctx(bio->bi_private);
} else {
- f2fs_end_io_crypto_work(bio->bi_private, bio);
- return;
+ return f2fs_end_io_crypto_work(bio->bi_private, bio);
}
}

- bio_for_each_segment_all(bv, bio, i) {
- struct page *page = bv->bv_page;
+ bio_for_each_segment_all(bvec, bio, i) {
+ struct page *page = bvec->bv_page;

if (!err) {
SetPageUptodate(page);
@@ -77,7 +53,6 @@ static void mpage_end_io(struct bio *bio, int err)
}
unlock_page(page);
}
-
bio_put(bio);
}

@@ -122,7 +97,7 @@ static struct bio *__bio_alloc(struct f2fs_sb_info *sbi, block_t blk_addr,
bio->bi_bdev = sbi->sb->s_bdev;
bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(blk_addr);
bio->bi_end_io = is_read ? f2fs_read_end_io : f2fs_write_end_io;
- bio->bi_private = sbi;
+ bio->bi_private = is_read ? NULL : sbi;

return bio;
}
@@ -1584,7 +1559,7 @@ submit_and_realloc:
}
bio->bi_bdev = bdev;
bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(block_nr);
- bio->bi_end_io = mpage_end_io;
+ bio->bi_end_io = f2fs_read_end_io;
bio->bi_private = ctx;
}

--
2.3.3


2015-05-19 17:48:58

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 1/2] f2fs: avoid duplicated code by reusing f2fs_read_end_io

Hi Chao,

On Tue, May 19, 2015 at 05:38:56PM +0800, Chao Yu wrote:
> This patch tries to clean up code as part code of f2fs_read_end_io
> and mpage_end_io are the same, so it's better to merge and reuse them.
>
> Signed-off-by: Chao Yu <[email protected]>
> ---
> fs/f2fs/data.c | 35 +++++------------------------------
> 1 file changed, 5 insertions(+), 30 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 8d04e24..5173d0f 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -34,40 +34,16 @@ static void f2fs_read_end_io(struct bio *bio, int err)
> struct bio_vec *bvec;
> int i;
>
> - bio_for_each_segment_all(bvec, bio, i) {
> - struct page *page = bvec->bv_page;
> -
> - if (!err) {
> - SetPageUptodate(page);
> - } else {
> - ClearPageUptodate(page);
> - SetPageError(page);
> - }
> - unlock_page(page);
> - }
> - bio_put(bio);
> -}
> -
> -/*
> - * I/O completion handler for multipage BIOs.
> - * copied from fs/mpage.c
> - */
> -static void mpage_end_io(struct bio *bio, int err)
> -{
> - struct bio_vec *bv;
> - int i;
> -
> if (f2fs_bio_encrypted(bio)) {
> if (err) {
> f2fs_release_crypto_ctx(bio->bi_private);
> } else {
> - f2fs_end_io_crypto_work(bio->bi_private, bio);
> - return;
> + return f2fs_end_io_crypto_work(bio->bi_private, bio);

IMO, it'd better remain the previous one to make clear that
f2fs_en_io_crypto_work defines a void return.
How about that?

Thanks,

> }
> }
>
> - bio_for_each_segment_all(bv, bio, i) {
> - struct page *page = bv->bv_page;
> + bio_for_each_segment_all(bvec, bio, i) {
> + struct page *page = bvec->bv_page;
>
> if (!err) {
> SetPageUptodate(page);
> @@ -77,7 +53,6 @@ static void mpage_end_io(struct bio *bio, int err)
> }
> unlock_page(page);
> }
> -
> bio_put(bio);
> }
>
> @@ -122,7 +97,7 @@ static struct bio *__bio_alloc(struct f2fs_sb_info *sbi, block_t blk_addr,
> bio->bi_bdev = sbi->sb->s_bdev;
> bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(blk_addr);
> bio->bi_end_io = is_read ? f2fs_read_end_io : f2fs_write_end_io;
> - bio->bi_private = sbi;
> + bio->bi_private = is_read ? NULL : sbi;
>
> return bio;
> }
> @@ -1584,7 +1559,7 @@ submit_and_realloc:
> }
> bio->bi_bdev = bdev;
> bio->bi_iter.bi_sector = SECTOR_FROM_BLOCK(block_nr);
> - bio->bi_end_io = mpage_end_io;
> + bio->bi_end_io = f2fs_read_end_io;
> bio->bi_private = ctx;
> }
>
> --
> 2.3.3

2015-05-25 10:02:54

by Chao Yu

[permalink] [raw]
Subject: RE: [PATCH 1/2] f2fs: avoid duplicated code by reusing f2fs_read_end_io

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:[email protected]]
> Sent: Wednesday, May 20, 2015 1:49 AM
> To: Chao Yu
> Cc: Changman Lee; [email protected]; [email protected]
> Subject: Re: [PATCH 1/2] f2fs: avoid duplicated code by reusing f2fs_read_end_io
>
> Hi Chao,
>
> On Tue, May 19, 2015 at 05:38:56PM +0800, Chao Yu wrote:
> > This patch tries to clean up code as part code of f2fs_read_end_io
> > and mpage_end_io are the same, so it's better to merge and reuse them.
> >
> > Signed-off-by: Chao Yu <[email protected]>
> > ---
> > fs/f2fs/data.c | 35 +++++------------------------------
> > 1 file changed, 5 insertions(+), 30 deletions(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 8d04e24..5173d0f 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -34,40 +34,16 @@ static void f2fs_read_end_io(struct bio *bio, int err)
> > struct bio_vec *bvec;
> > int i;
> >
> > - bio_for_each_segment_all(bvec, bio, i) {
> > - struct page *page = bvec->bv_page;
> > -
> > - if (!err) {
> > - SetPageUptodate(page);
> > - } else {
> > - ClearPageUptodate(page);
> > - SetPageError(page);
> > - }
> > - unlock_page(page);
> > - }
> > - bio_put(bio);
> > -}
> > -
> > -/*
> > - * I/O completion handler for multipage BIOs.
> > - * copied from fs/mpage.c
> > - */
> > -static void mpage_end_io(struct bio *bio, int err)
> > -{
> > - struct bio_vec *bv;
> > - int i;
> > -
> > if (f2fs_bio_encrypted(bio)) {
> > if (err) {
> > f2fs_release_crypto_ctx(bio->bi_private);
> > } else {
> > - f2fs_end_io_crypto_work(bio->bi_private, bio);
> > - return;
> > + return f2fs_end_io_crypto_work(bio->bi_private, bio);
>
> IMO, it'd better remain the previous one to make clear that
> f2fs_en_io_crypto_work defines a void return.
> How about that?

OK, will do.

Thanks,