Re: [PATCH 6/6] ext4: convert to new fserror helpers

[Gabriel Krisman Bertazi <gabriel@krisman.be>]

"Darrick J. Wong" <djwong@kernel.org> writes:

> From: Darrick J. Wong <djwong@kernel.org>
>
> Use the new fserror functions to report metadata errors to fsnotify.
> Note that ext4 inconsistently passes around negative and positive error
> numbers all over the codebase, so we force them all to negative for
> consistency in what we report to fserror, and fserror ensures that only
> positive error numbers are passed to fanotify, per the fanotify(7)
> manpage.
>
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ext4/ioctl.c |    2 ++
>  fs/ext4/super.c |   13 +++++++++----
>  2 files changed, 11 insertions(+), 4 deletions(-)
>
>
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 7ce0fc40aec2fb..ea26cd03d3ce28 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -26,6 +26,7 @@
>  #include <linux/fsmap.h>
>  #include "fsmap.h"
>  #include <trace/events/ext4.h>
> +#include <linux/fserror.h>
>  
>  typedef void ext4_update_sb_callback(struct ext4_sb_info *sbi,
>  				     struct ext4_super_block *es,
> @@ -844,6 +845,7 @@ int ext4_force_shutdown(struct super_block *sb, u32 flags)
>  		return -EINVAL;
>  	}
>  	clear_opt(sb, DISCARD);
> +	fserror_report_shutdown(sb, GFP_KERNEL);
>  	return 0;
>  }
>  
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 87205660c5d026..a6241ffb8639c3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -48,6 +48,7 @@
>  #include <linux/fsnotify.h>
>  #include <linux/fs_context.h>
>  #include <linux/fs_parser.h>
> +#include <linux/fserror.h>
>  
>  #include "ext4.h"
>  #include "ext4_extents.h"	/* Needed for trace points definition */
> @@ -824,7 +825,8 @@ void __ext4_error(struct super_block *sb, const char *function,
>  		       sb->s_id, function, line, current->comm, &vaf);
>  		va_end(args);
>  	}
> -	fsnotify_sb_error(sb, NULL, error ? error : EFSCORRUPTED);
> +	fserror_report_metadata(sb, error ? -abs(error) : -EFSCORRUPTED,
> +				GFP_ATOMIC);
>  
>  	ext4_handle_error(sb, force_ro, error, 0, block, function, line);
>  }
> @@ -856,7 +858,9 @@ void __ext4_error_inode(struct inode *inode, const char *function,
>  			       current->comm, &vaf);
>  		va_end(args);
>  	}
> -	fsnotify_sb_error(inode->i_sb, inode, error ? error : EFSCORRUPTED);
> +	fserror_report_file_metadata(inode,
> +				     error ? -abs(error) : -EFSCORRUPTED,
> +				     GFP_ATOMIC);
>  
>  	ext4_handle_error(inode->i_sb, false, error, inode->i_ino, block,
>  			  function, line);
> @@ -896,7 +900,7 @@ void __ext4_error_file(struct file *file, const char *function,
>  			       current->comm, path, &vaf);
>  		va_end(args);
>  	}
> -	fsnotify_sb_error(inode->i_sb, inode, EFSCORRUPTED);
> +	fserror_report_file_metadata(inode, -EFSCORRUPTED, GFP_ATOMIC);
>  
>  	ext4_handle_error(inode->i_sb, false, EFSCORRUPTED, inode->i_ino, block,
>  			  function, line);
> @@ -965,7 +969,8 @@ void __ext4_std_error(struct super_block *sb, const char *function,
>  		printk(KERN_CRIT "EXT4-fs error (device %s) in %s:%d: %s\n",
>  		       sb->s_id, function, line, errstr);
>  	}
> -	fsnotify_sb_error(sb, NULL, errno ? errno : EFSCORRUPTED);
> +	fserror_report_metadata(sb, errno ? -abs(errno) : -EFSCORRUPTED,
> +				GFP_ATOMIC);
>  
>  	ext4_handle_error(sb, false, -errno, 0, 0, function, line);
>  }
>

Perhaps also delete fsnotify_sb_error after this patch since it is now
implemented by fserror_worker and, if I follow correctly, we don't want
it to be called without the shutdown protection mechanism.

-- 
Gabriel Krisman Bertazi