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 for Android: free password hash cracker in your pocket
[<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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ