Date:   Mon, 6 Feb 2017 12:51:46 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     Theodore Ts'o <tytso@....edu>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH 6/7] ext4: add shutdown bit and check for it

On Feb 5, 2017, at 12:34 AM, Theodore Ts'o <tytso@....edu> wrote:
> 
> Add a shutdown bit that will cause ext4 processing to fail immediately
> with EIO.

Does it make sense to combine the checks for "shutdown", "abort", and
"read-only" into a single check, instead of sprinkling separate checks
throughout the code?  It seems like many of the places where
ext4_forced_shutdown() is checked should also be checking for read-only
and abort, even if they aren't today.

Cheers, Andreas

> Signed-off-by: Theodore Ts'o <tytso@....edu>
> ---
> fs/ext4/ext4.h      |  6 ++++++
> fs/ext4/ext4_jbd2.c | 11 +++++++++++
> fs/ext4/file.c      | 12 ++++++++++++
> fs/ext4/fsync.c     |  3 +++
> fs/ext4/ialloc.c    |  3 +++
> fs/ext4/inline.c    |  3 +++
> fs/ext4/inode.c     | 30 ++++++++++++++++++++++++++++--
> fs/ext4/namei.c     | 12 ++++++++++++
> fs/ext4/page-io.c   |  2 +-
> fs/ext4/super.c     | 18 ++++++++++++++++++
> fs/ext4/xattr.c     |  3 +++
> 11 files changed, 100 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 2e7e02f2f771..35d93ab7f3fb 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1836,6 +1836,12 @@ static inline bool ext4_has_incompat_features(struct super_block *sb)
>  * Superblock flags
>  */
> #define EXT4_FLAGS_RESIZING	0
> +#define EXT4_FLAGS_SHUTDOWN	1
> +
> +static inline int ext4_forced_shutdown(struct ext4_sb_info *sbi)
> +{
> +	return test_bit(EXT4_FLAGS_SHUTDOWN, &sbi->s_ext4_flags);
> +}
> 
> 
> /*
> diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
> index e770c1ee4613..dd106b1d5d89 100644
> --- a/fs/ext4/ext4_jbd2.c
> +++ b/fs/ext4/ext4_jbd2.c
> @@ -43,6 +43,10 @@ static int ext4_journal_check_start(struct super_block *sb)
> 	journal_t *journal;
> 
> 	might_sleep();
> +
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(sb))))
> +		return -EIO;
> +
> 	if (sb->s_flags & MS_RDONLY)
> 		return -EROFS;
> 	WARN_ON(sb->s_writers.frozen == SB_FREEZE_COMPLETE);
> @@ -161,6 +165,13 @@ int __ext4_journal_get_write_access(const char *where, unsigned int line,
> 	might_sleep();
> 
> 	if (ext4_handle_valid(handle)) {
> +		struct super_block *sb;
> +
> +		sb = handle->h_transaction->t_journal->j_private;
> +		if (unlikely(ext4_forced_shutdown(EXT4_SB(sb)))) {
> +			jbd2_journal_abort_handle(handle);
> +			return -EIO;
> +		}
> 		err = jbd2_journal_get_write_access(handle, bh);
> 		if (err)
> 			ext4_journal_abort_handle(where, line, __func__, bh,
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index d663d3d7c81c..ff3f6107b0ba 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -57,6 +57,9 @@ static ssize_t ext4_dax_read_iter(struct kiocb *iocb, struct iov_iter *to)
> 
> static ssize_t ext4_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> {
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(file_inode(iocb->ki_filp)->i_sb))))
> +		return -EIO;
> +
> 	if (!iov_iter_count(to))
> 		return 0; /* skip atime */
> 
> @@ -213,6 +216,9 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> 	int overwrite = 0;
> 	ssize_t ret;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> #ifdef CONFIG_FS_DAX
> 	if (IS_DAX(inode))
> 		return ext4_dax_write_iter(iocb, from);
> @@ -348,6 +354,9 @@ static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma)
> {
> 	struct inode *inode = file->f_mapping->host;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	if (ext4_encrypted_inode(inode)) {
> 		int err = fscrypt_get_encryption_info(inode);
> 		if (err)
> @@ -375,6 +384,9 @@ static int ext4_file_open(struct inode * inode, struct file * filp)
> 	char buf[64], *cp;
> 	int ret;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	if (unlikely(!(sbi->s_mount_flags & EXT4_MF_MNTDIR_SAMPLED) &&
> 		     !(sb->s_flags & MS_RDONLY))) {
> 		sbi->s_mount_flags |= EXT4_MF_MNTDIR_SAMPLED;
> diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c
> index 88effb1053c7..9d549608fd30 100644
> --- a/fs/ext4/fsync.c
> +++ b/fs/ext4/fsync.c
> @@ -100,6 +100,9 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
> 	tid_t commit_tid;
> 	bool needs_barrier = false;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	J_ASSERT(ext4_journal_current_handle() == NULL);
> 
> 	trace_ext4_sync_file_enter(file, datasync);
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index f372fc431b8e..b14bae2598bc 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -764,6 +764,9 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
> 	if (!dir || !dir->i_nlink)
> 		return ERR_PTR(-EPERM);
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(dir->i_sb))))
> +		return ERR_PTR(-EIO);
> +
> 	if ((ext4_encrypted_inode(dir) ||
> 	     DUMMY_ENCRYPTION_ENABLED(EXT4_SB(dir->i_sb))) &&
> 	    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode))) {
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index b777b8aa14ae..30a9f210d1e3 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -215,6 +215,9 @@ static void ext4_write_inline_data(struct inode *inode, struct ext4_iloc *iloc,
> 	struct ext4_inode *raw_inode;
> 	int cp_len = 0;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return;
> +
> 	BUG_ON(!EXT4_I(inode)->i_inline_off);
> 	BUG_ON(pos + len > EXT4_I(inode)->i_inline_size);
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index af97b9170358..e2b7dccc6a11 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1189,6 +1189,9 @@ static int ext4_write_begin(struct file *file, struct address_space *mapping,
> 	pgoff_t index;
> 	unsigned from, to;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	trace_ext4_write_begin(inode, pos, len, flags);
> 	/*
> 	 * Reserve one block more for addition to orphan list in case
> @@ -2047,6 +2050,12 @@ static int ext4_writepage(struct page *page,
> 	struct ext4_io_submit io_submit;
> 	bool keep_towrite = false;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb)))) {
> +		do_invalidatepage(page, 0, PAGE_SIZE);
> +		unlock_page(page);
> +		return -EIO;
> +	}
> +
> 	trace_ext4_writepage(page);
> 	size = i_size_read(inode);
> 	if (page->index == size >> PAGE_SHIFT)
> @@ -2422,7 +2431,8 @@ static int mpage_map_and_submit_extent(handle_t *handle,
> 		if (err < 0) {
> 			struct super_block *sb = inode->i_sb;
> 
> -			if (EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED)
> +			if (ext4_forced_shutdown(EXT4_SB(sb)) ||
> +			    EXT4_SB(sb)->s_mount_flags & EXT4_MF_FS_ABORTED)
> 				goto invalidate_dirty_pages;
> 			/*
> 			 * Let the uper layers retry transient errors.
> @@ -2644,6 +2654,9 @@ static int ext4_writepages(struct address_space *mapping,
> 	struct blk_plug plug;
> 	bool give_up_on_write = false;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	percpu_down_read(&sbi->s_journal_flag_rwsem);
> 	trace_ext4_writepages(inode, wbc);
> 
> @@ -2680,7 +2693,8 @@ static int ext4_writepages(struct address_space *mapping,
> 	 * *never* be called, so if that ever happens, we would want
> 	 * the stack trace.
> 	 */
> -	if (unlikely(sbi->s_mount_flags & EXT4_MF_FS_ABORTED)) {
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(mapping->host->i_sb)) ||
> +		     sbi->s_mount_flags & EXT4_MF_FS_ABORTED)) {
> 		ret = -EROFS;
> 		goto out_writepages;
> 	}
> @@ -2905,6 +2919,9 @@ static int ext4_da_write_begin(struct file *file, struct address_space *mapping,
> 	struct inode *inode = mapping->host;
> 	handle_t *handle;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	index = pos >> PAGE_SHIFT;
> 
> 	if (ext4_nonda_switch(inode->i_sb) ||
> @@ -5212,6 +5229,9 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
> 	int orphan = 0;
> 	const unsigned int ia_valid = attr->ia_valid;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	error = setattr_prepare(dentry, attr);
> 	if (error)
> 		return error;
> @@ -5498,6 +5518,9 @@ int ext4_mark_iloc_dirty(handle_t *handle,
> {
> 	int err = 0;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	if (IS_I_VERSION(inode))
> 		inode_inc_iversion(inode);
> 
> @@ -5521,6 +5544,9 @@ ext4_reserve_inode_write(handle_t *handle, struct inode *inode,
> {
> 	int err;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	err = ext4_get_inode_loc(inode, iloc);
> 	if (!err) {
> 		BUFFER_TRACE(iloc->bh, "get_write_access");
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 931da9d5d915..6ad612c576fc 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2939,6 +2939,9 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
> 	struct ext4_dir_entry_2 *de;
> 	handle_t *handle = NULL;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(dir->i_sb))))
> +		return -EIO;
> +
> 	/* Initialize quotas before so that eventual writes go in
> 	 * separate transaction */
> 	retval = dquot_initialize(dir);
> @@ -3012,6 +3015,9 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
> 	struct ext4_dir_entry_2 *de;
> 	handle_t *handle = NULL;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(dir->i_sb))))
> +		return -EIO;
> +
> 	trace_ext4_unlink_enter(dir, dentry);
> 	/* Initialize quotas before so that eventual writes go
> 	 * in separate transaction */
> @@ -3082,6 +3088,9 @@ static int ext4_symlink(struct inode *dir,
> 	struct fscrypt_str disk_link;
> 	struct fscrypt_symlink_data *sd = NULL;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(dir->i_sb))))
> +		return -EIO;
> +
> 	disk_link.len = len + 1;
> 	disk_link.name = (char *) symname;
> 
> @@ -3874,6 +3883,9 @@ static int ext4_rename2(struct inode *old_dir, struct dentry *old_dentry,
> 			struct inode *new_dir, struct dentry *new_dentry,
> 			unsigned int flags)
> {
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(old_dir->i_sb))))
> +		return -EIO;
> +
> 	if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
> 		return -EINVAL;
> 
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index d83b0f3c5fe9..f8808835a28b 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -158,7 +158,7 @@ static int ext4_end_io(ext4_io_end_t *io)
> 
> 	io->handle = NULL;	/* Following call will use up the handle */
> 	ret = ext4_convert_unwritten_extents(handle, inode, offset, size);
> -	if (ret < 0) {
> +	if (ret < 0 && !ext4_forced_shutdown(EXT4_SB(inode->i_sb))) {
> 		ext4_msg(inode->i_sb, KERN_EMERG,
> 			 "failed to convert unwritten extents to written "
> 			 "extents -- potential data loss!  "
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 514e5fc59893..879e3a6afd9d 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -438,6 +438,9 @@ void __ext4_error(struct super_block *sb, const char *function,
> 	struct va_format vaf;
> 	va_list args;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(sb))))
> +		return;
> +
> 	if (ext4_error_ratelimit(sb)) {
> 		va_start(args, fmt);
> 		vaf.fmt = fmt;
> @@ -459,6 +462,9 @@ void __ext4_error_inode(struct inode *inode, const char *function,
> 	struct va_format vaf;
> 	struct ext4_super_block *es = EXT4_SB(inode->i_sb)->s_es;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return;
> +
> 	es->s_last_error_ino = cpu_to_le32(inode->i_ino);
> 	es->s_last_error_block = cpu_to_le64(block);
> 	if (ext4_error_ratelimit(inode->i_sb)) {
> @@ -491,6 +497,9 @@ void __ext4_error_file(struct file *file, const char *function,
> 	struct inode *inode = file_inode(file);
> 	char pathname[80], *path;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return;
> +
> 	es = EXT4_SB(inode->i_sb)->s_es;
> 	es->s_last_error_ino = cpu_to_le32(inode->i_ino);
> 	if (ext4_error_ratelimit(inode->i_sb)) {
> @@ -567,6 +576,9 @@ void __ext4_std_error(struct super_block *sb, const char *function,
> 	char nbuf[16];
> 	const char *errstr;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(sb))))
> +		return;
> +
> 	/* Special case: if the error is EROFS, and we're not already
> 	 * inside a transaction, then there's really no point in logging
> 	 * an error. */
> @@ -600,6 +612,9 @@ void __ext4_abort(struct super_block *sb, const char *function,
> 	struct va_format vaf;
> 	va_list args;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(sb))))
> +		return;
> +
> 	save_error_info(sb, function, line);
> 	va_start(args, fmt);
> 	vaf.fmt = fmt;
> @@ -695,6 +710,9 @@ __acquires(bitlock)
> 	va_list args;
> 	struct ext4_super_block *es = EXT4_SB(sb)->s_es;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(sb))))
> +		return;
> +
> 	es->s_last_error_ino = cpu_to_le32(ino);
> 	es->s_last_error_block = cpu_to_le64(block);
> 	__save_error_info(sb, function, line);
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index c40bd55b6400..67636acf7624 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -411,6 +411,9 @@ ext4_xattr_get(struct inode *inode, int name_index, const char *name,
> {
> 	int error;
> 
> +	if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
> +		return -EIO;
> +
> 	if (strlen(name) > 255)
> 		return -ERANGE;
> 
> --
> 2.11.0.rc0.7.gbe5a750
> 


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)

Powered by blists - more mailing lists