linux-ext4 - Re: [PATCH 2/2] ext2: Add blk_issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <496E2276.8050309@redhat.com>
Date:	Wed, 14 Jan 2009 11:35:50 -0600
From:	Eric Sandeen <sandeen@...hat.com>
To:	Jan Kara <jack@...e.cz>
CC:	linux-ext4@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, pavel@...e.cz
Subject: Re: [PATCH 2/2] ext2: Add blk_issue_flush() to syncing paths

Jan Kara wrote:
> To be really safe that the data hit the platter, we should also flush drive's
> writeback caches on fsync and for O_SYNC files or O_DIRSYNC inodes.

Seems sane, but aren't we getting really divergent behavior here between
ext2, ext3, and ext4 w.r.t. drive cache flushing for sync paths?

(at least, ext3 has no calls to flush cache at this point)

-Eric

> Signed-off-by: Jan Kara <jack@...e.cz>
> CC: pavel@...e.cz
> ---
>  fs/ext2/dir.c   |    5 ++++-
>  fs/ext2/fsync.c |    7 +++++--
>  fs/ext2/inode.c |    7 +++++++
>  fs/ext2/xattr.c |    6 +++++-
>  4 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
> index 2999d72..d6cb59f 100644
> --- a/fs/ext2/dir.c
> +++ b/fs/ext2/dir.c
> @@ -25,6 +25,7 @@
>  #include <linux/buffer_head.h>
>  #include <linux/pagemap.h>
>  #include <linux/swap.h>
> +#include <linux/blkdev.h>		/* for blkdev_issue_flush() */
>  
>  typedef struct ext2_dir_entry_2 ext2_dirent;
>  
> @@ -97,8 +98,10 @@ static int ext2_commit_chunk(struct page *page, loff_t pos, unsigned len)
>  
>  	if (IS_DIRSYNC(dir)) {
>  		err = write_one_page(page, 1);
> -		if (!err)
> +		if (!err) {
>  			err = ext2_sync_inode(dir);
> +			blkdev_issue_flush(dir->i_sb->s_bdev, NULL);
> +		}
>  	} else {
>  		unlock_page(page);
>  	}
> diff --git a/fs/ext2/fsync.c b/fs/ext2/fsync.c
> index fc66c93..9cd1838 100644
> --- a/fs/ext2/fsync.c
> +++ b/fs/ext2/fsync.c
> @@ -24,6 +24,7 @@
>  
>  #include "ext2.h"
>  #include <linux/buffer_head.h>		/* for sync_mapping_buffers() */
> +#include <linux/blkdev.h>		/* for blkdev_issue_flush() */
>  
>  
>  /*
> @@ -39,12 +40,14 @@ int ext2_sync_file(struct file *file, struct dentry *dentry, int datasync)
>  
>  	ret = sync_mapping_buffers(inode->i_mapping);
>  	if (!(inode->i_state & I_DIRTY))
> -		return ret;
> +		goto out;
>  	if (datasync && !(inode->i_state & I_DIRTY_DATASYNC))
> -		return ret;
> +		goto out;
>  
>  	err = ext2_sync_inode(inode);
>  	if (ret == 0)
>  		ret = err;
> +out:
> +	blkdev_issue_flush(inode->i_sb->s_bdev, NULL);
>  	return ret;
>  }
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 23fff2f..49b479e 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -33,6 +33,7 @@
>  #include <linux/mpage.h>
>  #include <linux/fiemap.h>
>  #include <linux/namei.h>
> +#include <linux/blkdev.h>	/* for blkdev_issue_flush() */
>  #include "ext2.h"
>  #include "acl.h"
>  #include "xip.h"
> @@ -68,9 +69,14 @@ void ext2_delete_inode (struct inode * inode)
>  	mark_inode_dirty(inode);
>  	ext2_update_inode(inode, inode_needs_sync(inode));
>  
> +	/* Make sure inode deletion really gets to disk. Disk write caches
> +	 * are flushed either in ext2_truncate() or we do it explicitly */
>  	inode->i_size = 0;
>  	if (inode->i_blocks)
>  		ext2_truncate (inode);
> +	else if (inode_needs_sync(inode))
> +		blkdev_issue_flush(inode->i_sb->s_bdev, NULL);
> +
>  	ext2_free_inode (inode);
>  
>  	return;
> @@ -1104,6 +1110,7 @@ do_indirects:
>  	if (inode_needs_sync(inode)) {
>  		sync_mapping_buffers(inode->i_mapping);
>  		ext2_sync_inode (inode);
> +		blkdev_issue_flush(inode->i_sb->s_bdev, NULL);
>  	} else {
>  		mark_inode_dirty(inode);
>  	}
> diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
> index 987a526..d480216 100644
> --- a/fs/ext2/xattr.c
> +++ b/fs/ext2/xattr.c
> @@ -60,6 +60,7 @@
>  #include <linux/mbcache.h>
>  #include <linux/quotaops.h>
>  #include <linux/rwsem.h>
> +#include <linux/blkdev.h>		/* for blkdev_issue_flush() */
>  #include "ext2.h"
>  #include "xattr.h"
>  #include "acl.h"
> @@ -702,6 +703,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
>  				DQUOT_FREE_BLOCK(inode, 1);
>  			goto cleanup;
>  		}
> +		blkdev_issue_flush(sb->s_bdev, NULL);
>  	} else
>  		mark_inode_dirty(inode);
>  
> @@ -792,8 +794,10 @@ ext2_xattr_delete_inode(struct inode *inode)
>  			le32_to_cpu(HDR(bh)->h_refcount));
>  		unlock_buffer(bh);
>  		mark_buffer_dirty(bh);
> -		if (IS_SYNC(inode))
> +		if (IS_SYNC(inode)) {
>  			sync_dirty_buffer(bh);
> +			blkdev_issue_flush(inode->i_sb->s_bdev, NULL);
> +		}
>  		DQUOT_FREE_BLOCK(inode, 1);
>  	}
>  	EXT2_I(inode)->i_file_acl = 0;

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html