From: Andreas Dilger <adilger@dilger.ca> To: Jan Kara <jack@suse.cz> Cc: Ted Tso <tytso@mit.edu>, linux-ext4@vger.kernel.org Subject: Re: [PATCH 04/12] ext4: Make ext4_abort() use __ext4_error() Date: Sun, 29 Nov 2020 15:12:38 -0700 [thread overview] Message-ID: <75D9FAE2-6CCC-49A7-8D29-0FDC197C96E8@dilger.ca> (raw) In-Reply-To: <20201127113405.26867-5-jack@suse.cz> [-- Attachment #1: Type: text/plain, Size: 10804 bytes --] On Nov 27, 2020, at 4:33 AM, Jan Kara <jack@suse.cz> wrote: > > The only difference between __ext4_abort() and __ext4_error() is that > the former one ignores errors=continue mount option. Unify the code to > reduce duplication. > > Signed-off-by: Jan Kara <jack@suse.cz> Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > fs/ext4/ext4.h | 29 ++++++++---------- > fs/ext4/ext4_jbd2.c | 4 +-- > fs/ext4/inode.c | 2 +- > fs/ext4/super.c | 84 ++++++++++++++--------------------------------------- > 4 files changed, 37 insertions(+), 82 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 65ecaf96d0a4..e67291c4a10b 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -2952,9 +2952,9 @@ extern void ext4_mark_group_bitmap_corrupted(struct super_block *sb, > ext4_group_t block_group, > unsigned int flags); > > -extern __printf(6, 7) > -void __ext4_error(struct super_block *, const char *, unsigned int, int, __u64, > - const char *, ...); > +extern __printf(7, 8) > +void __ext4_error(struct super_block *, const char *, unsigned int, bool, > + int, __u64, const char *, ...); > extern __printf(6, 7) > void __ext4_error_inode(struct inode *, const char *, unsigned int, > ext4_fsblk_t, int, const char *, ...); > @@ -2963,9 +2963,6 @@ void __ext4_error_file(struct file *, const char *, unsigned int, ext4_fsblk_t, > const char *, ...); > extern void __ext4_std_error(struct super_block *, const char *, > unsigned int, int); > -extern __printf(5, 6) > -void __ext4_abort(struct super_block *, const char *, unsigned int, int, > - const char *, ...); > extern __printf(4, 5) > void __ext4_warning(struct super_block *, const char *, unsigned int, > const char *, ...); > @@ -2995,6 +2992,9 @@ void __ext4_grp_locked_error(const char *, unsigned int, > #define EXT4_ERROR_FILE(file, block, fmt, a...) \ > ext4_error_file((file), __func__, __LINE__, (block), (fmt), ## a) > > +#define ext4_abort(sb, err, fmt, a...) \ > + __ext4_error((sb), __func__, __LINE__, true, (err), 0, (fmt), ## a) > + > #ifdef CONFIG_PRINTK > > #define ext4_error_inode(inode, func, line, block, fmt, ...) \ > @@ -3005,11 +3005,11 @@ void __ext4_grp_locked_error(const char *, unsigned int, > #define ext4_error_file(file, func, line, block, fmt, ...) \ > __ext4_error_file(file, func, line, block, fmt, ##__VA_ARGS__) > #define ext4_error(sb, fmt, ...) \ > - __ext4_error((sb), __func__, __LINE__, 0, 0, (fmt), ##__VA_ARGS__) > + __ext4_error((sb), __func__, __LINE__, false, 0, 0, (fmt), \ > + ##__VA_ARGS__) > #define ext4_error_err(sb, err, fmt, ...) \ > - __ext4_error((sb), __func__, __LINE__, (err), 0, (fmt), ##__VA_ARGS__) > -#define ext4_abort(sb, err, fmt, ...) \ > - __ext4_abort((sb), __func__, __LINE__, (err), (fmt), ##__VA_ARGS__) > + __ext4_error((sb), __func__, __LINE__, false, (err), 0, (fmt), \ > + ##__VA_ARGS__) > #define ext4_warning(sb, fmt, ...) \ > __ext4_warning(sb, __func__, __LINE__, fmt, ##__VA_ARGS__) > #define ext4_warning_inode(inode, fmt, ...) \ > @@ -3042,17 +3042,12 @@ do { \ > #define ext4_error(sb, fmt, ...) \ > do { \ > no_printk(fmt, ##__VA_ARGS__); \ > - __ext4_error(sb, "", 0, 0, 0, " "); \ > + __ext4_error(sb, "", 0, false, 0, 0, " "); \ > } while (0) > #define ext4_error_err(sb, err, fmt, ...) \ > do { \ > no_printk(fmt, ##__VA_ARGS__); \ > - __ext4_error(sb, "", 0, err, 0, " "); \ > -} while (0) > -#define ext4_abort(sb, err, fmt, ...) \ > -do { \ > - no_printk(fmt, ##__VA_ARGS__); \ > - __ext4_abort(sb, "", 0, err, " "); \ > + __ext4_error(sb, "", 0, false, err, 0, " "); \ > } while (0) > #define ext4_warning(sb, fmt, ...) \ > do { \ > diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c > index 0fd0c42a4f7d..1a0a827a7f34 100644 > --- a/fs/ext4/ext4_jbd2.c > +++ b/fs/ext4/ext4_jbd2.c > @@ -296,8 +296,8 @@ int __ext4_forget(const char *where, unsigned int line, handle_t *handle, > if (err) { > ext4_journal_abort_handle(where, line, __func__, > bh, handle, err); > - __ext4_abort(inode->i_sb, where, line, -err, > - "error %d when attempting revoke", err); > + __ext4_error(inode->i_sb, where, line, true, -err, 0, > + "error %d when attempting revoke", err); > } > BUFFER_TRACE(bh, "exit"); > return err; > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 0d8385aea898..3a39fa0d6a3a 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -4610,7 +4610,7 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino, > (ino > le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count))) { > if (flags & EXT4_IGET_HANDLE) > return ERR_PTR(-ESTALE); > - __ext4_error(sb, function, line, EFSCORRUPTED, 0, > + __ext4_error(sb, function, line, false, EFSCORRUPTED, 0, > "inode #%lu: comm %s: iget: illegal inode #", > ino, current->comm); > return ERR_PTR(-EFSCORRUPTED); > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 61e6e5f156f3..dddaadc55475 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -662,16 +662,21 @@ static bool system_going_down(void) > * We'll just use the jbd2_journal_abort() error code to record an error in > * the journal instead. On recovery, the journal will complain about > * that error until we've noted it down and cleared it. > + * > + * If force_ro is set, we unconditionally force the filesystem into an > + * ABORT|READONLY state, unless the error response on the fs has been set to > + * panic in which case we take the easy way out and panic immediately. This is > + * used to deal with unrecoverable failures such as journal IO errors or ENOMEM > + * at a critical moment in log management. > */ > - > -static void ext4_handle_error(struct super_block *sb) > +static void ext4_handle_error(struct super_block *sb, bool force_ro) > { > journal_t *journal = EXT4_SB(sb)->s_journal; > > if (test_opt(sb, WARN_ON_ERROR)) > WARN_ON_ONCE(1); > > - if (sb_rdonly(sb) || test_opt(sb, ERRORS_CONT)) > + if (sb_rdonly(sb) || (!force_ro && test_opt(sb, ERRORS_CONT))) > return; > > ext4_set_mount_flag(sb, EXT4_MF_FS_ABORTED); > @@ -682,18 +687,17 @@ static void ext4_handle_error(struct super_block *sb) > * could panic during 'reboot -f' as the underlying device got already > * disabled. > */ > - if (test_opt(sb, ERRORS_RO) || system_going_down()) { > - ext4_msg(sb, KERN_CRIT, "Remounting filesystem read-only"); > - /* > - * Make sure updated value of ->s_mount_flags will be visible > - * before ->s_flags update > - */ > - smp_wmb(); > - sb->s_flags |= SB_RDONLY; > - } else if (test_opt(sb, ERRORS_PANIC)) { > + if (test_opt(sb, ERRORS_PANIC) && !system_going_down()) { > panic("EXT4-fs (device %s): panic forced after error\n", > sb->s_id); > } > + ext4_msg(sb, KERN_CRIT, "Remounting filesystem read-only"); > + /* > + * Make sure updated value of ->s_mount_flags will be visible before > + * ->s_flags update > + */ > + smp_wmb(); > + sb->s_flags |= SB_RDONLY; > } > > #define ext4_error_ratelimit(sb) \ > @@ -701,7 +705,7 @@ static void ext4_handle_error(struct super_block *sb) > "EXT4-fs error") > > void __ext4_error(struct super_block *sb, const char *function, > - unsigned int line, int error, __u64 block, > + unsigned int line, bool force_ro, int error, __u64 block, > const char *fmt, ...) > { > struct va_format vaf; > @@ -721,7 +725,7 @@ void __ext4_error(struct super_block *sb, const char *function, > va_end(args); > } > save_error_info(sb, error, 0, block, function, line); > - ext4_handle_error(sb); > + ext4_handle_error(sb, force_ro); > } > > void __ext4_error_inode(struct inode *inode, const char *function, > @@ -753,7 +757,7 @@ void __ext4_error_inode(struct inode *inode, const char *function, > } > save_error_info(inode->i_sb, error, inode->i_ino, block, > function, line); > - ext4_handle_error(inode->i_sb); > + ext4_handle_error(inode->i_sb, false); > } > > void __ext4_error_file(struct file *file, const char *function, > @@ -792,7 +796,7 @@ void __ext4_error_file(struct file *file, const char *function, > } > save_error_info(inode->i_sb, EFSCORRUPTED, inode->i_ino, block, > function, line); > - ext4_handle_error(inode->i_sb); > + ext4_handle_error(inode->i_sb, false); > } > > const char *ext4_decode_error(struct super_block *sb, int errno, > @@ -860,51 +864,7 @@ void __ext4_std_error(struct super_block *sb, const char *function, > } > > save_error_info(sb, -errno, 0, 0, function, line); > - ext4_handle_error(sb); > -} > - > -/* > - * ext4_abort is a much stronger failure handler than ext4_error. The > - * abort function may be used to deal with unrecoverable failures such > - * as journal IO errors or ENOMEM at a critical moment in log management. > - * > - * We unconditionally force the filesystem into an ABORT|READONLY state, > - * unless the error response on the fs has been set to panic in which > - * case we take the easy way out and panic immediately. > - */ > - > -void __ext4_abort(struct super_block *sb, const char *function, > - unsigned int line, int error, const char *fmt, ...) > -{ > - struct va_format vaf; > - va_list args; > - > - if (unlikely(ext4_forced_shutdown(EXT4_SB(sb)))) > - return; > - > - save_error_info(sb, error, 0, 0, function, line); > - va_start(args, fmt); > - vaf.fmt = fmt; > - vaf.va = &args; > - printk(KERN_CRIT "EXT4-fs error (device %s): %s:%d: %pV\n", > - sb->s_id, function, line, &vaf); > - va_end(args); > - > - if (sb_rdonly(sb) == 0) { > - ext4_set_mount_flag(sb, EXT4_MF_FS_ABORTED); > - if (EXT4_SB(sb)->s_journal) > - jbd2_journal_abort(EXT4_SB(sb)->s_journal, -EIO); > - > - ext4_msg(sb, KERN_CRIT, "Remounting filesystem read-only"); > - /* > - * Make sure updated value of ->s_mount_flags will be visible > - * before ->s_flags update > - */ > - smp_wmb(); > - sb->s_flags |= SB_RDONLY; > - } > - if (test_opt(sb, ERRORS_PANIC) && !system_going_down()) > - panic("EXT4-fs panic from previous error\n"); > + ext4_handle_error(sb, false); > } > > void __ext4_msg(struct super_block *sb, > @@ -1007,7 +967,7 @@ __acquires(bitlock) > > ext4_unlock_group(sb, grp); > ext4_commit_super(sb, 1); > - ext4_handle_error(sb); > + ext4_handle_error(sb, false); > /* > * We only get here in the ERRORS_RO case; relocking the group > * may be dangerous, but nothing bad will happen since the > -- > 2.16.4 > Cheers, Andreas [-- Attachment #2: Message signed with OpenPGP --] [-- Type: application/pgp-signature, Size: 873 bytes --]
next prev parent reply other threads:[~2020-11-29 22:13 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-27 11:33 [PATCH 00/12] ext4: Various fixes of ext4 handling of fs errors Jan Kara 2020-11-27 11:33 ` [PATCH 01/12] ext4: Don't remount read-only with errors=continue on reboot Jan Kara 2020-11-29 21:33 ` Andreas Dilger 2020-12-16 4:59 ` Theodore Y. Ts'o 2020-11-27 11:33 ` [PATCH 02/12] ext4: Remove redundant sb checksum recomputation Jan Kara 2020-11-29 22:11 ` Andreas Dilger 2020-11-30 10:59 ` Jan Kara 2020-12-16 5:01 ` Theodore Y. Ts'o 2020-11-27 11:33 ` [PATCH 03/12] ext4: Standardize error message in ext4_protect_reserved_inode() Jan Kara 2020-11-29 21:39 ` Andreas Dilger 2020-12-16 5:04 ` Theodore Y. Ts'o 2020-11-27 11:33 ` [PATCH 04/12] ext4: Make ext4_abort() use __ext4_error() Jan Kara 2020-11-29 22:12 ` Andreas Dilger [this message] 2020-12-16 5:07 ` Theodore Y. Ts'o 2020-11-27 11:33 ` [PATCH 05/12] ext4: Move functions in super.c Jan Kara 2020-11-29 22:13 ` Andreas Dilger 2020-12-16 5:09 ` Theodore Y. Ts'o 2020-11-27 11:33 ` [PATCH 06/12] ext4: Simplify ext4 error translation Jan Kara 2020-11-29 21:57 ` Andreas Dilger 2020-12-16 5:11 ` Theodore Y. Ts'o 2020-11-27 11:34 ` [PATCH 07/12] ext4: Defer saving error info from atomic context Jan Kara 2020-12-16 5:31 ` Theodore Y. Ts'o 2020-12-16 5:40 ` Theodore Y. Ts'o 2020-12-16 9:56 ` Jan Kara 2020-11-27 11:34 ` [PATCH 08/12] ext4: Combine ext4_handle_error() and save_error_info() Jan Kara 2020-12-14 19:23 ` harshad shirwadkar 2020-12-16 10:11 ` Jan Kara 2020-12-16 10:24 ` Jan Kara 2020-11-27 11:34 ` [PATCH 09/12] ext4: Drop sync argument of ext4_commit_super() Jan Kara 2020-12-14 19:25 ` harshad shirwadkar 2020-11-27 11:34 ` [PATCH 10/12] ext4: Protect superblock modifications with a buffer lock Jan Kara 2020-11-27 11:34 ` [PATCH 11/12] ext4: Save error info to sb through journal if available Jan Kara 2020-11-27 11:34 ` [PATCH 12/12] ext4: Use sbi instead of EXT4_SB(sb) in ext4_update_super() Jan Kara 2020-12-14 19:07 ` [PATCH 00/12] ext4: Various fixes of ext4 handling of fs errors harshad shirwadkar
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=75D9FAE2-6CCC-49A7-8D29-0FDC197C96E8@dilger.ca \ --to=adilger@dilger.ca \ --cc=jack@suse.cz \ --cc=linux-ext4@vger.kernel.org \ --cc=tytso@mit.edu \ --subject='Re: [PATCH 04/12] ext4: Make ext4_abort() use __ext4_error()' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.