diff mbox series

[RFC,1/3] jbd2: Drop useless return value of submit_bh

Message ID 57b9cb59e50dfdf68eef82ef38944fbceba4e585.1655703467.git.ritesh.list@gmail.com (mailing list archive)
State New, archived
Headers show
Series submit_bh: Drop unnecessary return values and API users | expand

Commit Message

Ritesh Harjani (IBM) June 20, 2022, 5:58 a.m. UTC 
submit_bh always returns 0. This patch cleans up 2 of it's caller
in jbd2 to drop submit_bh's useless return value.
Once all submit_bh callers are cleaned up, we can make it's return
type as void.

Signed-off-by: Ritesh Harjani <ritesh.list@gmail.com>
---
 fs/jbd2/commit.c  | 11 +++++------
 fs/jbd2/journal.c |  6 ++----
 2 files changed, 7 insertions(+), 10 deletions(-)

Comments

Christoph Hellwig June 20, 2022, 6:39 a.m. UTC | #1 
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Jan Kara June 20, 2022, 9:39 a.m. UTC | #2 
On Mon 20-06-22 11:28:40, Ritesh Harjani wrote:
> submit_bh always returns 0. This patch cleans up 2 of it's caller
> in jbd2 to drop submit_bh's useless return value.
> Once all submit_bh callers are cleaned up, we can make it's return
> type as void.
> 
> Signed-off-by: Ritesh Harjani <ritesh.list@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/jbd2/commit.c  | 11 +++++------
>  fs/jbd2/journal.c |  6 ++----
>  2 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index eb315e81f1a6..688fd960d01f 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -122,8 +122,8 @@ static int journal_submit_commit_record(journal_t *journal,
>  {
>  	struct commit_header *tmp;
>  	struct buffer_head *bh;
> -	int ret;
>  	struct timespec64 now;
> +	int write_flags = REQ_SYNC;
>  
>  	*cbh = NULL;
>  
> @@ -155,13 +155,12 @@ static int journal_submit_commit_record(journal_t *journal,
>  
>  	if (journal->j_flags & JBD2_BARRIER &&
>  	    !jbd2_has_feature_async_commit(journal))
> -		ret = submit_bh(REQ_OP_WRITE,
> -			REQ_SYNC | REQ_PREFLUSH | REQ_FUA, bh);
> -	else
> -		ret = submit_bh(REQ_OP_WRITE, REQ_SYNC, bh);
> +		write_flags |= REQ_PREFLUSH | REQ_FUA;
> +
> +	submit_bh(REQ_OP_WRITE, write_flags, bh);
>  
>  	*cbh = bh;
> -	return ret;
> +	return 0;
>  }
>  
>  /*
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index c0cbeeaec2d1..81a282e676bc 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1606,7 +1606,7 @@ static int jbd2_write_superblock(journal_t *journal, int write_flags)
>  {
>  	struct buffer_head *bh = journal->j_sb_buffer;
>  	journal_superblock_t *sb = journal->j_superblock;
> -	int ret;
> +	int ret = 0;
>  
>  	/* Buffer got discarded which means block device got invalidated */
>  	if (!buffer_mapped(bh)) {
> @@ -1636,14 +1636,12 @@ static int jbd2_write_superblock(journal_t *journal, int write_flags)
>  		sb->s_checksum = jbd2_superblock_csum(journal, sb);
>  	get_bh(bh);
>  	bh->b_end_io = end_buffer_write_sync;
> -	ret = submit_bh(REQ_OP_WRITE, write_flags, bh);
> +	submit_bh(REQ_OP_WRITE, write_flags, bh);
>  	wait_on_buffer(bh);
>  	if (buffer_write_io_error(bh)) {
>  		clear_buffer_write_io_error(bh);
>  		set_buffer_uptodate(bh);
>  		ret = -EIO;
> -	}
> -	if (ret) {
>  		printk(KERN_ERR "JBD2: Error %d detected when updating "
>  		       "journal superblock for %s.\n", ret,
>  		       journal->j_devname);
> -- 
> 2.35.3
>
Matthew Wilcox (Oracle) June 21, 2022, 1:39 a.m. UTC | #3 
On Mon, Jun 20, 2022 at 11:28:40AM +0530, Ritesh Harjani wrote:
> @@ -1636,14 +1636,12 @@ static int jbd2_write_superblock(journal_t *journal, int write_flags)
>  		sb->s_checksum = jbd2_superblock_csum(journal, sb);
>  	get_bh(bh);
>  	bh->b_end_io = end_buffer_write_sync;
> -	ret = submit_bh(REQ_OP_WRITE, write_flags, bh);
> +	submit_bh(REQ_OP_WRITE, write_flags, bh);
>  	wait_on_buffer(bh);
>  	if (buffer_write_io_error(bh)) {
>  		clear_buffer_write_io_error(bh);
>  		set_buffer_uptodate(bh);
>  		ret = -EIO;
> -	}
> -	if (ret) {
>  		printk(KERN_ERR "JBD2: Error %d detected when updating "
>  		       "journal superblock for %s.\n", ret,
>  		       journal->j_devname);

Maybe rephrase the error message?  And join it together to match the
current preferred style.

		printk(KERN_ERR "JBD2: I/O error when updating journal superblock for %s.\n",
				journal->j_devname);
Ritesh Harjani (IBM) July 4, 2022, 9:01 a.m. UTC | #4 
On 22/06/21 02:39AM, Matthew Wilcox wrote:
> On Mon, Jun 20, 2022 at 11:28:40AM +0530, Ritesh Harjani wrote:
> > @@ -1636,14 +1636,12 @@ static int jbd2_write_superblock(journal_t *journal, int write_flags)
> >  		sb->s_checksum = jbd2_superblock_csum(journal, sb);
> >  	get_bh(bh);
> >  	bh->b_end_io = end_buffer_write_sync;
> > -	ret = submit_bh(REQ_OP_WRITE, write_flags, bh);
> > +	submit_bh(REQ_OP_WRITE, write_flags, bh);
> >  	wait_on_buffer(bh);
> >  	if (buffer_write_io_error(bh)) {
> >  		clear_buffer_write_io_error(bh);
> >  		set_buffer_uptodate(bh);
> >  		ret = -EIO;
> > -	}
> > -	if (ret) {
> >  		printk(KERN_ERR "JBD2: Error %d detected when updating "
> >  		       "journal superblock for %s.\n", ret,
> >  		       journal->j_devname);
>
> Maybe rephrase the error message?  And join it together to match the
> current preferred style.
>
> 		printk(KERN_ERR "JBD2: I/O error when updating journal superblock for %s.\n",
> 				journal->j_devname);

Sure, I will update the printk message like above and send out a v3
(since I haven't receieved any other comments so I think v3 should be good to be
picked up now)

-ritesh
Ritesh Harjani (IBM) July 18, 2022, 5:55 p.m. UTC | #5 
On 22/07/04 02:31PM, Ritesh Harjani wrote:
> On 22/06/21 02:39AM, Matthew Wilcox wrote:
> > On Mon, Jun 20, 2022 at 11:28:40AM +0530, Ritesh Harjani wrote:
> > > @@ -1636,14 +1636,12 @@ static int jbd2_write_superblock(journal_t *journal, int write_flags)
> > >  		sb->s_checksum = jbd2_superblock_csum(journal, sb);
> > >  	get_bh(bh);
> > >  	bh->b_end_io = end_buffer_write_sync;
> > > -	ret = submit_bh(REQ_OP_WRITE, write_flags, bh);
> > > +	submit_bh(REQ_OP_WRITE, write_flags, bh);
> > >  	wait_on_buffer(bh);
> > >  	if (buffer_write_io_error(bh)) {
> > >  		clear_buffer_write_io_error(bh);
> > >  		set_buffer_uptodate(bh);
> > >  		ret = -EIO;
> > > -	}
> > > -	if (ret) {
> > >  		printk(KERN_ERR "JBD2: Error %d detected when updating "
> > >  		       "journal superblock for %s.\n", ret,
> > >  		       journal->j_devname);
> >
> > Maybe rephrase the error message?  And join it together to match the
> > current preferred style.
> >
> > 		printk(KERN_ERR "JBD2: I/O error when updating journal superblock for %s.\n",
> > 				journal->j_devname);
>
> Sure, I will update the printk message like above and send out a v3
> (since I haven't receieved any other comments so I think v3 should be good to be
> picked up now)


We were planning to send this patch series via ext4 tree.
But it seems this might conflict with the below mentioned patches sitting in
linux-next. So let me rebase my patches on top of these and maybe hold to this
series until the current set of changes land in linux tree to avoid any merge
conflicts later.
But either ways do let me know if you would like to take any other preferred
route. Since this is not critical, so I am fine with either ways you suggest.


-ritesh

author Bart Van Assche <bvanassche@acm.org> Thu Jul 14 11:07:13 2022 -0700

fs/buffer: Combine two submit_bh() and ll_rw_block() arguments

Both submit_bh() and ll_rw_block() accept a request operation type and
request flags as their first two arguments. Micro-optimize these two
functions by combining these first two arguments into a single argument.
This patch does not change the behavior of any of the modified code.
diff mbox series

Patch

diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index eb315e81f1a6..688fd960d01f 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -122,8 +122,8 @@  static int journal_submit_commit_record(journal_t *journal,
 {
 	struct commit_header *tmp;
 	struct buffer_head *bh;
-	int ret;
 	struct timespec64 now;
+	int write_flags = REQ_SYNC;
 
 	*cbh = NULL;
 
@@ -155,13 +155,12 @@  static int journal_submit_commit_record(journal_t *journal,
 
 	if (journal->j_flags & JBD2_BARRIER &&
 	    !jbd2_has_feature_async_commit(journal))
-		ret = submit_bh(REQ_OP_WRITE,
-			REQ_SYNC | REQ_PREFLUSH | REQ_FUA, bh);
-	else
-		ret = submit_bh(REQ_OP_WRITE, REQ_SYNC, bh);
+		write_flags |= REQ_PREFLUSH | REQ_FUA;
+
+	submit_bh(REQ_OP_WRITE, write_flags, bh);
 
 	*cbh = bh;
-	return ret;
+	return 0;
 }
 
 /*
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index c0cbeeaec2d1..81a282e676bc 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1606,7 +1606,7 @@  static int jbd2_write_superblock(journal_t *journal, int write_flags)
 {
 	struct buffer_head *bh = journal->j_sb_buffer;
 	journal_superblock_t *sb = journal->j_superblock;
-	int ret;
+	int ret = 0;
 
 	/* Buffer got discarded which means block device got invalidated */
 	if (!buffer_mapped(bh)) {
@@ -1636,14 +1636,12 @@  static int jbd2_write_superblock(journal_t *journal, int write_flags)
 		sb->s_checksum = jbd2_superblock_csum(journal, sb);
 	get_bh(bh);
 	bh->b_end_io = end_buffer_write_sync;
-	ret = submit_bh(REQ_OP_WRITE, write_flags, bh);
+	submit_bh(REQ_OP_WRITE, write_flags, bh);
 	wait_on_buffer(bh);
 	if (buffer_write_io_error(bh)) {
 		clear_buffer_write_io_error(bh);
 		set_buffer_uptodate(bh);
 		ret = -EIO;
-	}
-	if (ret) {
 		printk(KERN_ERR "JBD2: Error %d detected when updating "
 		       "journal superblock for %s.\n", ret,
 		       journal->j_devname);