[PATCH v2 1/3] ext4: define ext4_journal_destroy wrapper

Ojaswin Mujoo posted 3 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v2 1/3] ext4: define ext4_journal_destroy wrapper
Posted by Ojaswin Mujoo 11 months, 1 week ago
Define an ext4 wrapper over jbd2_journal_destroy to make sure we
have consistent behavior during journal destruction. This will also
come useful in the next patch where we add some ext4 specific logic
in the destroy path.

Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/ext4_jbd2.h | 14 ++++++++++++++
 fs/ext4/super.c     | 16 ++++++----------
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
index 3f2596c9e5f2..9b3c9df02a39 100644
--- a/fs/ext4/ext4_jbd2.h
+++ b/fs/ext4/ext4_jbd2.h
@@ -429,4 +429,18 @@ static inline int ext4_should_dioread_nolock(struct inode *inode)
 	return 1;
 }
 
+/*
+ * Pass journal explicitly as it may not be cached in the sbi->s_journal in some
+ * cases
+ */
+static inline int ext4_journal_destroy(struct ext4_sb_info *sbi, journal_t *journal)
+{
+	int err = 0;
+
+	err = jbd2_journal_destroy(journal);
+	sbi->s_journal = NULL;
+
+	return err;
+}
+
 #endif	/* _EXT4_JBD2_H */
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index a963ffda692a..8ad664d47806 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1297,8 +1297,7 @@ static void ext4_put_super(struct super_block *sb)
 
 	if (sbi->s_journal) {
 		aborted = is_journal_aborted(sbi->s_journal);
-		err = jbd2_journal_destroy(sbi->s_journal);
-		sbi->s_journal = NULL;
+		err = ext4_journal_destroy(sbi, sbi->s_journal);
 		if ((err < 0) && !aborted) {
 			ext4_abort(sb, -err, "Couldn't clean up the journal");
 		}
@@ -4960,8 +4959,7 @@ static int ext4_load_and_init_journal(struct super_block *sb,
 out:
 	/* flush s_sb_upd_work before destroying the journal. */
 	flush_work(&sbi->s_sb_upd_work);
-	jbd2_journal_destroy(sbi->s_journal);
-	sbi->s_journal = NULL;
+	ext4_journal_destroy(sbi, sbi->s_journal);
 	return -EINVAL;
 }
 
@@ -5652,8 +5650,7 @@ failed_mount8: __maybe_unused
 	if (sbi->s_journal) {
 		/* flush s_sb_upd_work before journal destroy. */
 		flush_work(&sbi->s_sb_upd_work);
-		jbd2_journal_destroy(sbi->s_journal);
-		sbi->s_journal = NULL;
+		ext4_journal_destroy(sbi, sbi->s_journal);
 	}
 failed_mount3a:
 	ext4_es_unregister_shrinker(sbi);
@@ -5958,7 +5955,7 @@ static journal_t *ext4_open_dev_journal(struct super_block *sb,
 	return journal;
 
 out_journal:
-	jbd2_journal_destroy(journal);
+	ext4_journal_destroy(EXT4_SB(sb), journal);
 out_bdev:
 	bdev_fput(bdev_file);
 	return ERR_PTR(errno);
@@ -6075,8 +6072,7 @@ static int ext4_load_journal(struct super_block *sb,
 	EXT4_SB(sb)->s_journal = journal;
 	err = ext4_clear_journal_err(sb, es);
 	if (err) {
-		EXT4_SB(sb)->s_journal = NULL;
-		jbd2_journal_destroy(journal);
+		ext4_journal_destroy(EXT4_SB(sb), journal);
 		return err;
 	}
 
@@ -6094,7 +6090,7 @@ static int ext4_load_journal(struct super_block *sb,
 	return 0;
 
 err_out:
-	jbd2_journal_destroy(journal);
+	ext4_journal_destroy(EXT4_SB(sb), journal);
 	return err;
 }
 
-- 
2.48.1
Re: [PATCH v2 1/3] ext4: define ext4_journal_destroy wrapper
Posted by Baokun Li 11 months ago
On 2025/3/6 22:28, Ojaswin Mujoo wrote:
> Define an ext4 wrapper over jbd2_journal_destroy to make sure we
> have consistent behavior during journal destruction. This will also
> come useful in the next patch where we add some ext4 specific logic
> in the destroy path.
>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Looks good to me.

Reviewed-by: Baokun Li <libaokun1@huawei.com>
> ---
>   fs/ext4/ext4_jbd2.h | 14 ++++++++++++++
>   fs/ext4/super.c     | 16 ++++++----------
>   2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index 3f2596c9e5f2..9b3c9df02a39 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
> @@ -429,4 +429,18 @@ static inline int ext4_should_dioread_nolock(struct inode *inode)
>   	return 1;
>   }
>   
> +/*
> + * Pass journal explicitly as it may not be cached in the sbi->s_journal in some
> + * cases
> + */
> +static inline int ext4_journal_destroy(struct ext4_sb_info *sbi, journal_t *journal)
> +{
> +	int err = 0;
> +
> +	err = jbd2_journal_destroy(journal);
> +	sbi->s_journal = NULL;
> +
> +	return err;
> +}
> +
>   #endif	/* _EXT4_JBD2_H */
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index a963ffda692a..8ad664d47806 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1297,8 +1297,7 @@ static void ext4_put_super(struct super_block *sb)
>   
>   	if (sbi->s_journal) {
>   		aborted = is_journal_aborted(sbi->s_journal);
> -		err = jbd2_journal_destroy(sbi->s_journal);
> -		sbi->s_journal = NULL;
> +		err = ext4_journal_destroy(sbi, sbi->s_journal);
>   		if ((err < 0) && !aborted) {
>   			ext4_abort(sb, -err, "Couldn't clean up the journal");
>   		}
> @@ -4960,8 +4959,7 @@ static int ext4_load_and_init_journal(struct super_block *sb,
>   out:
>   	/* flush s_sb_upd_work before destroying the journal. */
>   	flush_work(&sbi->s_sb_upd_work);
> -	jbd2_journal_destroy(sbi->s_journal);
> -	sbi->s_journal = NULL;
> +	ext4_journal_destroy(sbi, sbi->s_journal);
>   	return -EINVAL;
>   }
>   
> @@ -5652,8 +5650,7 @@ failed_mount8: __maybe_unused
>   	if (sbi->s_journal) {
>   		/* flush s_sb_upd_work before journal destroy. */
>   		flush_work(&sbi->s_sb_upd_work);
> -		jbd2_journal_destroy(sbi->s_journal);
> -		sbi->s_journal = NULL;
> +		ext4_journal_destroy(sbi, sbi->s_journal);
>   	}
>   failed_mount3a:
>   	ext4_es_unregister_shrinker(sbi);
> @@ -5958,7 +5955,7 @@ static journal_t *ext4_open_dev_journal(struct super_block *sb,
>   	return journal;
>   
>   out_journal:
> -	jbd2_journal_destroy(journal);
> +	ext4_journal_destroy(EXT4_SB(sb), journal);
>   out_bdev:
>   	bdev_fput(bdev_file);
>   	return ERR_PTR(errno);
> @@ -6075,8 +6072,7 @@ static int ext4_load_journal(struct super_block *sb,
>   	EXT4_SB(sb)->s_journal = journal;
>   	err = ext4_clear_journal_err(sb, es);
>   	if (err) {
> -		EXT4_SB(sb)->s_journal = NULL;
> -		jbd2_journal_destroy(journal);
> +		ext4_journal_destroy(EXT4_SB(sb), journal);
>   		return err;
>   	}
>   
> @@ -6094,7 +6090,7 @@ static int ext4_load_journal(struct super_block *sb,
>   	return 0;
>   
>   err_out:
> -	jbd2_journal_destroy(journal);
> +	ext4_journal_destroy(EXT4_SB(sb), journal);
>   	return err;
>   }
>
Re: [PATCH v2 1/3] ext4: define ext4_journal_destroy wrapper
Posted by Jan Kara 11 months, 1 week ago
On Thu 06-03-25 19:58:32, Ojaswin Mujoo wrote:
> Define an ext4 wrapper over jbd2_journal_destroy to make sure we
> have consistent behavior during journal destruction. This will also
> come useful in the next patch where we add some ext4 specific logic
> in the destroy path.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/ext4/ext4_jbd2.h | 14 ++++++++++++++
>  fs/ext4/super.c     | 16 ++++++----------
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
> index 3f2596c9e5f2..9b3c9df02a39 100644
> --- a/fs/ext4/ext4_jbd2.h
> +++ b/fs/ext4/ext4_jbd2.h
> @@ -429,4 +429,18 @@ static inline int ext4_should_dioread_nolock(struct inode *inode)
>  	return 1;
>  }
>  
> +/*
> + * Pass journal explicitly as it may not be cached in the sbi->s_journal in some
> + * cases
> + */
> +static inline int ext4_journal_destroy(struct ext4_sb_info *sbi, journal_t *journal)
> +{
> +	int err = 0;
> +
> +	err = jbd2_journal_destroy(journal);
> +	sbi->s_journal = NULL;
> +
> +	return err;
> +}
> +
>  #endif	/* _EXT4_JBD2_H */
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index a963ffda692a..8ad664d47806 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1297,8 +1297,7 @@ static void ext4_put_super(struct super_block *sb)
>  
>  	if (sbi->s_journal) {
>  		aborted = is_journal_aborted(sbi->s_journal);
> -		err = jbd2_journal_destroy(sbi->s_journal);
> -		sbi->s_journal = NULL;
> +		err = ext4_journal_destroy(sbi, sbi->s_journal);
>  		if ((err < 0) && !aborted) {
>  			ext4_abort(sb, -err, "Couldn't clean up the journal");
>  		}
> @@ -4960,8 +4959,7 @@ static int ext4_load_and_init_journal(struct super_block *sb,
>  out:
>  	/* flush s_sb_upd_work before destroying the journal. */
>  	flush_work(&sbi->s_sb_upd_work);
> -	jbd2_journal_destroy(sbi->s_journal);
> -	sbi->s_journal = NULL;
> +	ext4_journal_destroy(sbi, sbi->s_journal);
>  	return -EINVAL;
>  }
>  
> @@ -5652,8 +5650,7 @@ failed_mount8: __maybe_unused
>  	if (sbi->s_journal) {
>  		/* flush s_sb_upd_work before journal destroy. */
>  		flush_work(&sbi->s_sb_upd_work);
> -		jbd2_journal_destroy(sbi->s_journal);
> -		sbi->s_journal = NULL;
> +		ext4_journal_destroy(sbi, sbi->s_journal);
>  	}
>  failed_mount3a:
>  	ext4_es_unregister_shrinker(sbi);
> @@ -5958,7 +5955,7 @@ static journal_t *ext4_open_dev_journal(struct super_block *sb,
>  	return journal;
>  
>  out_journal:
> -	jbd2_journal_destroy(journal);
> +	ext4_journal_destroy(EXT4_SB(sb), journal);
>  out_bdev:
>  	bdev_fput(bdev_file);
>  	return ERR_PTR(errno);
> @@ -6075,8 +6072,7 @@ static int ext4_load_journal(struct super_block *sb,
>  	EXT4_SB(sb)->s_journal = journal;
>  	err = ext4_clear_journal_err(sb, es);
>  	if (err) {
> -		EXT4_SB(sb)->s_journal = NULL;
> -		jbd2_journal_destroy(journal);
> +		ext4_journal_destroy(EXT4_SB(sb), journal);
>  		return err;
>  	}
>  
> @@ -6094,7 +6090,7 @@ static int ext4_load_journal(struct super_block *sb,
>  	return 0;
>  
>  err_out:
> -	jbd2_journal_destroy(journal);
> +	ext4_journal_destroy(EXT4_SB(sb), journal);
>  	return err;
>  }
>  
> -- 
> 2.48.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR