Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1597854 > unrolled thread

[PATCH 4.4 49/91] ext4: preserve the needs_recovery flag when the journal is aborted

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4.4 49/91] ext4: preserve the needs_recovery flag when the journal is aborted Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-10 15:40 +0100
    Re: [PATCH 4.4 49/91] ext4: preserve the needs_recovery flag when  the journal is aborted Ben Hutchings <ben@decadent.org.uk> - 2017-03-10 18:00 +0100
      Re: [PATCH 4.4 49/91] ext4: preserve the needs_recovery flag when  the journal is aborted Theodore Ts'o <tytso@mit.edu> - 2017-03-10 21:20 +0100
        Re: [PATCH 4.4 49/91] ext4: preserve the needs_recovery flag when  the journal is aborted Ben Hutchings <ben@decadent.org.uk> - 2017-03-11 06:30 +0100

#1597854 — [PATCH 4.4 49/91] ext4: preserve the needs_recovery flag when the journal is aborted

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Theodore Ts'o <tytso@mit.edu>

commit 97abd7d4b5d9c48ec15c425485f054e1c15e591b upstream.

If the journal is aborted, the needs_recovery feature flag should not
be removed.  Otherwise, it's the journal might not get replayed and
this could lead to more data getting lost.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/ext4/super.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -793,6 +793,7 @@ static void ext4_put_super(struct super_
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct ext4_super_block *es = sbi->s_es;
+	int aborted = 0;
 	int i, err;
 
 	ext4_unregister_li_request(sb);
@@ -802,9 +803,10 @@ static void ext4_put_super(struct super_
 	destroy_workqueue(sbi->rsv_conversion_wq);
 
 	if (sbi->s_journal) {
+		aborted = is_journal_aborted(sbi->s_journal);
 		err = jbd2_journal_destroy(sbi->s_journal);
 		sbi->s_journal = NULL;
-		if (err < 0)
+		if ((err < 0) && !aborted)
 			ext4_abort(sb, "Couldn't clean up the journal");
 	}
 
@@ -816,7 +818,7 @@ static void ext4_put_super(struct super_
 	ext4_ext_release(sb);
 	ext4_xattr_put_super(sb);
 
-	if (!(sb->s_flags & MS_RDONLY)) {
+	if (!(sb->s_flags & MS_RDONLY) && !aborted) {
 		ext4_clear_feature_journal_needs_recovery(sb);
 		es->s_state = cpu_to_le16(sbi->s_mount_state);
 	}

[toc] | [next] | [standalone]

#1597959 — Re: [PATCH 4.4 49/91] ext4: preserve the needs_recovery flag when the journal is aborted

[Multipart message — attachments visible in raw view] — view raw

On Fri, 2017-03-10 at 10:08 +0100, Greg Kroah-Hartman wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Theodore Ts'o <tytso@mit.edu>
> 
> commit 97abd7d4b5d9c48ec15c425485f054e1c15e591b upstream.
> 
> If the journal is aborted, the needs_recovery feature flag should not
> be removed.  Otherwise, it's the journal might not get replayed and
> this could lead to more data getting lost.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
>  fs/ext4/super.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
[...]
> @@ -802,9 +803,10 @@ static void ext4_put_super(struct super_
>  	destroy_workqueue(sbi->rsv_conversion_wq);
>  
>  	if (sbi->s_journal) {
> +		aborted = is_journal_aborted(sbi->s_journal);
>  		err = jbd2_journal_destroy(sbi->s_journal);
>  		sbi->s_journal = NULL;
> -		if (err < 0)
> +		if ((err < 0) && !aborted)
>  			ext4_abort(sb, "Couldn't clean up the journal");
[...]

Shouldn't the aborted flag also be set here when err < 0?

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.

[toc] | [prev] | [next] | [standalone]

#1598065 — Re: [PATCH 4.4 49/91] ext4: preserve the needs_recovery flag when the journal is aborted

On Fri, Mar 10, 2017 at 04:58:02PM +0000, Ben Hutchings wrote:
> > ---
> >  fs/ext4/super.c |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> [...]
> > @@ -802,9 +803,10 @@ static void ext4_put_super(struct super_
> >  	destroy_workqueue(sbi->rsv_conversion_wq);
> >  
> >  	if (sbi->s_journal) {
> > +		aborted = is_journal_aborted(sbi->s_journal);
> >  		err = jbd2_journal_destroy(sbi->s_journal);
> >  		sbi->s_journal = NULL;
> > -		if (err < 0)
> > +		if ((err < 0) && !aborted)
> >  			ext4_abort(sb, "Couldn't clean up the journal");
> [...]
> 
> Shouldn't the aborted flag also be set here when err < 0?

Nice catch.  That's a separate issue (the bug was there before this
commit), though I'll send a separate patch to fix this in mainline and
then cc stable, OK?

						- Ted

[toc] | [prev] | [next] | [standalone]

#1598258 — Re: [PATCH 4.4 49/91] ext4: preserve the needs_recovery flag when the journal is aborted

[Multipart message — attachments visible in raw view] — view raw

On Fri, 2017-03-10 at 15:14 -0500, Theodore Ts'o wrote:
> On Fri, Mar 10, 2017 at 04:58:02PM +0000, Ben Hutchings wrote:
> > > ---
> > >  fs/ext4/super.c |    6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > --- a/fs/ext4/super.c
> > > +++ b/fs/ext4/super.c
> > 
> > [...]
> > > @@ -802,9 +803,10 @@ static void ext4_put_super(struct super_
> > >  	destroy_workqueue(sbi->rsv_conversion_wq);
> > >  
> > >  	if (sbi->s_journal) {
> > > +		aborted = is_journal_aborted(sbi->s_journal);
> > >  		err = jbd2_journal_destroy(sbi->s_journal);
> > >  		sbi->s_journal = NULL;
> > > -		if (err < 0)
> > > +		if ((err < 0) && !aborted)
> > >  			ext4_abort(sb, "Couldn't clean up the journal");
> > 
> > [...]
> > 
> > Shouldn't the aborted flag also be set here when err < 0?
> 
> Nice catch.  That's a separate issue (the bug was there before this
> commit), though I'll send a separate patch to fix this in mainline and
> then cc stable, OK?

Sure, that's not an objection to including this patch in stable now.

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web