[PATCH 0/5] simply and speedup I/O completion handling

[PATCH 0/5] simply and speedup I/O completion handling

[Christoph Hellwig]

Get rid of our own counter for pending I/O completion handling which
is superflous and lifelock-prone, and optimize the code around it while
we're at it.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/5] xfs: remove dead ENODEV handling in xfs_destroy_ioend

[Christoph Hellwig]

[-- Attachment #1: xfs-remove-dead-ENODEV-handling --]
[-- Type: text/plain, Size: 1063 bytes --]

No driver returns ENODEV from it bio completion handler, not has this
ever been documented.  Remove the dead code dealing with it.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_aops.c	2011-07-28 17:22:27.277106627 +0200
+++ xfs/fs/xfs/xfs_aops.c	2011-07-28 17:24:19.016501283 +0200
@@ -122,17 +122,6 @@ xfs_destroy_ioend(
 		bh->b_end_io(bh, !ioend->io_error);
 	}
 
-	/*
-	 * Volume managers supporting multiple paths can send back ENODEV
-	 * when the final path disappears.  In this case continuing to fill
-	 * the page cache with dirty data which cannot be written out is
-	 * evil, so prevent that.
-	 */
-	if (unlikely(ioend->io_error == -ENODEV)) {
-		xfs_do_force_shutdown(ip->i_mount, SHUTDOWN_DEVICE_REQ,
-				      __FILE__, __LINE__);
-	}
-
 	xfs_ioend_wake(ip);
 	mempool_free(ioend, xfs_ioend_pool);
 }

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/5] xfs: defer AIO/DIO completions

[Christoph Hellwig]

[-- Attachment #1: xfs-fix-aio-completions --]
[-- Type: text/plain, Size: 2262 bytes --]

We really shouldn't complete AIO or DIO requests until we have finished
the unwritten extent conversion and size update.  This means fsync never
has to pick up any ioends as all work has been completed when signalling
I/O completion.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_aops.c	2011-08-13 06:54:41.188441655 -0700
+++ xfs/fs/xfs/xfs_aops.c	2011-08-13 10:56:13.109932177 -0700
@@ -122,6 +122,11 @@ xfs_destroy_ioend(
 		bh->b_end_io(bh, !ioend->io_error);
 	}
 
+	if (ioend->io_iocb) {
+		if (ioend->io_buffer_tail == (struct buffer_head *)1)
+			aio_complete(ioend->io_iocb, ioend->io_result, 0);
+		inode_dio_done(ioend->io_inode);
+	}
 	xfs_ioend_wake(ip);
 	mempool_free(ioend, xfs_ioend_pool);
 }
@@ -236,8 +241,6 @@ xfs_end_io(
 		/* ensure we don't spin on blocked ioends */
 		delay(1);
 	} else {
-		if (ioend->io_iocb)
-			aio_complete(ioend->io_iocb, ioend->io_result, 0);
 		xfs_destroy_ioend(ioend);
 	}
 }
@@ -1299,28 +1302,24 @@ xfs_end_io_direct_write(
 
 	ioend->io_offset = offset;
 	ioend->io_size = size;
+	ioend->io_iocb = iocb;
+	ioend->io_result = ret;
 	if (private && size > 0)
 		ioend->io_type = IO_UNWRITTEN;
 
 	if (is_async) {
 		/*
-		 * If we are converting an unwritten extent we need to delay
-		 * the AIO completion until after the unwrittent extent
-		 * conversion has completed, otherwise do it ASAP.
+		 * Abuse a field not use for direct I/O as an indicator that
+		 * we have to call aio_complete.  Unfortunatly an
+		 * is_sync_kiocb check is not enough for that as some
+		 * various aio-submitted iocbs may be treated as synchronous
+		 * I/O in the end.
 		 */
-		if (ioend->io_type == IO_UNWRITTEN) {
-			ioend->io_iocb = iocb;
-			ioend->io_result = ret;
-		} else {
-			aio_complete(iocb, ret, 0);
-		}
+		ioend->io_buffer_tail = (struct buffer_head *)1;
 		xfs_finish_ioend(ioend);
 	} else {
 		xfs_finish_ioend_sync(ioend);
 	}
-
-	/* XXX: probably should move into the real I/O completion handler */
-	inode_dio_done(ioend->io_inode);
 }
 
 STATIC ssize_t

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/5] xfs: reduce ioend latency

[Christoph Hellwig]

[-- Attachment #1: xfs-fix-io-latency --]
[-- Type: text/plain, Size: 1704 bytes --]

There is no reason to queue up ioends for processing in user context
unless we actually need it.  Just complete ioends that do not convert
unwritten extents or need a size update from the end_io context.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_aops.c	2011-08-13 10:57:57.559366326 -0700
+++ xfs/fs/xfs/xfs_aops.c	2011-08-13 10:57:57.979364052 -0700
@@ -150,6 +150,15 @@ xfs_ioend_new_eof(
 }
 
 /*
+ * Fast and loose check if this write could update the on-disk inode size.
+ */
+static inline bool xfs_ioend_is_append(struct xfs_ioend *ioend)
+{
+	return ioend->io_offset + ioend->io_size >
+		XFS_I(ioend->io_inode)->i_d.di_size;
+}
+
+/*
  * Update on-disk file size now that data has been written to disk.  The
  * current in-memory file size is i_size.  If a write is beyond eof i_new_size
  * will be the intended file size until i_size is updated.  If this write does
@@ -186,6 +195,9 @@ xfs_setfilesize(
 
 /*
  * Schedule IO completion handling on the final put of an ioend.
+ *
+ * If there is no work to do we might as well call it a day and free the
+ * ioend right now.
  */
 STATIC void
 xfs_finish_ioend(
@@ -194,8 +206,10 @@ xfs_finish_ioend(
 	if (atomic_dec_and_test(&ioend->io_remaining)) {
 		if (ioend->io_type == IO_UNWRITTEN)
 			queue_work(xfsconvertd_workqueue, &ioend->io_work);
-		else
+		else if (xfs_ioend_is_append(ioend))
 			queue_work(xfsdatad_workqueue, &ioend->io_work);
+		else
+			xfs_destroy_ioend(ioend);
 	}
 }
 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/5] xfs: wait for I/O completion when writing out pages in xfs_setattr_size

[Christoph Hellwig]

[-- Attachment #1: xfs-setattr-wait-for-writeback --]
[-- Type: text/plain, Size: 958 bytes --]

The current code relies on the xfs_ioend_wait call later on to make sure
all I/O actually has completed.  The xfs_ioend_wait call will go away soon,
so prepare for that by using the waiting filemap function.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_iops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iops.c	2011-08-13 09:18:59.458202448 -0700
+++ xfs/fs/xfs/xfs_iops.c	2011-08-13 09:19:21.664748812 -0700
@@ -825,8 +825,8 @@ xfs_setattr_size(
 	 * care about here.
 	 */
 	if (ip->i_size != ip->i_d.di_size && iattr->ia_size > ip->i_d.di_size) {
-		error = xfs_flush_pages(ip, ip->i_d.di_size, iattr->ia_size,
-					XBF_ASYNC, FI_NONE);
+		error = xfs_flush_pages(ip, ip->i_d.di_size, iattr->ia_size, 0,
+					FI_NONE);
 		if (error)
 			goto out_unlock;
 	}

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 5/5] xfs: remove i_iocount

[Christoph Hellwig]

[-- Attachment #1: xfs-remove-iocount --]
[-- Type: text/plain, Size: 8671 bytes --]

We now have an i_dio_count filed and surrounding infrastructure to wait
for direct I/O completion instead of i_icount, and we have never needed
to iocount waits for buffered I/O given that we only set the page uptodate
after finishing all required work.  Thus remove i_iocount, and replace
the actually needed waits with calls to inode_dio_wait.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_aops.c	2011-08-13 10:57:57.979364052 -0700
+++ xfs/fs/xfs/xfs_aops.c	2011-08-13 10:58:03.769332684 -0700
@@ -38,40 +38,6 @@
 #include <linux/pagevec.h>
 #include <linux/writeback.h>
 
-
-/*
- * Prime number of hash buckets since address is used as the key.
- */
-#define NVSYNC		37
-#define to_ioend_wq(v)	(&xfs_ioend_wq[((unsigned long)v) % NVSYNC])
-static wait_queue_head_t xfs_ioend_wq[NVSYNC];
-
-void __init
-xfs_ioend_init(void)
-{
-	int i;
-
-	for (i = 0; i < NVSYNC; i++)
-		init_waitqueue_head(&xfs_ioend_wq[i]);
-}
-
-void
-xfs_ioend_wait(
-	xfs_inode_t	*ip)
-{
-	wait_queue_head_t *wq = to_ioend_wq(ip);
-
-	wait_event(*wq, (atomic_read(&ip->i_iocount) == 0));
-}
-
-STATIC void
-xfs_ioend_wake(
-	xfs_inode_t	*ip)
-{
-	if (atomic_dec_and_test(&ip->i_iocount))
-		wake_up(to_ioend_wq(ip));
-}
-
 void
 xfs_count_page_state(
 	struct page		*page,
@@ -115,7 +81,6 @@ xfs_destroy_ioend(
 	xfs_ioend_t		*ioend)
 {
 	struct buffer_head	*bh, *next;
-	struct xfs_inode	*ip = XFS_I(ioend->io_inode);
 
 	for (bh = ioend->io_buffer_head; bh; bh = next) {
 		next = bh->b_private;
@@ -127,7 +92,7 @@ xfs_destroy_ioend(
 			aio_complete(ioend->io_iocb, ioend->io_result, 0);
 		inode_dio_done(ioend->io_inode);
 	}
-	xfs_ioend_wake(ip);
+
 	mempool_free(ioend, xfs_ioend_pool);
 }
 
@@ -297,7 +262,6 @@ xfs_alloc_ioend(
 	ioend->io_inode = inode;
 	ioend->io_buffer_head = NULL;
 	ioend->io_buffer_tail = NULL;
-	atomic_inc(&XFS_I(ioend->io_inode)->i_iocount);
 	ioend->io_offset = 0;
 	ioend->io_size = 0;
 	ioend->io_iocb = NULL;
@@ -557,7 +521,6 @@ xfs_cancel_ioend(
 			unlock_buffer(bh);
 		} while ((bh = next_bh) != NULL);
 
-		xfs_ioend_wake(XFS_I(ioend->io_inode));
 		mempool_free(ioend, xfs_ioend_pool);
 	} while ((ioend = next) != NULL);
 }
Index: xfs/fs/xfs/xfs_file.c
===================================================================
--- xfs.orig/fs/xfs/xfs_file.c	2011-08-13 10:45:14.693499125 -0700
+++ xfs/fs/xfs/xfs_file.c	2011-08-13 10:58:03.769332684 -0700
@@ -149,10 +149,6 @@ xfs_file_fsync(
 
 	xfs_iflags_clear(ip, XFS_ITRUNCATED);
 
-	xfs_ilock(ip, XFS_IOLOCK_SHARED);
-	xfs_ioend_wait(ip);
-	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
-
 	if (mp->m_flags & XFS_MOUNT_BARRIER) {
 		/*
 		 * If we have an RT and/or log subvolume we need to make sure
@@ -721,7 +717,7 @@ xfs_file_aio_write_checks(
  * the dio layer.  To avoid the problem with aio, we also need to wait for
  * outstanding IOs to complete so that unwritten extent conversion is completed
  * before we try to map the overlapping block. This is currently implemented by
- * hitting it with a big hammer (i.e. xfs_ioend_wait()).
+ * hitting it with a big hammer (i.e. inode_dio_wait()).
  *
  * Returns with locks held indicated by @iolock and errors indicated by
  * negative return values.
@@ -776,7 +772,7 @@ xfs_file_dio_aio_write(
 	 * otherwise demote the lock if we had to flush cached pages
 	 */
 	if (unaligned_io)
-		xfs_ioend_wait(ip);
+		inode_dio_wait(inode);
 	else if (*iolock == XFS_IOLOCK_EXCL) {
 		xfs_rw_ilock_demote(ip, XFS_IOLOCK_EXCL);
 		*iolock = XFS_IOLOCK_SHARED;
Index: xfs/fs/xfs/xfs_super.c
===================================================================
--- xfs.orig/fs/xfs/xfs_super.c	2011-08-13 10:45:14.703499072 -0700
+++ xfs/fs/xfs/xfs_super.c	2011-08-13 10:58:03.772665999 -0700
@@ -794,8 +794,6 @@ xfs_fs_destroy_inode(
 	if (is_bad_inode(inode))
 		goto out_reclaim;
 
-	xfs_ioend_wait(ip);
-
 	ASSERT(XFS_FORCED_SHUTDOWN(ip->i_mount) || ip->i_delayed_blks == 0);
 
 	/*
@@ -835,7 +833,6 @@ xfs_fs_inode_init_once(
 	inode_init_once(VFS_I(ip));
 
 	/* xfs inode */
-	atomic_set(&ip->i_iocount, 0);
 	atomic_set(&ip->i_pincount, 0);
 	spin_lock_init(&ip->i_flags_lock);
 	init_waitqueue_head(&ip->i_ipin_wait);
@@ -928,7 +925,6 @@ xfs_fs_write_inode(
 		 * ->sync_fs call do that for thus, which reduces the number
 		 * of synchronous log foces dramatically.
 		 */
-		xfs_ioend_wait(ip);
 		xfs_ilock(ip, XFS_ILOCK_SHARED);
 		if (ip->i_update_core) {
 			error = xfs_log_inode(ip);
@@ -1695,7 +1691,6 @@ init_xfs_fs(void)
 	printk(KERN_INFO XFS_VERSION_STRING " with "
 			 XFS_BUILD_OPTIONS " enabled\n");
 
-	xfs_ioend_init();
 	xfs_dir_startup();
 
 	error = xfs_init_zones();
Index: xfs/fs/xfs/xfs_sync.c
===================================================================
--- xfs.orig/fs/xfs/xfs_sync.c	2011-08-13 10:45:14.710165702 -0700
+++ xfs/fs/xfs/xfs_sync.c	2011-08-13 10:58:03.775999314 -0700
@@ -227,21 +227,17 @@ xfs_sync_inode_data(
 	int			error = 0;
 
 	if (!mapping_tagged(mapping, PAGECACHE_TAG_DIRTY))
-		goto out_wait;
+		return 0;
 
 	if (!xfs_ilock_nowait(ip, XFS_IOLOCK_SHARED)) {
 		if (flags & SYNC_TRYLOCK)
-			goto out_wait;
+			return 0;
 		xfs_ilock(ip, XFS_IOLOCK_SHARED);
 	}
 
 	error = xfs_flush_pages(ip, 0, -1, (flags & SYNC_WAIT) ?
 				0 : XBF_ASYNC, FI_NONE);
 	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
-
- out_wait:
-	if (flags & SYNC_WAIT)
-		xfs_ioend_wait(ip);
 	return error;
 }
 
Index: xfs/fs/xfs/xfs_vnodeops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_vnodeops.c	2011-08-13 10:45:14.720165647 -0700
+++ xfs/fs/xfs/xfs_vnodeops.c	2011-08-13 10:58:03.775999314 -0700
@@ -647,8 +647,6 @@ xfs_inactive(
 	if (truncate) {
 		xfs_ilock(ip, XFS_IOLOCK_EXCL);
 
-		xfs_ioend_wait(ip);
-
 		error = xfs_trans_reserve(tp, 0,
 					  XFS_ITRUNCATE_LOG_RES(mp),
 					  0, XFS_TRANS_PERM_LOG_RES,
@@ -2076,7 +2074,7 @@ xfs_free_file_space(
 	if (need_iolock) {
 		xfs_ilock(ip, XFS_IOLOCK_EXCL);
 		/* wait for the completion of any pending DIOs */
-		xfs_ioend_wait(ip);
+		inode_dio_wait(VFS_I(ip));
 	}
 
 	rounding = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
Index: xfs/fs/xfs/xfs_aops.h
===================================================================
--- xfs.orig/fs/xfs/xfs_aops.h	2011-08-13 10:57:51.966063295 -0700
+++ xfs/fs/xfs/xfs_aops.h	2011-08-13 10:58:03.779332629 -0700
@@ -60,9 +60,6 @@ typedef struct xfs_ioend {
 extern const struct address_space_operations xfs_address_space_operations;
 extern int xfs_get_blocks(struct inode *, sector_t, struct buffer_head *, int);
 
-extern void xfs_ioend_init(void);
-extern void xfs_ioend_wait(struct xfs_inode *);
-
 extern void xfs_count_page_state(struct page *, int *, int *);
 
 #endif /* __XFS_AOPS_H__ */
Index: xfs/fs/xfs/xfs_iops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iops.c	2011-08-13 10:58:00.786015513 -0700
+++ xfs/fs/xfs/xfs_iops.c	2011-08-13 10:58:03.782665944 -0700
@@ -832,9 +832,9 @@ xfs_setattr_size(
 	}
 
 	/*
-	 * Wait for all I/O to complete.
+	 * Wait for all direct I/O to complete.
 	 */
-	xfs_ioend_wait(ip);
+	inode_dio_wait(inode);
 
 	error = -block_truncate_page(inode->i_mapping, iattr->ia_size,
 				     xfs_get_blocks);
Index: xfs/fs/xfs/xfs_iget.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iget.c	2011-08-13 10:45:14.760165430 -0700
+++ xfs/fs/xfs/xfs_iget.c	2011-08-13 10:58:03.782665944 -0700
@@ -75,7 +75,6 @@ xfs_inode_alloc(
 		return NULL;
 	}
 
-	ASSERT(atomic_read(&ip->i_iocount) == 0);
 	ASSERT(atomic_read(&ip->i_pincount) == 0);
 	ASSERT(!spin_is_locked(&ip->i_flags_lock));
 	ASSERT(completion_done(&ip->i_flush));
@@ -150,7 +149,6 @@ xfs_inode_free(
 	}
 
 	/* asserts to verify all state is correct here */
-	ASSERT(atomic_read(&ip->i_iocount) == 0);
 	ASSERT(atomic_read(&ip->i_pincount) == 0);
 	ASSERT(!spin_is_locked(&ip->i_flags_lock));
 	ASSERT(completion_done(&ip->i_flush));
Index: xfs/fs/xfs/xfs_inode.h
===================================================================
--- xfs.orig/fs/xfs/xfs_inode.h	2011-08-13 10:45:14.776832008 -0700
+++ xfs/fs/xfs/xfs_inode.h	2011-08-13 10:58:03.785999260 -0700
@@ -257,7 +257,6 @@ typedef struct xfs_inode {
 
 	xfs_fsize_t		i_size;		/* in-memory size */
 	xfs_fsize_t		i_new_size;	/* size when write completes */
-	atomic_t		i_iocount;	/* outstanding I/O count */
 
 	/* VFS inode */
 	struct inode		i_vnode;	/* embedded VFS inode */

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/5] xfs: remove dead ENODEV handling in xfs_destroy_ioend

[Dave Chinner]

On Sun, Aug 14, 2011 at 06:24:13PM -0400, Christoph Hellwig wrote:
> No driver returns ENODEV from it bio completion handler, not has this
> ever been documented.  Remove the dead code dealing with it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

IIRC, this was added years ago for XVM to cause filesystems to shut
down when the storage went away (e.g. someone tripped over a cable
bundle or fenced the machine).  It would be nice if there was some
standard way of handling this rather than having the filesytem
continue to allow applications to dirty memory when there is no
chance of ever cleaning it....

As it is, I see no point in keeping something that won't ever be
used, so

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/5] xfs: defer AIO/DIO completions

[Dave Chinner]

On Sun, Aug 14, 2011 at 06:24:14PM -0400, Christoph Hellwig wrote:
> We really shouldn't complete AIO or DIO requests until we have finished
> the unwritten extent conversion and size update.  This means fsync never
> has to pick up any ioends as all work has been completed when signalling
> I/O completion.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I don't really like the field abuse. pahole tells me:

struct xfs_ioend {
        struct xfs_ioend *         io_list;              /*     0     8 */
        unsigned int               io_type;              /*     8     4 */
        int                        io_error;             /*    12     4 */
        atomic_t                   io_remaining;         /*    16     4 */

        /* XXX 4 bytes hole, try to pack */

        struct inode *             io_inode;             /*    24     8 */
        struct buffer_head *       io_buffer_head;       /*    32     8 */
        struct buffer_head *       io_buffer_tail;       /*    40     8 */
        size_t                     io_size;              /*    48     8 */
        xfs_off_t                  io_offset;            /*    56     8 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        struct work_struct         io_work;              /*    64    32 */
        struct kiocb *             io_iocb;              /*    96     8 */
        int                        io_result;            /*   104     4 */

There's a 4 byte hole in the xfs_ioend structure where you could put
a flags field to communicate this and not grow the structure....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/5] xfs: reduce ioend latency

[Dave Chinner]

On Sun, Aug 14, 2011 at 06:24:15PM -0400, Christoph Hellwig wrote:
> There is no reason to queue up ioends for processing in user context
> unless we actually need it.  Just complete ioends that do not convert
> unwritten extents or need a size update from the end_io context.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: xfs/fs/xfs/xfs_aops.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_aops.c	2011-08-13 10:57:57.559366326 -0700
> +++ xfs/fs/xfs/xfs_aops.c	2011-08-13 10:57:57.979364052 -0700
> @@ -150,6 +150,15 @@ xfs_ioend_new_eof(
>  }
>  
>  /*
> + * Fast and loose check if this write could update the on-disk inode size.
> + */
> +static inline bool xfs_ioend_is_append(struct xfs_ioend *ioend)
> +{
> +	return ioend->io_offset + ioend->io_size >
> +		XFS_I(ioend->io_inode)->i_d.di_size;
> +}
> +
> +/*
>   * Update on-disk file size now that data has been written to disk.  The
>   * current in-memory file size is i_size.  If a write is beyond eof i_new_size
>   * will be the intended file size until i_size is updated.  If this write does
> @@ -186,6 +195,9 @@ xfs_setfilesize(
>  
>  /*
>   * Schedule IO completion handling on the final put of an ioend.
> + *
> + * If there is no work to do we might as well call it a day and free the
> + * ioend right now.
>   */
>  STATIC void
>  xfs_finish_ioend(
> @@ -194,8 +206,10 @@ xfs_finish_ioend(
>  	if (atomic_dec_and_test(&ioend->io_remaining)) {
>  		if (ioend->io_type == IO_UNWRITTEN)
>  			queue_work(xfsconvertd_workqueue, &ioend->io_work);
> -		else
> +		else if (xfs_ioend_is_append(ioend))
>  			queue_work(xfsdatad_workqueue, &ioend->io_work);
> +		else
> +			xfs_destroy_ioend(ioend);
>  	}
>  }

That's similar to a check I added in a previous patch series to
avoid taking the ILOCK in IO completion if it wasn't necessary. THis
just checks earlier to avoid the workqueue switch, so it definitely
better than what I did.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/5] xfs: wait for I/O completion when writing out pages in xfs_setattr_size

[Dave Chinner]

On Sun, Aug 14, 2011 at 06:24:16PM -0400, Christoph Hellwig wrote:
> The current code relies on the xfs_ioend_wait call later on to make sure
> all I/O actually has completed.  The xfs_ioend_wait call will go away soon,
> so prepare for that by using the waiting filemap function.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 5/5] xfs: remove i_iocount

[Dave Chinner]

On Sun, Aug 14, 2011 at 06:24:17PM -0400, Christoph Hellwig wrote:
> We now have an i_dio_count filed and surrounding infrastructure to wait
> for direct I/O completion instead of i_icount, and we have never needed
> to iocount waits for buffered I/O given that we only set the page uptodate
> after finishing all required work.  Thus remove i_iocount, and replace
> the actually needed waits with calls to inode_dio_wait.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks sane. One less bit of XFS specific code. ;)

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/5] xfs: remove dead ENODEV handling in xfs_destroy_ioend

[Christoph Hellwig]

On Mon, Aug 22, 2011 at 04:32:45PM +1000, Dave Chinner wrote:
> On Sun, Aug 14, 2011 at 06:24:13PM -0400, Christoph Hellwig wrote:
> > No driver returns ENODEV from it bio completion handler, not has this
> > ever been documented.  Remove the dead code dealing with it.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> IIRC, this was added years ago for XVM to cause filesystems to shut
> down when the storage went away (e.g. someone tripped over a cable
> bundle or fenced the machine).  It would be nice if there was some
> standard way of handling this rather than having the filesytem
> continue to allow applications to dirty memory when there is no
> chance of ever cleaning it....
> 
> As it is, I see no point in keeping something that won't ever be
> used, so

Initially I just made sure to defer contexts with ENODEV returns
to the workqueues as well, as we can't really shutdown the fs from
irq context.  But given that this code is dead, and we are mixing
up positive and negative errors in io_error I decided it's better
to remove it for now.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/5] xfs: defer AIO/DIO completions

[Christoph Hellwig]

On Mon, Aug 22, 2011 at 04:40:28PM +1000, Dave Chinner wrote:
> There's a 4 byte hole in the xfs_ioend structure where you could put
> a flags field to communicate this and not grow the structure....

I'll respin it using a separate field.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/5] xfs: reduce ioend latency

[Christoph Hellwig]

On Mon, Aug 22, 2011 at 04:43:45PM +1000, Dave Chinner wrote:
> That's similar to a check I added in a previous patch series to
> avoid taking the ILOCK in IO completion if it wasn't necessary. THis
> just checks earlier to avoid the workqueue switch, so it definitely
> better than what I did.

Yes, that's where it came from.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs