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 \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.