All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@redhat.com>
To: ext4 development <linux-ext4@vger.kernel.org>
Subject: [PATCH] ext4: serialize unaligned asynchronous DIO
Date: Thu, 13 Jan 2011 16:23:14 -0600	[thread overview]
Message-ID: <4D2F7B52.1040209@redhat.com> (raw)



ext4 has a data corruption case when doing non-block-aligned
asynchronous direct IO into a sparse file, as demonstrated
by xfstest 240.

The root cause is that while ext4 preallocates space in the
hole, mappings of that space still look "new" and 
dio_zero_block() will zero out the unwritten portions.  When
more than one AIO thread is going, they both find this "new"
block and race to zero out their portion; this is uncoordinated
and causes data corruption.

Dave Chinner fixed this for xfs by simply serializing all
unaligned asynchronous direct IO.  I've done the same here.
This is a very big hammer, and I'm not very pleased with
stuffing this into ext4_file_write().  But since ext4 is
DIO_LOCKING, we need to serialize it at this high level.

I tried to move this into ext4_ext_direct_IO, but by then
we have the i_mutex already, and we will wait on the
work queue to do conversions - which must also take the
i_mutex.  So that won't work.

This was originally exposed by qemu-kvm installing to
a raw disk image with a normal sector-63 alignment.  I've
tested a backport of this patch with qemu, and it does
avoid the corruption.  It is also quite a lot slower
(14 min for package installs, vs. 8 min for well-aligned)
but I'll take slow correctness over fast corruption any day.

Mingming suggested that perhaps we can track outstanding
conversions, and wait on that instead so that non-sparse
files won't be affected, but I've had trouble making that
work so far, and would like to get the corruption hole
plugged ASAP.  Perhaps adding a prink_once() warning of
the perf degradation on this path would be useful?

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Index: linux-2.6/fs/ext4/ext4.h
===================================================================
--- linux-2.6.orig/fs/ext4/ext4.h
+++ linux-2.6/fs/ext4/ext4.h
@@ -848,6 +848,7 @@ struct ext4_inode_info {
 	atomic_t i_ioend_count;	/* Number of outstanding io_end structs */
 	/* current io_end structure for async DIO write*/
 	ext4_io_end_t *cur_aio_dio;
+	struct mutex i_aio_mutex; /* big hammer for unaligned AIO */
 
 	spinlock_t i_block_reservation_lock;
 
Index: linux-2.6/fs/ext4/file.c
===================================================================
--- linux-2.6.orig/fs/ext4/file.c
+++ linux-2.6/fs/ext4/file.c
@@ -55,11 +55,31 @@ static int ext4_release_file(struct inod
 	return 0;
 }
 
+static int
+ext4_unaligned_aio(struct inode *inode, const struct iovec *iov,
+		unsigned long nr_segs, loff_t pos)
+{
+	struct super_block *sb = inode->i_sb;
+	int blockmask = sb->s_blocksize - 1;
+	size_t count = iov_length(iov, nr_segs);
+	loff_t final_size = pos + count;
+
+	if (pos >= inode->i_size)
+		return 0;
+
+	if ((pos & blockmask) || (final_size & blockmask))
+		return 1;
+
+	return 0;
+}
+
 static ssize_t
 ext4_file_write(struct kiocb *iocb, const struct iovec *iov,
 		unsigned long nr_segs, loff_t pos)
 {
 	struct inode *inode = iocb->ki_filp->f_path.dentry->d_inode;
+	int unaligned_aio = 0;
+	int ret;
 
 	/*
 	 * If we have encountered a bitmap-format file, the size limit
@@ -78,9 +98,21 @@ ext4_file_write(struct kiocb *iocb, cons
 			nr_segs = iov_shorten((struct iovec *)iov, nr_segs,
 					      sbi->s_bitmap_maxbytes - pos);
 		}
+	} else if (unlikely((iocb->ki_filp->f_flags & O_DIRECT) &&
+		            !is_sync_kiocb(iocb)))
+		unaligned_aio = ext4_unaligned_aio(inode, iov, nr_segs, pos);
+
+	if (unaligned_aio) {
+		mutex_lock(&EXT4_I(inode)->i_aio_mutex);
+		ext4_ioend_wait(inode);
 	}
 
-	return generic_file_aio_write(iocb, iov, nr_segs, pos);
+	ret = generic_file_aio_write(iocb, iov, nr_segs, pos);
+
+	if (unaligned_aio)
+		mutex_unlock(&EXT4_I(inode)->i_aio_mutex);
+
+	return ret;
 }
 
 static const struct vm_operations_struct ext4_file_vm_ops = {
Index: linux-2.6/fs/ext4/super.c
===================================================================
--- linux-2.6.orig/fs/ext4/super.c
+++ linux-2.6/fs/ext4/super.c
@@ -875,6 +875,7 @@ static void init_once(void *foo)
 	init_rwsem(&ei->xattr_sem);
 #endif
 	init_rwsem(&ei->i_data_sem);
+	mutex_init(&ei->i_aio_mutex);
 	inode_init_once(&ei->vfs_inode);
 }
 


             reply	other threads:[~2011-01-13 22:23 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-13 22:23 Eric Sandeen [this message]
2011-01-14  4:15 ` [PATCH] ext4: serialize unaligned asynchronous DIO Ted Ts'o
2011-01-14  4:41   ` Eric Sandeen
2011-01-14 17:28   ` [PATCH V2] " Eric Sandeen
2011-01-18 16:23     ` Eric Sandeen
2011-01-21 16:00     ` Eric Sandeen
2011-01-21 18:26     ` [PATCH V3 RESEND 2] " Eric Sandeen
2011-01-21 23:27       ` Ted Ts'o
2011-02-07  2:33       ` Ted Ts'o
2011-02-07 15:59         ` Ted Ts'o
2011-02-07 17:58           ` Eric Sandeen
2011-02-07 22:18             ` Mingming Cao
2012-02-23 13:23           ` backport "ext4: serialize unaligned asynchronous DIO" to 2.6.32 Philipp Hahn
2012-02-23 15:15             ` Eric Sandeen

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=4D2F7B52.1040209@redhat.com \
    --to=sandeen@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.