All of lore.kernel.org
 help / color / mirror / Atom feed
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 --]

  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.