| Message ID | 20230410172039.1752440-1-u202112087@hust.edu.cn |
|---|---|
| State | New |
| Headers | show |
| Series | fs: jbd2: fix an incorrect warn log | expand |
On Tue, Apr 11, 2023 at 01:20:38AM +0800, Guoqing Cai wrote: > In jbd2_journal_load(), when journal_reset fails, it prints an incorrect > warn log. > > Fix this by changing the goto statement to return statement. > > Signed-off-by: Guoqing Cai <u202112087@hust.edu.cn> > Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn> > --- > fs/jbd2/journal.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c > index e80c781731f8..555f09ca1d99 100644 > --- a/fs/jbd2/journal.c > +++ b/fs/jbd2/journal.c > @@ -2082,8 +2082,10 @@ int jbd2_journal_load(journal_t *journal) > > /* Let the recovery code check whether it needs to recover any > * data from the journal. */ > - if (jbd2_journal_recover(journal)) > - goto recovery_error; > + if (jbd2_journal_recover(journal)) { > + printk(KERN_WARNING "JBD2: recovery failed\n"); > + return -EIO; > + } > > if (journal->j_failed_commit) { > printk(KERN_ERR "JBD2: journal transaction %u on %s " > @@ -2101,14 +2103,10 @@ int jbd2_journal_load(journal_t *journal) > * reinitialise the dynamic contents of the superblock in memory > * and reset them on disk. */ > if (journal_reset(journal)) > - goto recovery_error; > + return -EIO; Why wouldn't you warn about the journal reset failing here? --D > > journal->j_flags |= JBD2_LOADED; > return 0; > - > -recovery_error: > - printk(KERN_WARNING "JBD2: recovery failed\n"); > - return -EIO; > } > > /** > -- > 2.40.0 >
On Tue 11-04-23 01:20:38, Guoqing Cai wrote: > In jbd2_journal_load(), when journal_reset fails, it prints an incorrect > warn log. > > Fix this by changing the goto statement to return statement. > > Signed-off-by: Guoqing Cai <u202112087@hust.edu.cn> > Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn> ... > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c > index e80c781731f8..555f09ca1d99 100644 > --- a/fs/jbd2/journal.c > +++ b/fs/jbd2/journal.c > @@ -2082,8 +2082,10 @@ int jbd2_journal_load(journal_t *journal) > > /* Let the recovery code check whether it needs to recover any > * data from the journal. */ > - if (jbd2_journal_recover(journal)) > - goto recovery_error; > + if (jbd2_journal_recover(journal)) { > + printk(KERN_WARNING "JBD2: recovery failed\n"); > + return -EIO; > + } > > if (journal->j_failed_commit) { > printk(KERN_ERR "JBD2: journal transaction %u on %s " > @@ -2101,14 +2103,10 @@ int jbd2_journal_load(journal_t *journal) > * reinitialise the dynamic contents of the superblock in memory > * and reset them on disk. */ > if (journal_reset(journal)) > - goto recovery_error; > + return -EIO; So when you touch this code it would make more sense to actually return the error returned by journal_reset() - that is most definitely not going to be EIO but most likely ENOMEM. Honza
On Mon 10-04-23 10:45:45, Darrick J. Wong wrote: > On Tue, Apr 11, 2023 at 01:20:38AM +0800, Guoqing Cai wrote: > > In jbd2_journal_load(), when journal_reset fails, it prints an incorrect > > warn log. > > > > Fix this by changing the goto statement to return statement. > > > > Signed-off-by: Guoqing Cai <u202112087@hust.edu.cn> > > Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn> > > --- > > fs/jbd2/journal.c | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c > > index e80c781731f8..555f09ca1d99 100644 > > --- a/fs/jbd2/journal.c > > +++ b/fs/jbd2/journal.c > > @@ -2082,8 +2082,10 @@ int jbd2_journal_load(journal_t *journal) > > > > /* Let the recovery code check whether it needs to recover any > > * data from the journal. */ > > - if (jbd2_journal_recover(journal)) > > - goto recovery_error; > > + if (jbd2_journal_recover(journal)) { > > + printk(KERN_WARNING "JBD2: recovery failed\n"); > > + return -EIO; > > + } > > > > if (journal->j_failed_commit) { > > printk(KERN_ERR "JBD2: journal transaction %u on %s " > > @@ -2101,14 +2103,10 @@ int jbd2_journal_load(journal_t *journal) > > * reinitialise the dynamic contents of the superblock in memory > > * and reset them on disk. */ > > if (journal_reset(journal)) > > - goto recovery_error; > > + return -EIO; > > Why wouldn't you warn about the journal reset failing here? Well, ext4 will still print: ext4_msg(sb, KERN_ERR, "error loading journal"); which I guess is good enough for such strange failure. Honza
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c index e80c781731f8..555f09ca1d99 100644 --- a/fs/jbd2/journal.c +++ b/fs/jbd2/journal.c @@ -2082,8 +2082,10 @@ int jbd2_journal_load(journal_t *journal) /* Let the recovery code check whether it needs to recover any * data from the journal. */ - if (jbd2_journal_recover(journal)) - goto recovery_error; + if (jbd2_journal_recover(journal)) { + printk(KERN_WARNING "JBD2: recovery failed\n"); + return -EIO; + } if (journal->j_failed_commit) { printk(KERN_ERR "JBD2: journal transaction %u on %s " @@ -2101,14 +2103,10 @@ int jbd2_journal_load(journal_t *journal) * reinitialise the dynamic contents of the superblock in memory * and reset them on disk. */ if (journal_reset(journal)) - goto recovery_error; + return -EIO; journal->j_flags |= JBD2_LOADED; return 0; - -recovery_error: - printk(KERN_WARNING "JBD2: recovery failed\n"); - return -EIO; } /**