[PATCH 0/6] Minor cleanups to jbd2

[PATCH 0/6] Minor cleanups to jbd2

[Kemeng Shi]

This series contains some random minor cleanups to jbd2. No funtional
change is intended. More details can be found in respective patches.
Thanks.

Kemeng Shi (6):
  jbd2: remove unused h_jdata flag of handle
  jbd2: remove unused return value of jbd2_journal_cancel_revoke
  jbd2: remove unused return value of do_readahead
  jbd2: remove stale comment of update_t_max_wait
  jbd2: correct stale function name in comment
  jbd2: Correct stale comment of release_buffer_page

 fs/jbd2/commit.c      |  4 ++--
 fs/jbd2/recovery.c    | 11 +++--------
 fs/jbd2/revoke.c      | 13 +++++--------
 fs/jbd2/transaction.c |  3 +--
 include/linux/jbd2.h  |  4 +---
 5 files changed, 12 insertions(+), 23 deletions(-)

-- 
2.30.0

[PATCH 1/6] jbd2: remove unused h_jdata flag of handle

[Kemeng Shi]

Flag h_jdata is not used, just remove it.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 include/linux/jbd2.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 50f7ea8714bf..c7fdb2b1b9a6 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -459,7 +459,6 @@ struct jbd2_revoke_table_s;
  * @h_ref: Reference count on this handle.
  * @h_err: Field for caller's use to track errors through large fs operations.
  * @h_sync: Flag for sync-on-close.
- * @h_jdata: Flag to force data journaling.
  * @h_reserved: Flag for handle for reserved credits.
  * @h_aborted: Flag indicating fatal error on handle.
  * @h_type: For handle statistics.
@@ -491,7 +490,6 @@ struct jbd2_journal_handle
 
 	/* Flags [no locking] */
 	unsigned int	h_sync:		1;
-	unsigned int	h_jdata:	1;
 	unsigned int	h_reserved:	1;
 	unsigned int	h_aborted:	1;
 	unsigned int	h_type:		8;
-- 
2.30.0

[PATCH 2/6] jbd2: remove unused return value of jbd2_journal_cancel_revoke

[Kemeng Shi]

Remove unused return value of jbd2_journal_cancel_revoke.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/jbd2/revoke.c     | 5 +----
 include/linux/jbd2.h | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index 4556e4689024..af0208ed3619 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -420,12 +420,11 @@ int jbd2_journal_revoke(handle_t *handle, unsigned long long blocknr,
  * do not trust the Revoked bit on buffers unless RevokeValid is also
  * set.
  */
-int jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
+void jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
 {
 	struct jbd2_revoke_record_s *record;
 	journal_t *journal = handle->h_transaction->t_journal;
 	int need_cancel;
-	int did_revoke = 0;	/* akpm: debug */
 	struct buffer_head *bh = jh2bh(jh);
 
 	jbd2_debug(4, "journal_head %p, cancelling revoke\n", jh);
@@ -450,7 +449,6 @@ int jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
 			list_del(&record->hash);
 			spin_unlock(&journal->j_revoke_lock);
 			kmem_cache_free(jbd2_revoke_record_cache, record);
-			did_revoke = 1;
 		}
 	}
 
@@ -473,7 +471,6 @@ int jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
 			__brelse(bh2);
 		}
 	}
-	return did_revoke;
 }
 
 /*
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index c7fdb2b1b9a6..e2d1426d3e06 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1635,7 +1635,7 @@ extern int __init jbd2_journal_init_revoke_table_cache(void);
 
 extern void	   jbd2_journal_destroy_revoke(journal_t *);
 extern int	   jbd2_journal_revoke (handle_t *, unsigned long long, struct buffer_head *);
-extern int	   jbd2_journal_cancel_revoke(handle_t *, struct journal_head *);
+extern void	   jbd2_journal_cancel_revoke(handle_t *, struct journal_head *);
 extern void	   jbd2_journal_write_revoke_records(transaction_t *transaction,
 						     struct list_head *log_bufs);
 
-- 
2.30.0

[PATCH 3/6] jbd2: remove unused return value of do_readahead

[Kemeng Shi]

Remove unused return value of do_readahead.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/jbd2/recovery.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
index 9192be7c19d8..a671f8ee7dd2 100644
--- a/fs/jbd2/recovery.c
+++ b/fs/jbd2/recovery.c
@@ -65,9 +65,8 @@ static void journal_brelse_array(struct buffer_head *b[], int n)
  */
 
 #define MAXBUF 8
-static int do_readahead(journal_t *journal, unsigned int start)
+static void do_readahead(journal_t *journal, unsigned int start)
 {
-	int err;
 	unsigned int max, nbufs, next;
 	unsigned long long blocknr;
 	struct buffer_head *bh;
@@ -85,7 +84,7 @@ static int do_readahead(journal_t *journal, unsigned int start)
 	nbufs = 0;
 
 	for (next = start; next < max; next++) {
-		err = jbd2_journal_bmap(journal, next, &blocknr);
+		int err = jbd2_journal_bmap(journal, next, &blocknr);
 
 		if (err) {
 			printk(KERN_ERR "JBD2: bad block at offset %u\n",
@@ -94,10 +93,8 @@ static int do_readahead(journal_t *journal, unsigned int start)
 		}
 
 		bh = __getblk(journal->j_dev, blocknr, journal->j_blocksize);
-		if (!bh) {
-			err = -ENOMEM;
+		if (!bh)
 			goto failed;
-		}
 
 		if (!buffer_uptodate(bh) && !buffer_locked(bh)) {
 			bufs[nbufs++] = bh;
@@ -112,12 +109,10 @@ static int do_readahead(journal_t *journal, unsigned int start)
 
 	if (nbufs)
 		bh_readahead_batch(nbufs, bufs, 0);
-	err = 0;
 
 failed:
 	if (nbufs)
 		journal_brelse_array(bufs, nbufs);
-	return err;
 }
 
 #endif /* __KERNEL__ */
-- 
2.30.0

[PATCH 4/6] jbd2: remove stale comment of update_t_max_wait

[Kemeng Shi]

Commit 2d44292058828 "jbd2: remove CONFIG_JBD2_DEBUG to update t_max_wait"
removed jbd2_journal_enable_debug, just remove stale comment about
jbd2_journal_enable_debug.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/jbd2/transaction.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 66513c18ca29..e00b87635512 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -119,7 +119,6 @@ static void jbd2_get_transaction(journal_t *journal,
  * t_max_wait is carefully updated here with use of atomic compare exchange.
  * Note that there could be multiplre threads trying to do this simultaneously
  * hence using cmpxchg to avoid any use of locks in this case.
- * With this t_max_wait can be updated w/o enabling jbd2_journal_enable_debug.
  */
 static inline void update_t_max_wait(transaction_t *transaction,
 				     unsigned long ts)
-- 
2.30.0

[PATCH 5/6] jbd2: correct stale function name in comment

[Kemeng Shi]

Rename stale journal_clear_revoked_flag to jbd2_clear_buffer_revoked_flags.
Rename stale journal_switch_revoke to jbd2_journal_switch_revoke_table.
Rename stale __journal_file_buffer to __jbd2_journal_file_buffer.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/jbd2/revoke.c      | 8 ++++----
 fs/jbd2/transaction.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index af0208ed3619..5b7350109c5a 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -474,7 +474,7 @@ void jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
 }
 
 /*
- * journal_clear_revoked_flag clears revoked flag of buffers in
+ * jbd2_clear_buffer_revoked_flags clears revoked flag of buffers in
  * revoke table to reflect there is no revoked buffers in the next
  * transaction which is going to be started.
  */
@@ -503,9 +503,9 @@ void jbd2_clear_buffer_revoked_flags(journal_t *journal)
 	}
 }
 
-/* journal_switch_revoke table select j_revoke for next transaction
- * we do not want to suspend any processing until all revokes are
- * written -bzzz
+/* jbd2_journal_switch_revoke_table table select j_revoke for next
+ * transaction we do not want to suspend any processing until all
+ * revokes are written -bzzz
  */
 void jbd2_journal_switch_revoke_table(journal_t *journal)
 {
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index e00b87635512..908baf73b188 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -2191,7 +2191,7 @@ static int __dispose_buffer(struct journal_head *jh, transaction_t *transaction)
 		/*
 		 * We don't want to write the buffer anymore, clear the
 		 * bit so that we don't confuse checks in
-		 * __journal_file_buffer
+		 * __jbd2_journal_file_buffer
 		 */
 		clear_buffer_dirty(bh);
 		__jbd2_journal_file_buffer(jh, transaction, BJ_Forget);
-- 
2.30.0

[PATCH 6/6] jbd2: Correct stale comment of release_buffer_page

[Kemeng Shi]

Update stale lock info in comment of release_buffer_page.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 fs/jbd2/commit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index 9153ff3a08e7..d812d15f295e 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -57,8 +57,8 @@ static void journal_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
  * So here, we have a buffer which has just come off the forget list.  Look to
  * see if we can strip all buffers from the backing page.
  *
- * Called under lock_journal(), and possibly under journal_datalist_lock.  The
- * caller provided us with a ref against the buffer, and we drop that here.
+ * Called under j_list_lock. The caller provided us with a ref against the
+ * buffer, and we drop that here.
  */
 static void release_buffer_page(struct buffer_head *bh)
 {
-- 
2.30.0

Re: [PATCH 1/6] jbd2: remove unused h_jdata flag of handle

[Jan Kara]

On Wed 25-12-24 04:27:02, Kemeng Shi wrote:
> Flag h_jdata is not used, just remove it.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Looks good. Feel free to add:

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

								Honza


> ---
>  include/linux/jbd2.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 50f7ea8714bf..c7fdb2b1b9a6 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -459,7 +459,6 @@ struct jbd2_revoke_table_s;
>   * @h_ref: Reference count on this handle.
>   * @h_err: Field for caller's use to track errors through large fs operations.
>   * @h_sync: Flag for sync-on-close.
> - * @h_jdata: Flag to force data journaling.
>   * @h_reserved: Flag for handle for reserved credits.
>   * @h_aborted: Flag indicating fatal error on handle.
>   * @h_type: For handle statistics.
> @@ -491,7 +490,6 @@ struct jbd2_journal_handle
>  
>  	/* Flags [no locking] */
>  	unsigned int	h_sync:		1;
> -	unsigned int	h_jdata:	1;
>  	unsigned int	h_reserved:	1;
>  	unsigned int	h_aborted:	1;
>  	unsigned int	h_type:		8;
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 5/6] jbd2: correct stale function name in comment

[Jan Kara]

On Wed 25-12-24 04:27:06, Kemeng Shi wrote:
> Rename stale journal_clear_revoked_flag to jbd2_clear_buffer_revoked_flags.
> Rename stale journal_switch_revoke to jbd2_journal_switch_revoke_table.
> Rename stale __journal_file_buffer to __jbd2_journal_file_buffer.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Thanks! Feel free to add:

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

								Honza

> ---
>  fs/jbd2/revoke.c      | 8 ++++----
>  fs/jbd2/transaction.c | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
> index af0208ed3619..5b7350109c5a 100644
> --- a/fs/jbd2/revoke.c
> +++ b/fs/jbd2/revoke.c
> @@ -474,7 +474,7 @@ void jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
>  }
>  
>  /*
> - * journal_clear_revoked_flag clears revoked flag of buffers in
> + * jbd2_clear_buffer_revoked_flags clears revoked flag of buffers in
>   * revoke table to reflect there is no revoked buffers in the next
>   * transaction which is going to be started.
>   */
> @@ -503,9 +503,9 @@ void jbd2_clear_buffer_revoked_flags(journal_t *journal)
>  	}
>  }
>  
> -/* journal_switch_revoke table select j_revoke for next transaction
> - * we do not want to suspend any processing until all revokes are
> - * written -bzzz
> +/* jbd2_journal_switch_revoke_table table select j_revoke for next
> + * transaction we do not want to suspend any processing until all
> + * revokes are written -bzzz
>   */
>  void jbd2_journal_switch_revoke_table(journal_t *journal)
>  {
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index e00b87635512..908baf73b188 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -2191,7 +2191,7 @@ static int __dispose_buffer(struct journal_head *jh, transaction_t *transaction)
>  		/*
>  		 * We don't want to write the buffer anymore, clear the
>  		 * bit so that we don't confuse checks in
> -		 * __journal_file_buffer
> +		 * __jbd2_journal_file_buffer
>  		 */
>  		clear_buffer_dirty(bh);
>  		__jbd2_journal_file_buffer(jh, transaction, BJ_Forget);
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/6] jbd2: remove unused return value of jbd2_journal_cancel_revoke

[Jan Kara]

On Wed 25-12-24 04:27:03, Kemeng Shi wrote:
> Remove unused return value of jbd2_journal_cancel_revoke.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Yeah, nobody used it for years. Feel free to add:

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

								Honza

> ---
>  fs/jbd2/revoke.c     | 5 +----
>  include/linux/jbd2.h | 2 +-
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
> index 4556e4689024..af0208ed3619 100644
> --- a/fs/jbd2/revoke.c
> +++ b/fs/jbd2/revoke.c
> @@ -420,12 +420,11 @@ int jbd2_journal_revoke(handle_t *handle, unsigned long long blocknr,
>   * do not trust the Revoked bit on buffers unless RevokeValid is also
>   * set.
>   */
> -int jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
> +void jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
>  {
>  	struct jbd2_revoke_record_s *record;
>  	journal_t *journal = handle->h_transaction->t_journal;
>  	int need_cancel;
> -	int did_revoke = 0;	/* akpm: debug */
>  	struct buffer_head *bh = jh2bh(jh);
>  
>  	jbd2_debug(4, "journal_head %p, cancelling revoke\n", jh);
> @@ -450,7 +449,6 @@ int jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
>  			list_del(&record->hash);
>  			spin_unlock(&journal->j_revoke_lock);
>  			kmem_cache_free(jbd2_revoke_record_cache, record);
> -			did_revoke = 1;
>  		}
>  	}
>  
> @@ -473,7 +471,6 @@ int jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
>  			__brelse(bh2);
>  		}
>  	}
> -	return did_revoke;
>  }
>  
>  /*
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index c7fdb2b1b9a6..e2d1426d3e06 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1635,7 +1635,7 @@ extern int __init jbd2_journal_init_revoke_table_cache(void);
>  
>  extern void	   jbd2_journal_destroy_revoke(journal_t *);
>  extern int	   jbd2_journal_revoke (handle_t *, unsigned long long, struct buffer_head *);
> -extern int	   jbd2_journal_cancel_revoke(handle_t *, struct journal_head *);
> +extern void	   jbd2_journal_cancel_revoke(handle_t *, struct journal_head *);
>  extern void	   jbd2_journal_write_revoke_records(transaction_t *transaction,
>  						     struct list_head *log_bufs);
>  
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/6] jbd2: remove unused return value of do_readahead

[Jan Kara]

On Wed 25-12-24 04:27:04, Kemeng Shi wrote:
> Remove unused return value of do_readahead.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Yeah, we are unlikely to act on errors from readahead. Feel free to add:

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

								Honza

> ---
>  fs/jbd2/recovery.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 9192be7c19d8..a671f8ee7dd2 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -65,9 +65,8 @@ static void journal_brelse_array(struct buffer_head *b[], int n)
>   */
>  
>  #define MAXBUF 8
> -static int do_readahead(journal_t *journal, unsigned int start)
> +static void do_readahead(journal_t *journal, unsigned int start)
>  {
> -	int err;
>  	unsigned int max, nbufs, next;
>  	unsigned long long blocknr;
>  	struct buffer_head *bh;
> @@ -85,7 +84,7 @@ static int do_readahead(journal_t *journal, unsigned int start)
>  	nbufs = 0;
>  
>  	for (next = start; next < max; next++) {
> -		err = jbd2_journal_bmap(journal, next, &blocknr);
> +		int err = jbd2_journal_bmap(journal, next, &blocknr);
>  
>  		if (err) {
>  			printk(KERN_ERR "JBD2: bad block at offset %u\n",
> @@ -94,10 +93,8 @@ static int do_readahead(journal_t *journal, unsigned int start)
>  		}
>  
>  		bh = __getblk(journal->j_dev, blocknr, journal->j_blocksize);
> -		if (!bh) {
> -			err = -ENOMEM;
> +		if (!bh)
>  			goto failed;
> -		}
>  
>  		if (!buffer_uptodate(bh) && !buffer_locked(bh)) {
>  			bufs[nbufs++] = bh;
> @@ -112,12 +109,10 @@ static int do_readahead(journal_t *journal, unsigned int start)
>  
>  	if (nbufs)
>  		bh_readahead_batch(nbufs, bufs, 0);
> -	err = 0;
>  
>  failed:
>  	if (nbufs)
>  		journal_brelse_array(bufs, nbufs);
> -	return err;
>  }
>  
>  #endif /* __KERNEL__ */
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 4/6] jbd2: remove stale comment of update_t_max_wait

[Jan Kara]

On Wed 25-12-24 04:27:05, Kemeng Shi wrote:
> Commit 2d44292058828 "jbd2: remove CONFIG_JBD2_DEBUG to update t_max_wait"
> removed jbd2_journal_enable_debug, just remove stale comment about
> jbd2_journal_enable_debug.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Sure. Feel free to add:

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

								Honza

> ---
>  fs/jbd2/transaction.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 66513c18ca29..e00b87635512 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -119,7 +119,6 @@ static void jbd2_get_transaction(journal_t *journal,
>   * t_max_wait is carefully updated here with use of atomic compare exchange.
>   * Note that there could be multiplre threads trying to do this simultaneously
>   * hence using cmpxchg to avoid any use of locks in this case.
> - * With this t_max_wait can be updated w/o enabling jbd2_journal_enable_debug.
>   */
>  static inline void update_t_max_wait(transaction_t *transaction,
>  				     unsigned long ts)
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 6/6] jbd2: Correct stale comment of release_buffer_page

[Jan Kara]

On Wed 25-12-24 04:27:07, Kemeng Shi wrote:
> Update stale lock info in comment of release_buffer_page.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/jbd2/commit.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 9153ff3a08e7..d812d15f295e 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -57,8 +57,8 @@ static void journal_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
>   * So here, we have a buffer which has just come off the forget list.  Look to
>   * see if we can strip all buffers from the backing page.
>   *
> - * Called under lock_journal(), and possibly under journal_datalist_lock.  The
> - * caller provided us with a ref against the buffer, and we drop that here.
> + * Called under j_list_lock. The caller provided us with a ref against the
> + * buffer, and we drop that here.
>   */
>  static void release_buffer_page(struct buffer_head *bh)
>  {
> -- 
> 2.30.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/6] jbd2: remove unused h_jdata flag of handle

[Zhang Yi]

On 2024/12/25 4:27, Kemeng Shi wrote:
> Flag h_jdata is not used, just remove it.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  include/linux/jbd2.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 50f7ea8714bf..c7fdb2b1b9a6 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -459,7 +459,6 @@ struct jbd2_revoke_table_s;
>   * @h_ref: Reference count on this handle.
>   * @h_err: Field for caller's use to track errors through large fs operations.
>   * @h_sync: Flag for sync-on-close.
> - * @h_jdata: Flag to force data journaling.
>   * @h_reserved: Flag for handle for reserved credits.
>   * @h_aborted: Flag indicating fatal error on handle.
>   * @h_type: For handle statistics.
> @@ -491,7 +490,6 @@ struct jbd2_journal_handle
>  
>  	/* Flags [no locking] */
>  	unsigned int	h_sync:		1;
> -	unsigned int	h_jdata:	1;
>  	unsigned int	h_reserved:	1;
>  	unsigned int	h_aborted:	1;
>  	unsigned int	h_type:		8;

Re: [PATCH 2/6] jbd2: remove unused return value of jbd2_journal_cancel_revoke

[Zhang Yi]

On 2024/12/25 4:27, Kemeng Shi wrote:
> Remove unused return value of jbd2_journal_cancel_revoke.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/jbd2/revoke.c     | 5 +----
>  include/linux/jbd2.h | 2 +-
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
> index 4556e4689024..af0208ed3619 100644
> --- a/fs/jbd2/revoke.c
> +++ b/fs/jbd2/revoke.c
> @@ -420,12 +420,11 @@ int jbd2_journal_revoke(handle_t *handle, unsigned long long blocknr,
>   * do not trust the Revoked bit on buffers unless RevokeValid is also
>   * set.
>   */
> -int jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
> +void jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
>  {
>  	struct jbd2_revoke_record_s *record;
>  	journal_t *journal = handle->h_transaction->t_journal;
>  	int need_cancel;
> -	int did_revoke = 0;	/* akpm: debug */
>  	struct buffer_head *bh = jh2bh(jh);
>  
>  	jbd2_debug(4, "journal_head %p, cancelling revoke\n", jh);
> @@ -450,7 +449,6 @@ int jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
>  			list_del(&record->hash);
>  			spin_unlock(&journal->j_revoke_lock);
>  			kmem_cache_free(jbd2_revoke_record_cache, record);
> -			did_revoke = 1;
>  		}
>  	}
>  
> @@ -473,7 +471,6 @@ int jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
>  			__brelse(bh2);
>  		}
>  	}
> -	return did_revoke;
>  }
>  
>  /*
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index c7fdb2b1b9a6..e2d1426d3e06 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1635,7 +1635,7 @@ extern int __init jbd2_journal_init_revoke_table_cache(void);
>  
>  extern void	   jbd2_journal_destroy_revoke(journal_t *);
>  extern int	   jbd2_journal_revoke (handle_t *, unsigned long long, struct buffer_head *);
> -extern int	   jbd2_journal_cancel_revoke(handle_t *, struct journal_head *);
> +extern void	   jbd2_journal_cancel_revoke(handle_t *, struct journal_head *);
>  extern void	   jbd2_journal_write_revoke_records(transaction_t *transaction,
>  						     struct list_head *log_bufs);
>  

Re: [PATCH 3/6] jbd2: remove unused return value of do_readahead

[Zhang Yi]

On 2024/12/25 4:27, Kemeng Shi wrote:
> Remove unused return value of do_readahead.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/jbd2/recovery.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/jbd2/recovery.c b/fs/jbd2/recovery.c
> index 9192be7c19d8..a671f8ee7dd2 100644
> --- a/fs/jbd2/recovery.c
> +++ b/fs/jbd2/recovery.c
> @@ -65,9 +65,8 @@ static void journal_brelse_array(struct buffer_head *b[], int n)
>   */
>  
>  #define MAXBUF 8
> -static int do_readahead(journal_t *journal, unsigned int start)
> +static void do_readahead(journal_t *journal, unsigned int start)
>  {
> -	int err;
>  	unsigned int max, nbufs, next;
>  	unsigned long long blocknr;
>  	struct buffer_head *bh;
> @@ -85,7 +84,7 @@ static int do_readahead(journal_t *journal, unsigned int start)
>  	nbufs = 0;
>  
>  	for (next = start; next < max; next++) {
> -		err = jbd2_journal_bmap(journal, next, &blocknr);
> +		int err = jbd2_journal_bmap(journal, next, &blocknr);
>  
>  		if (err) {
>  			printk(KERN_ERR "JBD2: bad block at offset %u\n",
> @@ -94,10 +93,8 @@ static int do_readahead(journal_t *journal, unsigned int start)
>  		}
>  
>  		bh = __getblk(journal->j_dev, blocknr, journal->j_blocksize);
> -		if (!bh) {
> -			err = -ENOMEM;
> +		if (!bh)
>  			goto failed;
> -		}
>  
>  		if (!buffer_uptodate(bh) && !buffer_locked(bh)) {
>  			bufs[nbufs++] = bh;
> @@ -112,12 +109,10 @@ static int do_readahead(journal_t *journal, unsigned int start)
>  
>  	if (nbufs)
>  		bh_readahead_batch(nbufs, bufs, 0);
> -	err = 0;
>  
>  failed:
>  	if (nbufs)
>  		journal_brelse_array(bufs, nbufs);
> -	return err;
>  }
>  
>  #endif /* __KERNEL__ */

Re: [PATCH 4/6] jbd2: remove stale comment of update_t_max_wait

[Zhang Yi]

On 2024/12/25 4:27, Kemeng Shi wrote:
> Commit 2d44292058828 "jbd2: remove CONFIG_JBD2_DEBUG to update t_max_wait"
> removed jbd2_journal_enable_debug, just remove stale comment about
> jbd2_journal_enable_debug.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  fs/jbd2/transaction.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 66513c18ca29..e00b87635512 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -119,7 +119,6 @@ static void jbd2_get_transaction(journal_t *journal,
>   * t_max_wait is carefully updated here with use of atomic compare exchange.
>   * Note that there could be multiplre threads trying to do this simultaneously
>   * hence using cmpxchg to avoid any use of locks in this case.
> - * With this t_max_wait can be updated w/o enabling jbd2_journal_enable_debug.
>   */
>  static inline void update_t_max_wait(transaction_t *transaction,
>  				     unsigned long ts)

Hi, Kemeng,

It seems that the first sentence in this comment should be removed
together, as it also appears staled.

  "Update transaction's maximum wait time, if debugging is enabled."

Thanks,
Yi.

Re: [PATCH 5/6] jbd2: correct stale function name in comment

[Zhang Yi]

On 2024/12/25 4:27, Kemeng Shi wrote:
> Rename stale journal_clear_revoked_flag to jbd2_clear_buffer_revoked_flags.
> Rename stale journal_switch_revoke to jbd2_journal_switch_revoke_table.
> Rename stale __journal_file_buffer to __jbd2_journal_file_buffer.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/jbd2/revoke.c      | 8 ++++----
>  fs/jbd2/transaction.c | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
> index af0208ed3619..5b7350109c5a 100644
> --- a/fs/jbd2/revoke.c
> +++ b/fs/jbd2/revoke.c
> @@ -474,7 +474,7 @@ void jbd2_journal_cancel_revoke(handle_t *handle, struct journal_head *jh)
>  }
>  
>  /*
> - * journal_clear_revoked_flag clears revoked flag of buffers in
> + * jbd2_clear_buffer_revoked_flags clears revoked flag of buffers in
>   * revoke table to reflect there is no revoked buffers in the next
>   * transaction which is going to be started.
>   */
> @@ -503,9 +503,9 @@ void jbd2_clear_buffer_revoked_flags(journal_t *journal)
>  	}
>  }
>  
> -/* journal_switch_revoke table select j_revoke for next transaction
> - * we do not want to suspend any processing until all revokes are
> - * written -bzzz
> +/* jbd2_journal_switch_revoke_table table select j_revoke for next
> + * transaction we do not want to suspend any processing until all
> + * revokes are written -bzzz
>   */
>  void jbd2_journal_switch_revoke_table(journal_t *journal)
>  {
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index e00b87635512..908baf73b188 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -2191,7 +2191,7 @@ static int __dispose_buffer(struct journal_head *jh, transaction_t *transaction)
>  		/*
>  		 * We don't want to write the buffer anymore, clear the
>  		 * bit so that we don't confuse checks in
> -		 * __journal_file_buffer
> +		 * __jbd2_journal_file_buffer
>  		 */
>  		clear_buffer_dirty(bh);
>  		__jbd2_journal_file_buffer(jh, transaction, BJ_Forget);

Re: [PATCH 6/6] jbd2: Correct stale comment of release_buffer_page

[Zhang Yi]

On 2024/12/25 4:27, Kemeng Shi wrote:
> Update stale lock info in comment of release_buffer_page.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/jbd2/commit.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 9153ff3a08e7..d812d15f295e 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -57,8 +57,8 @@ static void journal_end_buffer_io_sync(struct buffer_head *bh, int uptodate)
>   * So here, we have a buffer which has just come off the forget list.  Look to
>   * see if we can strip all buffers from the backing page.
>   *
> - * Called under lock_journal(), and possibly under journal_datalist_lock.  The
> - * caller provided us with a ref against the buffer, and we drop that here.
> + * Called under j_list_lock. The caller provided us with a ref against the
> + * buffer, and we drop that here.
>   */
>  static void release_buffer_page(struct buffer_head *bh)
>  {