On 01/14/2011 11:28 AM, Eric Sandeen wrote:
> 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?
Ted, if you haven't merged this already, you might hold off.
I've got a version going which only synchronizes IO on the inode
if there is a pending unwritten extent conversion, which speeds
things up a bit. It's still not the most optimal solution
for this suboptimal workload, but it's as simple as the one
proposed here, and faster.
I'll send in a bit.
-Eric
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> V2: Add comments and daily printk
>
> 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,42 @@ static int ext4_release_file(struct inod
> return 0;
> }
>
> +/*
> + * This tests whether the IO in question is block-aligned or
> + * not. ext4 utilizes unwritten extents when hole-filling
> + * during direct IO, and they are converted to written only
> + * after the IO is complete. Until they are mapped, these
> + * blocks appear as holes, so dio_zero_block() will assume
> + * that it needs to zero out portions of the start and/or
> + * end block. If 2 AIO threads are at work on the same block,
> + * they must be synchronized or one thread will zero the others'
> + * data, causing corruption.
> + */
> +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 +109,30 @@ 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);
> +
> + /* Unaligned direct AIO must be serialized; see comment above */
> + if (unaligned_aio) {
> + static unsigned long unaligned_warn_time;
> +
> + /* Warn about this once per day */
> + if (printk_timed_ratelimit(&unaligned_warn_time, 60*60*24*HZ))
> + ext4_msg(inode->i_sb, KERN_WARNING,
> + "Unaligned AIO/DIO on inode %ld by %s; "
> + "performance will be poor.",
> + inode->i_ino, current->comm);
> + 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);
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
