2022-06-20 09:05:54

by Ritesh Harjani

[permalink] [raw]
Subject: [PATCHv2 0/4] submit_bh: Drop unnecessary return values and API users

submit_bh/submit_bh_wbc are non-blocking functions which just submits
the bio and returns. The caller of submit_bh/submit_bh_wbc needs to wait
on buffer till I/O completion and then check buffer head's b_state field
to know if there was any I/O error.

Hence there is no need for these functions to have any return type.
Even now they always returns 0. Hence drop the return value and make
their return type as void to avoid any confusion.

RFC -> PATCHv2
===============
1. Added Patch-2 to fix ntfs_submit_bh_for_read() caller.
2. Added Reviewed-by from Christoph.

Ritesh Harjani (4):
jbd2: Drop useless return value of submit_bh
fs/ntfs: Drop useless return value of submit_bh from ntfs_submit_bh_for_read
fs/buffer: Drop useless return value of submit_bh
fs/buffer: Make submit_bh & submit_bh_wbc return type as void

fs/buffer.c | 19 ++++++++-----------
fs/jbd2/commit.c | 11 +++++------
fs/jbd2/journal.c | 6 ++----
fs/ntfs/file.c | 4 ++--
include/linux/buffer_head.h | 2 +-
5 files changed, 18 insertions(+), 24 deletions(-)

--
2.35.3


2022-06-20 09:05:55

by Ritesh Harjani

[permalink] [raw]
Subject: [PATCHv2 2/4] fs/ntfs: Drop useless return value of submit_bh from ntfs_submit_bh_for_read

submit_bh always returns 0. This patch drops the useless return value of
submit_bh from ntfs_submit_bh_for_read(). Once all of submit_bh callers are
cleaned up, we can make it's return type as void.

Reported-by: kernel test robot <[email protected]>
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/ntfs/file.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index a8abe2296514..2389bfa654a2 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -532,12 +532,12 @@ static inline int __ntfs_grab_cache_pages(struct address_space *mapping,
goto out;
}

-static inline int ntfs_submit_bh_for_read(struct buffer_head *bh)
+static inline void ntfs_submit_bh_for_read(struct buffer_head *bh)
{
lock_buffer(bh);
get_bh(bh);
bh->b_end_io = end_buffer_read_sync;
- return submit_bh(REQ_OP_READ, 0, bh);
+ submit_bh(REQ_OP_READ, 0, bh);
}

/**
--
2.35.3

2022-06-20 09:06:05

by Ritesh Harjani

[permalink] [raw]
Subject: [PATCHv2 3/4] fs/buffer: Drop useless return value of submit_bh

submit_bh always returns 0. This patch drops the useless return value of
submit_bh from __sync_dirty_buffer(). Once all of submit_bh callers are
cleaned up, we can make it's return type as void.

Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/buffer.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 898c7f301b1b..313283af15b6 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -3121,8 +3121,6 @@ EXPORT_SYMBOL(write_dirty_buffer);
*/
int __sync_dirty_buffer(struct buffer_head *bh, int op_flags)
{
- int ret = 0;
-
WARN_ON(atomic_read(&bh->b_count) < 1);
lock_buffer(bh);
if (test_clear_buffer_dirty(bh)) {
@@ -3137,14 +3135,14 @@ int __sync_dirty_buffer(struct buffer_head *bh, int op_flags)

get_bh(bh);
bh->b_end_io = end_buffer_write_sync;
- ret = submit_bh(REQ_OP_WRITE, op_flags, bh);
+ submit_bh(REQ_OP_WRITE, op_flags, bh);
wait_on_buffer(bh);
- if (!ret && !buffer_uptodate(bh))
- ret = -EIO;
+ if (!buffer_uptodate(bh))
+ return -EIO;
} else {
unlock_buffer(bh);
}
- return ret;
+ return 0;
}
EXPORT_SYMBOL(__sync_dirty_buffer);

--
2.35.3

2022-06-20 09:06:27

by Ritesh Harjani

[permalink] [raw]
Subject: [PATCHv2 4/4] fs/buffer: Make submit_bh & submit_bh_wbc return type as void

submit_bh/submit_bh_wbc are non-blocking functions which just submit
the bio and return. The caller of submit_bh/submit_bh_wbc needs to wait
on buffer till I/O completion and then check buffer head's b_state field
to know if there was any I/O error.

Hence there is no need for these functions to have any return type.
Even now they always returns 0. Hence drop the return value and make
their return type as void to avoid any confusion.

Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/buffer.c | 9 ++++-----
include/linux/buffer_head.h | 2 +-
2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 313283af15b6..6671abc98e21 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -52,7 +52,7 @@
#include "internal.h"

static int fsync_buffers_list(spinlock_t *lock, struct list_head *list);
-static int submit_bh_wbc(int op, int op_flags, struct buffer_head *bh,
+static void submit_bh_wbc(int op, int op_flags, struct buffer_head *bh,
struct writeback_control *wbc);

#define BH_ENTRY(list) list_entry((list), struct buffer_head, b_assoc_buffers)
@@ -2994,7 +2994,7 @@ static void end_bio_bh_io_sync(struct bio *bio)
bio_put(bio);
}

-static int submit_bh_wbc(int op, int op_flags, struct buffer_head *bh,
+static void submit_bh_wbc(int op, int op_flags, struct buffer_head *bh,
struct writeback_control *wbc)
{
struct bio *bio;
@@ -3037,12 +3037,11 @@ static int submit_bh_wbc(int op, int op_flags, struct buffer_head *bh,
}

submit_bio(bio);
- return 0;
}

-int submit_bh(int op, int op_flags, struct buffer_head *bh)
+void submit_bh(int op, int op_flags, struct buffer_head *bh)
{
- return submit_bh_wbc(op, op_flags, bh, NULL);
+ submit_bh_wbc(op, op_flags, bh, NULL);
}
EXPORT_SYMBOL(submit_bh);

diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index c9d1463bb20f..392d7d5aec05 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -205,7 +205,7 @@ void ll_rw_block(int, int, int, struct buffer_head * bh[]);
int sync_dirty_buffer(struct buffer_head *bh);
int __sync_dirty_buffer(struct buffer_head *bh, int op_flags);
void write_dirty_buffer(struct buffer_head *bh, int op_flags);
-int submit_bh(int, int, struct buffer_head *);
+void submit_bh(int op, int op_flags, struct buffer_head *bh);
void write_boundary_block(struct block_device *bdev,
sector_t bblock, unsigned blocksize);
int bh_uptodate_or_lock(struct buffer_head *bh);
--
2.35.3

2022-06-20 09:20:35

by Ritesh Harjani

[permalink] [raw]
Subject: Re: [PATCHv2 2/4] fs/ntfs: Drop useless return value of submit_bh from ntfs_submit_bh_for_read

+ [email protected]
(sorry about not cc'ing this in the first place)

On 22/06/20 02:34PM, Ritesh Harjani wrote:
> submit_bh always returns 0. This patch drops the useless return value of
> submit_bh from ntfs_submit_bh_for_read(). Once all of submit_bh callers are
> cleaned up, we can make it's return type as void.
>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Ritesh Harjani <[email protected]>
> ---
> fs/ntfs/file.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index a8abe2296514..2389bfa654a2 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -532,12 +532,12 @@ static inline int __ntfs_grab_cache_pages(struct address_space *mapping,
> goto out;
> }
>
> -static inline int ntfs_submit_bh_for_read(struct buffer_head *bh)
> +static inline void ntfs_submit_bh_for_read(struct buffer_head *bh)
> {
> lock_buffer(bh);
> get_bh(bh);
> bh->b_end_io = end_buffer_read_sync;
> - return submit_bh(REQ_OP_READ, 0, bh);
> + submit_bh(REQ_OP_READ, 0, bh);
> }
>
> /**
> --
> 2.35.3
>

2022-06-20 09:52:51

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCHv2 2/4] fs/ntfs: Drop useless return value of submit_bh from ntfs_submit_bh_for_read

On Mon 20-06-22 14:34:35, Ritesh Harjani wrote:
> submit_bh always returns 0. This patch drops the useless return value of
> submit_bh from ntfs_submit_bh_for_read(). Once all of submit_bh callers are
> cleaned up, we can make it's return type as void.
>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Ritesh Harjani <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> fs/ntfs/file.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index a8abe2296514..2389bfa654a2 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -532,12 +532,12 @@ static inline int __ntfs_grab_cache_pages(struct address_space *mapping,
> goto out;
> }
>
> -static inline int ntfs_submit_bh_for_read(struct buffer_head *bh)
> +static inline void ntfs_submit_bh_for_read(struct buffer_head *bh)
> {
> lock_buffer(bh);
> get_bh(bh);
> bh->b_end_io = end_buffer_read_sync;
> - return submit_bh(REQ_OP_READ, 0, bh);
> + submit_bh(REQ_OP_READ, 0, bh);
> }
>
> /**
> --
> 2.35.3
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2022-06-21 06:36:26

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCHv2 2/4] fs/ntfs: Drop useless return value of submit_bh from ntfs_submit_bh_for_read

On Mon, Jun 20, 2022 at 02:34:35PM +0530, Ritesh Harjani wrote:
> submit_bh always returns 0. This patch drops the useless return value of
> submit_bh from ntfs_submit_bh_for_read(). Once all of submit_bh callers are
> cleaned up, we can make it's return type as void.

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

and so sad that a newly merged file systems still does all this buffer head
crap :(