All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] patch queue for Linux 3.2
@ 2011-08-23  8:28 Christoph Hellwig
  2011-08-23  8:28 ` [PATCH 01/11] xfs: remove delwri buffer handling from xfs_buf_iorequest Christoph Hellwig
                   ` (10 more replies)
  0 siblings, 11 replies; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

This is a resend of my previous two series.  Compared to the last versions
I fixed a non-debug compile failure in the buffer patches, and used a new
field for the aio indicator in the ioend, as suggested by Dave.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 01/11] xfs: remove delwri buffer handling from xfs_buf_iorequest
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:21   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 02/11] xfs: remove the unlock argument to xfs_buf_delwri_queue Christoph Hellwig
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-simplify-xfs_buf_iorequest --]
[-- Type: text/plain, Size: 1002 bytes --]

We cannot ever reach xfs_buf_iorequest for a buffer with XBF_DELWRI set,
given that all write handlers make sure that the buffer is remove from
the delwri queue before, and we never do reads with the XBF_DELWRI flag
set (which the code would not handle correctly anyway).

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

--- xfs.orig/fs/xfs_buf.c	2011-08-10 10:33:22.758525637 +0200
+++ xfs/fs/xfs/xfs_buf.c	2011-08-11 22:24:09.236752446 +0200
@@ -1275,15 +1276,10 @@ xfs_buf_iorequest(
 {
 	trace_xfs_buf_iorequest(bp, _RET_IP_);
 
-	if (bp->b_flags & XBF_DELWRI) {
-		xfs_buf_delwri_queue(bp, 1);
-		return 0;
-	}
+	ASSERT(!(bp->b_flags & XBF_DELWRI));
 
-	if (bp->b_flags & XBF_WRITE) {
+	if (bp->b_flags & XBF_WRITE)
 		xfs_buf_wait_unpin(bp);
-	}
-
 	xfs_buf_hold(bp);
 
 	/* Set the count to 1 initially, this will stop an I/O

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 02/11] xfs: remove the unlock argument to xfs_buf_delwri_queue
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
  2011-08-23  8:28 ` [PATCH 01/11] xfs: remove delwri buffer handling from xfs_buf_iorequest Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:22   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 03/11] xfs: move more delwri setup into xfs_buf_delwri_queue Christoph Hellwig
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-simplify-xfs_buf_delwri_queue --]
[-- Type: text/plain, Size: 1965 bytes --]

We can just unlock the buffer in the caller, and the decrement of b_hold
would also be needed in the !unlock, we just never hit that case currently
given that the caller handles that case.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

--- xfs.orig/fs/xfs/xfs_buf.c	2011-08-11 22:29:23.490069868 +0200
+++ xfs/fs/xfs/xfs_buf.c	2011-08-11 22:31:10.196731133 +0200
@@ -43,7 +43,7 @@
 
 static kmem_zone_t *xfs_buf_zone;
 STATIC int xfsbufd(void *);
-STATIC void xfs_buf_delwri_queue(xfs_buf_t *, int);
+STATIC void xfs_buf_delwri_queue(xfs_buf_t *);
 
 static struct workqueue_struct *xfslogd_workqueue;
 struct workqueue_struct *xfsdatad_workqueue;
@@ -940,7 +940,7 @@ xfs_buf_unlock(
 	if ((bp->b_flags & (XBF_DELWRI|_XBF_DELWRI_Q)) == XBF_DELWRI) {
 		atomic_inc(&bp->b_hold);
 		bp->b_flags |= XBF_ASYNC;
-		xfs_buf_delwri_queue(bp, 0);
+		xfs_buf_delwri_queue(bp);
 	}
 
 	XB_CLEAR_OWNER(bp);
@@ -1049,7 +1049,8 @@ xfs_bdwrite(
 	bp->b_flags &= ~XBF_READ;
 	bp->b_flags |= (XBF_DELWRI | XBF_ASYNC);
 
-	xfs_buf_delwri_queue(bp, 1);
+	xfs_buf_delwri_queue(bp);
+	xfs_buf_unlock(bp);
 }
 
 /*
@@ -1562,8 +1563,7 @@ error:
  */
 STATIC void
 xfs_buf_delwri_queue(
-	xfs_buf_t		*bp,
-	int			unlock)
+	xfs_buf_t		*bp)
 {
 	struct list_head	*dwq = &bp->b_target->bt_delwrite_queue;
 	spinlock_t		*dwlk = &bp->b_target->bt_delwrite_lock;
@@ -1576,8 +1576,7 @@ xfs_buf_delwri_queue(
 	/* If already in the queue, dequeue and place at tail */
 	if (!list_empty(&bp->b_list)) {
 		ASSERT(bp->b_flags & _XBF_DELWRI_Q);
-		if (unlock)
-			atomic_dec(&bp->b_hold);
+		atomic_dec(&bp->b_hold);
 		list_del(&bp->b_list);
 	}
 
@@ -1590,9 +1589,6 @@ xfs_buf_delwri_queue(
 	list_add_tail(&bp->b_list, dwq);
 	bp->b_queuetime = jiffies;
 	spin_unlock(dwlk);
-
-	if (unlock)
-		xfs_buf_unlock(bp);
 }
 
 void

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 03/11] xfs: move more delwri setup into xfs_buf_delwri_queue
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
  2011-08-23  8:28 ` [PATCH 01/11] xfs: remove delwri buffer handling from xfs_buf_iorequest Christoph Hellwig
  2011-08-23  8:28 ` [PATCH 02/11] xfs: remove the unlock argument to xfs_buf_delwri_queue Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:22   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 04/11] xfs: call xfs_buf_delwri_queue directly Christoph Hellwig
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-simplify-xfs_buf_delwri_queue-2 --]
[-- Type: text/plain, Size: 2394 bytes --]

Do not transfer a reference held by the caller to the buffer on the list,
or decrement it in xfs_buf_delwri_queue, but instead grab a new reference
if needed, and let the caller drop its own reference.  Also move setting
of the XBF_DELWRI and XBF_ASYNC flags into xfs_buf_delwri_queue, and
only do it if needed.  Note that for now xfs_buf_unlock already has
XBF_DELWRI, but that will change in the following patches.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

Index: xfs/fs/xfs/xfs_buf.c
===================================================================
--- xfs.orig/fs/xfs/xfs_buf.c	2011-08-13 06:36:49.564247143 -0700
+++ xfs/fs/xfs/xfs_buf.c	2011-08-13 06:37:38.907313161 -0700
@@ -937,11 +937,8 @@ void
 xfs_buf_unlock(
 	struct xfs_buf		*bp)
 {
-	if ((bp->b_flags & (XBF_DELWRI|_XBF_DELWRI_Q)) == XBF_DELWRI) {
-		atomic_inc(&bp->b_hold);
-		bp->b_flags |= XBF_ASYNC;
+	if ((bp->b_flags & (XBF_DELWRI|_XBF_DELWRI_Q)) == XBF_DELWRI)
 		xfs_buf_delwri_queue(bp);
-	}
 
 	XB_CLEAR_OWNER(bp);
 	up(&bp->b_sema);
@@ -1046,11 +1043,8 @@ xfs_bdwrite(
 {
 	trace_xfs_buf_bdwrite(bp, _RET_IP_);
 
-	bp->b_flags &= ~XBF_READ;
-	bp->b_flags |= (XBF_DELWRI | XBF_ASYNC);
-
 	xfs_buf_delwri_queue(bp);
-	xfs_buf_unlock(bp);
+	xfs_buf_relse(bp);
 }
 
 /*
@@ -1570,23 +1564,22 @@ xfs_buf_delwri_queue(
 
 	trace_xfs_buf_delwri_queue(bp, _RET_IP_);
 
-	ASSERT((bp->b_flags&(XBF_DELWRI|XBF_ASYNC)) == (XBF_DELWRI|XBF_ASYNC));
+	ASSERT(!(bp->b_flags & XBF_READ));
 
 	spin_lock(dwlk);
-	/* If already in the queue, dequeue and place at tail */
 	if (!list_empty(&bp->b_list)) {
+		/* if already in the queue, move it to the tail */
 		ASSERT(bp->b_flags & _XBF_DELWRI_Q);
-		atomic_dec(&bp->b_hold);
-		list_del(&bp->b_list);
-	}
-
-	if (list_empty(dwq)) {
+		list_move_tail(&bp->b_list, dwq);
+	} else {
 		/* start xfsbufd as it is about to have something to do */
-		wake_up_process(bp->b_target->bt_task);
-	}
+		if (list_empty(dwq))
+			wake_up_process(bp->b_target->bt_task);
 
-	bp->b_flags |= _XBF_DELWRI_Q;
-	list_add_tail(&bp->b_list, dwq);
+		atomic_inc(&bp->b_hold);
+		bp->b_flags |= XBF_DELWRI | _XBF_DELWRI_Q | XBF_ASYNC;
+		list_add_tail(&bp->b_list, dwq);
+	}
 	bp->b_queuetime = jiffies;
 	spin_unlock(dwlk);
 }

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 04/11] xfs: call xfs_buf_delwri_queue directly
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2011-08-23  8:28 ` [PATCH 03/11] xfs: move more delwri setup into xfs_buf_delwri_queue Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:22   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 05/11] xfs: let xfs_bwrite callers handle the xfs_buf_relse Christoph Hellwig
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-call-xfs_buf_delwri_queue-directly --]
[-- Type: text/plain, Size: 9624 bytes --]

Unify the ways we add buffers to the delwri queue by always calling
xfs_buf_delwri_queue directly.  The xfs_bdwrite functions is removed and
opencoded in its callers, and the two places setting XBF_DELWRI while a
buffer is locked and expecting xfs_buf_unlock to pick it up are converted
to call xfs_buf_delwri_queue directly, too.  Also replace the
XFS_BUF_UNDELAYWRITE macro with direct calls to xfs_buf_delwri_dequeue
to make the explicit queuing/dequeuing more obvious.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

Index: xfs/fs/xfs/xfs_buf.c
===================================================================
--- xfs.orig/fs/xfs/xfs_buf.c	2011-08-23 03:56:16.198380574 +0200
+++ xfs/fs/xfs/xfs_buf.c	2011-08-23 03:56:36.038273093 +0200
@@ -43,7 +43,6 @@
 
 static kmem_zone_t *xfs_buf_zone;
 STATIC int xfsbufd(void *);
-STATIC void xfs_buf_delwri_queue(xfs_buf_t *);
 
 static struct workqueue_struct *xfslogd_workqueue;
 struct workqueue_struct *xfsdatad_workqueue;
@@ -937,9 +936,6 @@ void
 xfs_buf_unlock(
 	struct xfs_buf		*bp)
 {
-	if ((bp->b_flags & (XBF_DELWRI|_XBF_DELWRI_Q)) == XBF_DELWRI)
-		xfs_buf_delwri_queue(bp);
-
 	XB_CLEAR_OWNER(bp);
 	up(&bp->b_sema);
 
@@ -1036,17 +1032,6 @@ xfs_bwrite(
 	return error;
 }
 
-void
-xfs_bdwrite(
-	void			*mp,
-	struct xfs_buf		*bp)
-{
-	trace_xfs_buf_bdwrite(bp, _RET_IP_);
-
-	xfs_buf_delwri_queue(bp);
-	xfs_buf_relse(bp);
-}
-
 /*
  * Called when we want to stop a buffer from getting written or read.
  * We attach the EIO error, muck with its flags, and call xfs_buf_ioend
@@ -1069,7 +1054,7 @@ xfs_bioerror(
 	 * We're calling xfs_buf_ioend, so delete XBF_DONE flag.
 	 */
 	XFS_BUF_UNREAD(bp);
-	XFS_BUF_UNDELAYWRITE(bp);
+	xfs_buf_delwri_dequeue(bp);
 	XFS_BUF_UNDONE(bp);
 	XFS_BUF_STALE(bp);
 
@@ -1098,7 +1083,7 @@ xfs_bioerror_relse(
 	 * change that interface.
 	 */
 	XFS_BUF_UNREAD(bp);
-	XFS_BUF_UNDELAYWRITE(bp);
+	xfs_buf_delwri_dequeue(bp);
 	XFS_BUF_DONE(bp);
 	XFS_BUF_STALE(bp);
 	bp->b_iodone = NULL;
@@ -1555,7 +1540,7 @@ error:
 /*
  *	Delayed write buffer handling
  */
-STATIC void
+void
 xfs_buf_delwri_queue(
 	xfs_buf_t		*bp)
 {
Index: xfs/fs/xfs/xfs_buf.h
===================================================================
--- xfs.orig/fs/xfs/xfs_buf.h	2011-08-23 03:55:16.702036227 +0200
+++ xfs/fs/xfs/xfs_buf.h	2011-08-23 03:56:17.315041192 +0200
@@ -198,7 +198,6 @@ extern void xfs_buf_unlock(xfs_buf_t *);
 
 /* Buffer Read and Write Routines */
 extern int xfs_bwrite(struct xfs_mount *mp, struct xfs_buf *bp);
-extern void xfs_bdwrite(void *mp, xfs_buf_t *bp);
 
 extern void xfsbdstrat(struct xfs_mount *, struct xfs_buf *);
 extern int xfs_bdstrat_cb(struct xfs_buf *);
@@ -221,8 +220,9 @@ static inline int xfs_buf_geterror(xfs_b
 extern xfs_caddr_t xfs_buf_offset(xfs_buf_t *, size_t);
 
 /* Delayed Write Buffer Routines */
-extern void xfs_buf_delwri_dequeue(xfs_buf_t *);
-extern void xfs_buf_delwri_promote(xfs_buf_t *);
+extern void xfs_buf_delwri_queue(struct xfs_buf *);
+extern void xfs_buf_delwri_dequeue(struct xfs_buf *);
+extern void xfs_buf_delwri_promote(struct xfs_buf *);
 
 /* Buffer Daemon Setup Routines */
 extern int xfs_buf_init(void);
@@ -251,8 +251,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 					XFS_BUF_DONE(bp);	\
 				} while (0)
 
-#define XFS_BUF_DELAYWRITE(bp)		((bp)->b_flags |= XBF_DELWRI)
-#define XFS_BUF_UNDELAYWRITE(bp)	xfs_buf_delwri_dequeue(bp)
 #define XFS_BUF_ISDELAYWRITE(bp)	((bp)->b_flags & XBF_DELWRI)
 
 #define XFS_BUF_DONE(bp)	((bp)->b_flags |= XBF_DONE)
Index: xfs/fs/xfs/xfs_trace.h
===================================================================
--- xfs.orig/fs/xfs/xfs_trace.h	2011-08-23 03:55:16.705369543 +0200
+++ xfs/fs/xfs/xfs_trace.h	2011-08-23 03:56:17.318374507 +0200
@@ -320,7 +320,6 @@ DEFINE_BUF_EVENT(xfs_buf_rele);
 DEFINE_BUF_EVENT(xfs_buf_iodone);
 DEFINE_BUF_EVENT(xfs_buf_iorequest);
 DEFINE_BUF_EVENT(xfs_buf_bawrite);
-DEFINE_BUF_EVENT(xfs_buf_bdwrite);
 DEFINE_BUF_EVENT(xfs_buf_lock);
 DEFINE_BUF_EVENT(xfs_buf_lock_done);
 DEFINE_BUF_EVENT(xfs_buf_trylock);
Index: xfs/fs/xfs/xfs_dquot.c
===================================================================
--- xfs.orig/fs/xfs/xfs_dquot.c	2011-08-23 03:55:16.712036173 +0200
+++ xfs/fs/xfs/xfs_dquot.c	2011-08-23 03:56:17.318374507 +0200
@@ -1243,8 +1243,10 @@ xfs_qm_dqflush(
 
 	if (flags & SYNC_WAIT)
 		error = xfs_bwrite(mp, bp);
-	else
-		xfs_bdwrite(mp, bp);
+	else {
+		xfs_buf_delwri_queue(bp);
+		xfs_buf_relse(bp);
+	}
 
 	trace_xfs_dqflush_done(dqp);
 
Index: xfs/fs/xfs/xfs_qm.c
===================================================================
--- xfs.orig/fs/xfs/xfs_qm.c	2011-08-23 03:55:16.718702803 +0200
+++ xfs/fs/xfs/xfs_qm.c	2011-08-23 03:56:17.321707822 +0200
@@ -1296,7 +1296,8 @@ xfs_qm_dqiter_bufs(
 			break;
 
 		xfs_qm_reset_dqcounts(mp, bp, firstid, type);
-		xfs_bdwrite(mp, bp);
+		xfs_buf_delwri_queue(bp);
+		xfs_buf_relse(bp);
 		/*
 		 * goto the next block.
 		 */
Index: xfs/fs/xfs/xfs_attr.c
===================================================================
--- xfs.orig/fs/xfs/xfs_attr.c	2011-08-23 03:55:16.725369434 +0200
+++ xfs/fs/xfs/xfs_attr.c	2011-08-23 03:56:17.325041137 +0200
@@ -2189,7 +2189,7 @@ xfs_attr_rmtval_remove(xfs_da_args_t *ar
 		bp = xfs_incore(mp->m_ddev_targp, dblkno, blkcnt, XBF_TRYLOCK);
 		if (bp) {
 			XFS_BUF_STALE(bp);
-			XFS_BUF_UNDELAYWRITE(bp);
+			xfs_buf_delwri_dequeue(bp);
 			xfs_buf_relse(bp);
 			bp = NULL;
 		}
Index: xfs/fs/xfs/xfs_buf_item.c
===================================================================
--- xfs.orig/fs/xfs/xfs_buf_item.c	2011-08-23 03:55:16.732036066 +0200
+++ xfs/fs/xfs/xfs_buf_item.c	2011-08-23 03:56:17.325041137 +0200
@@ -992,7 +992,7 @@ xfs_buf_iodone_callbacks(
 		xfs_buf_ioerror(bp, 0); /* errno of 0 unsets the flag */
 
 		if (!XFS_BUF_ISSTALE(bp)) {
-			XFS_BUF_DELAYWRITE(bp);
+			xfs_buf_delwri_queue(bp);
 			XFS_BUF_DONE(bp);
 		}
 		ASSERT(bp->b_iodone != NULL);
@@ -1007,7 +1007,7 @@ xfs_buf_iodone_callbacks(
 	 */
 	XFS_BUF_STALE(bp);
 	XFS_BUF_DONE(bp);
-	XFS_BUF_UNDELAYWRITE(bp);
+	xfs_buf_delwri_dequeue(bp);
 
 	trace_xfs_buf_error_relse(bp, _RET_IP_);
 
Index: xfs/fs/xfs/xfs_inode.c
===================================================================
--- xfs.orig/fs/xfs/xfs_inode.c	2011-08-23 03:55:16.735369381 +0200
+++ xfs/fs/xfs/xfs_inode.c	2011-08-23 03:56:17.328374452 +0200
@@ -2598,8 +2598,10 @@ xfs_iflush(
 
 	if (flags & SYNC_WAIT)
 		error = xfs_bwrite(mp, bp);
-	else
-		xfs_bdwrite(mp, bp);
+	else {
+		xfs_buf_delwri_queue(bp);
+		xfs_buf_relse(bp);
+	}
 	return error;
 
 corrupt_out:
Index: xfs/fs/xfs/xfs_log_recover.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log_recover.c	2011-08-23 03:55:16.742036011 +0200
+++ xfs/fs/xfs/xfs_log_recover.c	2011-08-23 03:56:17.331707768 +0200
@@ -2176,7 +2176,8 @@ xlog_recover_buffer_pass2(
 	} else {
 		ASSERT(bp->b_target->bt_mount == mp);
 		bp->b_iodone = xlog_recover_iodone;
-		xfs_bdwrite(mp, bp);
+		xfs_buf_delwri_queue(bp);
+		xfs_buf_relse(bp);
 	}
 
 	return (error);
@@ -2439,7 +2440,8 @@ xlog_recover_inode_pass2(
 write_inode_buffer:
 	ASSERT(bp->b_target->bt_mount == mp);
 	bp->b_iodone = xlog_recover_iodone;
-	xfs_bdwrite(mp, bp);
+	xfs_buf_delwri_queue(bp);
+	xfs_buf_relse(bp);
 error:
 	if (need_free)
 		kmem_free(in_f);
@@ -2561,7 +2563,8 @@ xlog_recover_dquot_pass2(
 	ASSERT(dq_f->qlf_size == 2);
 	ASSERT(bp->b_target->bt_mount == mp);
 	bp->b_iodone = xlog_recover_iodone;
-	xfs_bdwrite(mp, bp);
+	xfs_buf_delwri_queue(bp);
+	xfs_buf_relse(bp);
 
 	return (0);
 }
Index: xfs/fs/xfs/xfs_mount.c
===================================================================
--- xfs.orig/fs/xfs/xfs_mount.c	2011-08-23 03:55:16.748702641 +0200
+++ xfs/fs/xfs/xfs_mount.c	2011-08-23 03:56:17.331707768 +0200
@@ -1612,7 +1612,7 @@ xfs_unmountfs_writesb(xfs_mount_t *mp)
 
 		XFS_BUF_UNDONE(sbp);
 		XFS_BUF_UNREAD(sbp);
-		XFS_BUF_UNDELAYWRITE(sbp);
+		xfs_buf_delwri_dequeue(sbp);
 		XFS_BUF_WRITE(sbp);
 		XFS_BUF_UNASYNC(sbp);
 		ASSERT(sbp->b_target == mp->m_ddev_targp);
Index: xfs/fs/xfs/xfs_rw.c
===================================================================
--- xfs.orig/fs/xfs/xfs_rw.c	2011-08-23 03:55:16.755369271 +0200
+++ xfs/fs/xfs/xfs_rw.c	2011-08-23 03:56:17.335041084 +0200
@@ -149,7 +149,7 @@ xfs_read_buf(
 		}
 		if (bp) {
 			XFS_BUF_UNDONE(bp);
-			XFS_BUF_UNDELAYWRITE(bp);
+			xfs_buf_delwri_dequeue(bp);
 			XFS_BUF_STALE(bp);
 			/*
 			 * brelse clears B_ERROR and b_error
Index: xfs/fs/xfs/xfs_trans_buf.c
===================================================================
--- xfs.orig/fs/xfs/xfs_trans_buf.c	2011-08-23 03:55:16.762035902 +0200
+++ xfs/fs/xfs/xfs_trans_buf.c	2011-08-23 03:56:17.335041084 +0200
@@ -643,13 +643,14 @@ xfs_trans_log_buf(xfs_trans_t	*tp,
 	 * inside the b_bdstrat callback so that this won't get written to
 	 * disk.
 	 */
-	XFS_BUF_DELAYWRITE(bp);
 	XFS_BUF_DONE(bp);
 
 	ASSERT(atomic_read(&bip->bli_refcount) > 0);
 	bp->b_iodone = xfs_buf_iodone_callbacks;
 	bip->bli_item.li_cb = xfs_buf_iodone;
 
+	xfs_buf_delwri_queue(bp);
+
 	trace_xfs_trans_log_buf(bip);
 
 	/*
@@ -738,7 +739,7 @@ xfs_trans_binval(
 	 * We set the stale bit in the buffer as well since we're getting
 	 * rid of it.
 	 */
-	XFS_BUF_UNDELAYWRITE(bp);
+	xfs_buf_delwri_dequeue(bp);
 	XFS_BUF_STALE(bp);
 	bip->bli_flags |= XFS_BLI_STALE;
 	bip->bli_flags &= ~(XFS_BLI_INODE_BUF | XFS_BLI_LOGGED | XFS_BLI_DIRTY);

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 05/11] xfs: let xfs_bwrite callers handle the xfs_buf_relse
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2011-08-23  8:28 ` [PATCH 04/11] xfs: call xfs_buf_delwri_queue directly Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:22   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 06/11] xfs: use the "delwri" terminology consistently Christoph Hellwig
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-bwrite-dont-unlock --]
[-- Type: text/plain, Size: 7404 bytes --]

Remove the xfs_buf_relse from xfs_bwrite and let the caller handle it to
mirror the delwri and read paths.

Also remove the mount pointer passed to xfs_bwrite, which is superflous now
that we have a mount pointer in the buftarg.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

Index: xfs/fs/xfs/xfs_buf.c
===================================================================
--- xfs.orig/fs/xfs/xfs_buf.c	2011-08-13 06:37:41.257300430 -0700
+++ xfs/fs/xfs/xfs_buf.c	2011-08-13 06:38:15.950445815 -0700
@@ -1014,7 +1014,6 @@ xfs_buf_ioerror(
 
 int
 xfs_bwrite(
-	struct xfs_mount	*mp,
 	struct xfs_buf		*bp)
 {
 	int			error;
@@ -1026,9 +1025,10 @@ xfs_bwrite(
 	xfs_bdstrat_cb(bp);
 
 	error = xfs_buf_iowait(bp);
-	if (error)
-		xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR);
-	xfs_buf_relse(bp);
+	if (error) {
+		xfs_force_shutdown(bp->b_target->bt_mount,
+				   SHUTDOWN_META_IO_ERROR);
+	}
 	return error;
 }
 
Index: xfs/fs/xfs/xfs_buf.h
===================================================================
--- xfs.orig/fs/xfs/xfs_buf.h	2011-08-13 06:37:41.257300430 -0700
+++ xfs/fs/xfs/xfs_buf.h	2011-08-13 06:38:15.950445815 -0700
@@ -197,7 +197,7 @@ extern void xfs_buf_unlock(xfs_buf_t *);
 	((bp)->b_sema.count <= 0)
 
 /* Buffer Read and Write Routines */
-extern int xfs_bwrite(struct xfs_mount *mp, struct xfs_buf *bp);
+extern int xfs_bwrite(struct xfs_buf *bp);
 
 extern void xfsbdstrat(struct xfs_mount *, struct xfs_buf *);
 extern int xfs_bdstrat_cb(struct xfs_buf *);
Index: xfs/fs/xfs/xfs_sync.c
===================================================================
--- xfs.orig/fs/xfs/xfs_sync.c	2011-08-13 06:35:33.177994295 -0700
+++ xfs/fs/xfs/xfs_sync.c	2011-08-13 06:38:15.950445815 -0700
@@ -322,6 +322,7 @@ xfs_sync_fsdata(
 	struct xfs_mount	*mp)
 {
 	struct xfs_buf		*bp;
+	int			error;
 
 	/*
 	 * If the buffer is pinned then push on the log so we won't get stuck
@@ -334,8 +335,9 @@ xfs_sync_fsdata(
 	bp = xfs_getsb(mp, 0);
 	if (xfs_buf_ispinned(bp))
 		xfs_log_force(mp, 0);
-
-	return xfs_bwrite(mp, bp);
+	error = xfs_bwrite(bp);
+	xfs_buf_relse(bp);
+	return error;
 }
 
 /*
Index: xfs/fs/xfs/xfs_dquot.c
===================================================================
--- xfs.orig/fs/xfs/xfs_dquot.c	2011-08-13 06:37:41.260633745 -0700
+++ xfs/fs/xfs/xfs_dquot.c	2011-08-13 06:38:15.953779130 -0700
@@ -1242,11 +1242,11 @@ xfs_qm_dqflush(
 	}
 
 	if (flags & SYNC_WAIT)
-		error = xfs_bwrite(mp, bp);
-	else {
+		error = xfs_bwrite(bp);
+	else
 		xfs_buf_delwri_queue(bp);
-		xfs_buf_relse(bp);
-	}
+
+	xfs_buf_relse(bp);
 
 	trace_xfs_dqflush_done(dqp);
 
Index: xfs/fs/xfs/xfs_attr.c
===================================================================
--- xfs.orig/fs/xfs/xfs_attr.c	2011-08-13 06:37:41.000000000 -0700
+++ xfs/fs/xfs/xfs_attr.c	2011-08-13 06:38:15.957112445 -0700
@@ -2128,9 +2128,10 @@ xfs_attr_rmtval_set(xfs_da_args_t *args)
 		xfs_buf_iomove(bp, 0, tmp, src, XBRW_WRITE);
 		if (tmp < XFS_BUF_SIZE(bp))
 			xfs_buf_zero(bp, tmp, XFS_BUF_SIZE(bp) - tmp);
-		if ((error = xfs_bwrite(mp, bp))) {/* GROT: NOTE: synchronous write */
-			return (error);
-		}
+		error = xfs_bwrite(bp);	/* GROT: NOTE: synchronous write */
+		xfs_buf_relse(bp);
+		if (error)
+			return error;
 		src += tmp;
 		valuelen -= tmp;
 
Index: xfs/fs/xfs/xfs_fsops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_fsops.c	2011-08-13 06:35:14.000000000 -0700
+++ xfs/fs/xfs/xfs_fsops.c	2011-08-13 06:38:15.957112445 -0700
@@ -216,10 +216,11 @@ xfs_growfs_data_private(
 		tmpsize = agsize - XFS_PREALLOC_BLOCKS(mp);
 		agf->agf_freeblks = cpu_to_be32(tmpsize);
 		agf->agf_longest = cpu_to_be32(tmpsize);
-		error = xfs_bwrite(mp, bp);
-		if (error) {
+		error = xfs_bwrite(bp);
+		xfs_buf_relse(bp);
+		if (error)
 			goto error0;
-		}
+
 		/*
 		 * AG inode header block
 		 */
@@ -240,10 +241,11 @@ xfs_growfs_data_private(
 		agi->agi_dirino = cpu_to_be32(NULLAGINO);
 		for (bucket = 0; bucket < XFS_AGI_UNLINKED_BUCKETS; bucket++)
 			agi->agi_unlinked[bucket] = cpu_to_be32(NULLAGINO);
-		error = xfs_bwrite(mp, bp);
-		if (error) {
+		error = xfs_bwrite(bp);
+		xfs_buf_relse(bp);
+		if (error)
 			goto error0;
-		}
+
 		/*
 		 * BNO btree root block
 		 */
@@ -262,10 +264,11 @@ xfs_growfs_data_private(
 		arec->ar_startblock = cpu_to_be32(XFS_PREALLOC_BLOCKS(mp));
 		arec->ar_blockcount = cpu_to_be32(
 			agsize - be32_to_cpu(arec->ar_startblock));
-		error = xfs_bwrite(mp, bp);
-		if (error) {
+		error = xfs_bwrite(bp);
+		xfs_buf_relse(bp);
+		if (error)
 			goto error0;
-		}
+
 		/*
 		 * CNT btree root block
 		 */
@@ -285,10 +288,11 @@ xfs_growfs_data_private(
 		arec->ar_blockcount = cpu_to_be32(
 			agsize - be32_to_cpu(arec->ar_startblock));
 		nfree += be32_to_cpu(arec->ar_blockcount);
-		error = xfs_bwrite(mp, bp);
-		if (error) {
+		error = xfs_bwrite(bp);
+		xfs_buf_relse(bp);
+		if (error)
 			goto error0;
-		}
+
 		/*
 		 * INO btree root block
 		 */
@@ -303,10 +307,10 @@ xfs_growfs_data_private(
 		block->bb_numrecs = 0;
 		block->bb_u.s.bb_leftsib = cpu_to_be32(NULLAGBLOCK);
 		block->bb_u.s.bb_rightsib = cpu_to_be32(NULLAGBLOCK);
-		error = xfs_bwrite(mp, bp);
-		if (error) {
+		error = xfs_bwrite(bp);
+		xfs_buf_relse(bp);
+		if (error)
 			goto error0;
-		}
 	}
 	xfs_trans_agblocks_delta(tp, nfree);
 	/*
@@ -396,9 +400,9 @@ xfs_growfs_data_private(
 		 * just issue a warning and continue.  The real work is
 		 * already done and committed.
 		 */
-		if (!(error = xfs_bwrite(mp, bp))) {
-			continue;
-		} else {
+		error = xfs_bwrite(bp);
+		xfs_buf_relse(bp);
+		if (error) {
 			xfs_warn(mp,
 		"write error %d updating secondary superblock for ag %d",
 				error, agno);
Index: xfs/fs/xfs/xfs_inode.c
===================================================================
--- xfs.orig/fs/xfs/xfs_inode.c	2011-08-13 06:37:41.000000000 -0700
+++ xfs/fs/xfs/xfs_inode.c	2011-08-13 06:38:15.960445760 -0700
@@ -2597,11 +2597,11 @@ xfs_iflush(
 		goto cluster_corrupt_out;
 
 	if (flags & SYNC_WAIT)
-		error = xfs_bwrite(mp, bp);
-	else {
+		error = xfs_bwrite(bp);
+	else
 		xfs_buf_delwri_queue(bp);
-		xfs_buf_relse(bp);
-	}
+
+	xfs_buf_relse(bp);
 	return error;
 
 corrupt_out:
Index: xfs/fs/xfs/xfs_log_recover.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log_recover.c	2011-08-13 06:37:41.000000000 -0700
+++ xfs/fs/xfs/xfs_log_recover.c	2011-08-13 06:38:15.960445760 -0700
@@ -268,9 +268,12 @@ xlog_bwrite(
 	xfs_buf_lock(bp);
 	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
 
-	if ((error = xfs_bwrite(log->l_mp, bp)))
+	error = xfs_bwrite(bp);
+	if (error) {
 		xfs_ioerror_alert("xlog_bwrite", log->l_mp,
 				  bp, XFS_BUF_ADDR(bp));
+	}
+	xfs_buf_relse(bp);
 	return error;
 }
 
@@ -2172,15 +2175,15 @@ xlog_recover_buffer_pass2(
 	    (XFS_BUF_COUNT(bp) != MAX(log->l_mp->m_sb.sb_blocksize,
 			(__uint32_t)XFS_INODE_CLUSTER_SIZE(log->l_mp)))) {
 		XFS_BUF_STALE(bp);
-		error = xfs_bwrite(mp, bp);
+		error = xfs_bwrite(bp);
 	} else {
 		ASSERT(bp->b_target->bt_mount == mp);
 		bp->b_iodone = xlog_recover_iodone;
 		xfs_buf_delwri_queue(bp);
-		xfs_buf_relse(bp);
 	}
 
-	return (error);
+	xfs_buf_relse(bp);
+	return error;
 }
 
 STATIC int

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 06/11] xfs: use the "delwri" terminology consistently
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2011-08-23  8:28 ` [PATCH 05/11] xfs: let xfs_bwrite callers handle the xfs_buf_relse Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:22   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 07/11] xfs: remove dead ENODEV handling in xfs_destroy_ioend Christoph Hellwig
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-cleanup-delwrite --]
[-- Type: text/plain, Size: 4805 bytes --]

And also remove the strange local lock and delwri list pointers in a few
functions.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

Index: xfs/fs/xfs/xfs_buf.c
===================================================================
--- xfs.orig/fs/xfs/xfs_buf.c	2011-08-11 22:37:58.616710453 +0200
+++ xfs/fs/xfs/xfs_buf.c	2011-08-11 22:41:08.463367486 +0200
@@ -1489,12 +1489,12 @@ xfs_setsize_buftarg(
 }
 
 STATIC int
-xfs_alloc_delwrite_queue(
+xfs_alloc_delwri_queue(
 	xfs_buftarg_t		*btp,
 	const char		*fsname)
 {
-	INIT_LIST_HEAD(&btp->bt_delwrite_queue);
-	spin_lock_init(&btp->bt_delwrite_lock);
+	INIT_LIST_HEAD(&btp->bt_delwri_queue);
+	spin_lock_init(&btp->bt_delwri_lock);
 	btp->bt_flags = 0;
 	btp->bt_task = kthread_run(xfsbufd, btp, "xfsbufd/%s", fsname);
 	if (IS_ERR(btp->bt_task))
@@ -1524,7 +1524,7 @@ xfs_alloc_buftarg(
 	spin_lock_init(&btp->bt_lru_lock);
 	if (xfs_setsize_buftarg_early(btp, bdev))
 		goto error;
-	if (xfs_alloc_delwrite_queue(btp, fsname))
+	if (xfs_alloc_delwri_queue(btp, fsname))
 		goto error;
 	btp->bt_shrinker.shrink = xfs_buftarg_shrink;
 	btp->bt_shrinker.seeks = DEFAULT_SEEKS;
@@ -1544,46 +1544,44 @@ STATIC void
 xfs_buf_delwri_queue(
 	xfs_buf_t		*bp)
 {
-	struct list_head	*dwq = &bp->b_target->bt_delwrite_queue;
-	spinlock_t		*dwlk = &bp->b_target->bt_delwrite_lock;
+	struct xfs_buftarg	*btp = bp->b_target;
 
 	trace_xfs_buf_delwri_queue(bp, _RET_IP_);
 
 	ASSERT(!(bp->b_flags & XBF_READ));
 
-	spin_lock(dwlk);
+	spin_lock(&btp->bt_delwri_lock);
 	if (!list_empty(&bp->b_list)) {
 		/* if already in the queue, move it to the tail */
 		ASSERT(bp->b_flags & _XBF_DELWRI_Q);
-		list_move_tail(&bp->b_list, dwq);
+		list_move_tail(&bp->b_list, &btp->bt_delwri_queue);
 	} else {
 		/* start xfsbufd as it is about to have something to do */
-		if (list_empty(dwq))
+		if (list_empty(&btp->bt_delwri_queue))
 			wake_up_process(bp->b_target->bt_task);
 
 		atomic_inc(&bp->b_hold);
 		bp->b_flags |= XBF_DELWRI | _XBF_DELWRI_Q | XBF_ASYNC;
-		list_add_tail(&bp->b_list, dwq);
+		list_add_tail(&bp->b_list, &btp->bt_delwri_queue);
 	}
 	bp->b_queuetime = jiffies;
-	spin_unlock(dwlk);
+	spin_unlock(&btp->bt_delwri_lock);
 }
 
 void
 xfs_buf_delwri_dequeue(
 	xfs_buf_t		*bp)
 {
-	spinlock_t		*dwlk = &bp->b_target->bt_delwrite_lock;
 	int			dequeued = 0;
 
-	spin_lock(dwlk);
+	spin_lock(&bp->b_target->bt_delwri_lock);
 	if ((bp->b_flags & XBF_DELWRI) && !list_empty(&bp->b_list)) {
 		ASSERT(bp->b_flags & _XBF_DELWRI_Q);
 		list_del_init(&bp->b_list);
 		dequeued = 1;
 	}
 	bp->b_flags &= ~(XBF_DELWRI|_XBF_DELWRI_Q);
-	spin_unlock(dwlk);
+	spin_unlock(&bp->b_target->bt_delwri_lock);
 
 	if (dequeued)
 		xfs_buf_rele(bp);
@@ -1615,9 +1613,9 @@ xfs_buf_delwri_promote(
 	if (bp->b_queuetime < jiffies - age)
 		return;
 	bp->b_queuetime = jiffies - age;
-	spin_lock(&btp->bt_delwrite_lock);
-	list_move(&bp->b_list, &btp->bt_delwrite_queue);
-	spin_unlock(&btp->bt_delwrite_lock);
+	spin_lock(&btp->bt_delwri_lock);
+	list_move(&bp->b_list, &btp->bt_delwri_queue);
+	spin_unlock(&btp->bt_delwri_lock);
 }
 
 STATIC void
@@ -1638,15 +1636,13 @@ xfs_buf_delwri_split(
 	unsigned long	age)
 {
 	xfs_buf_t	*bp, *n;
-	struct list_head *dwq = &target->bt_delwrite_queue;
-	spinlock_t	*dwlk = &target->bt_delwrite_lock;
 	int		skipped = 0;
 	int		force;
 
 	force = test_and_clear_bit(XBT_FORCE_FLUSH, &target->bt_flags);
 	INIT_LIST_HEAD(list);
-	spin_lock(dwlk);
-	list_for_each_entry_safe(bp, n, dwq, b_list) {
+	spin_lock(&target->bt_delwri_lock);
+	list_for_each_entry_safe(bp, n, &target->bt_delwri_queue, b_list) {
 		ASSERT(bp->b_flags & XBF_DELWRI);
 
 		if (!xfs_buf_ispinned(bp) && xfs_buf_trylock(bp)) {
@@ -1663,10 +1659,9 @@ xfs_buf_delwri_split(
 		} else
 			skipped++;
 	}
-	spin_unlock(dwlk);
 
+	spin_unlock(&target->bt_delwri_lock);
 	return skipped;
-
 }
 
 /*
@@ -1716,7 +1711,7 @@ xfsbufd(
 		}
 
 		/* sleep for a long time if there is nothing to do. */
-		if (list_empty(&target->bt_delwrite_queue))
+		if (list_empty(&target->bt_delwri_queue))
 			tout = MAX_SCHEDULE_TIMEOUT;
 		schedule_timeout_interruptible(tout);
 
Index: xfs/fs/xfs/xfs_buf.h
===================================================================
--- xfs.orig/fs/xfs/xfs_buf.h	2011-08-11 22:37:58.620043786 +0200
+++ xfs/fs/xfs/xfs_buf.h	2011-08-11 22:39:35.703372204 +0200
@@ -105,8 +105,8 @@ typedef struct xfs_buftarg {
 
 	/* per device delwri queue */
 	struct task_struct	*bt_task;
-	struct list_head	bt_delwrite_queue;
-	spinlock_t		bt_delwrite_lock;
+	struct list_head	bt_delwri_queue;
+	spinlock_t		bt_delwri_lock;
 	unsigned long		bt_flags;
 
 	/* LRU control structures */

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 07/11] xfs: remove dead ENODEV handling in xfs_destroy_ioend
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2011-08-23  8:28 ` [PATCH 06/11] xfs: use the "delwri" terminology consistently Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:22   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 08/11] xfs: defer AIO/DIO completions Christoph Hellwig
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-remove-dead-ENODEV-handling --]
[-- Type: text/plain, Size: 1111 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>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 08/11] xfs: defer AIO/DIO completions
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2011-08-23  8:28 ` [PATCH 07/11] xfs: remove dead ENODEV handling in xfs_destroy_ioend Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-08-24  0:01   ` Dave Chinner
  2011-09-01 19:22   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 09/11] xfs: reduce ioend latency Christoph Hellwig
                   ` (2 subsequent siblings)
  10 siblings, 2 replies; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-fix-aio-completions --]
[-- Type: text/plain, Size: 2840 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-23 04:39:29.027667347 +0200
+++ xfs/fs/xfs/xfs_aops.c	2011-08-23 04:39:56.914196267 +0200
@@ -122,6 +122,11 @@ xfs_destroy_ioend(
 		bh->b_end_io(bh, !ioend->io_error);
 	}
 
+	if (ioend->io_iocb) {
+		if (ioend->io_isasync)
+			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);
 	}
 }
@@ -274,6 +277,7 @@ xfs_alloc_ioend(
 	 * all the I/O from calling the completion routine too early.
 	 */
 	atomic_set(&ioend->io_remaining, 1);
+	ioend->io_isasync = 0;
 	ioend->io_error = 0;
 	ioend->io_list = NULL;
 	ioend->io_type = type;
@@ -1299,28 +1303,17 @@ 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.
-		 */
-		if (ioend->io_type == IO_UNWRITTEN) {
-			ioend->io_iocb = iocb;
-			ioend->io_result = ret;
-		} else {
-			aio_complete(iocb, ret, 0);
-		}
+		ioend->io_isasync = 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
Index: xfs/fs/xfs/xfs_aops.h
===================================================================
--- xfs.orig/fs/xfs/xfs_aops.h	2011-08-23 04:39:24.057694271 +0200
+++ xfs/fs/xfs/xfs_aops.h	2011-08-23 04:39:29.900995938 +0200
@@ -47,6 +47,7 @@ typedef struct xfs_ioend {
 	unsigned int		io_type;	/* delalloc / unwritten */
 	int			io_error;	/* I/O error code */
 	atomic_t		io_remaining;	/* hold count */
+	unsigned int		io_isasync : 1;	/* needs aio_complete */
 	struct inode		*io_inode;	/* file being written to */
 	struct buffer_head	*io_buffer_head;/* buffer linked list head */
 	struct buffer_head	*io_buffer_tail;/* buffer linked list tail */

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 09/11] xfs: reduce ioend latency
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
                   ` (7 preceding siblings ...)
  2011-08-23  8:28 ` [PATCH 08/11] xfs: defer AIO/DIO completions Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:22   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 10/11] xfs: wait for I/O completion when writing out pages in xfs_setattr_size Christoph Hellwig
  2011-08-23  8:28 ` [PATCH 11/11] xfs: remove i_iocount Christoph Hellwig
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-fix-io-latency --]
[-- Type: text/plain, Size: 1752 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>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 10/11] xfs: wait for I/O completion when writing out pages in xfs_setattr_size
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
                   ` (8 preceding siblings ...)
  2011-08-23  8:28 ` [PATCH 09/11] xfs: reduce ioend latency Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:22   ` Alex Elder
  2011-08-23  8:28 ` [PATCH 11/11] xfs: remove i_iocount Christoph Hellwig
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-setattr-wait-for-writeback --]
[-- Type: text/plain, Size: 1006 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>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 11/11] xfs: remove i_iocount
  2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
                   ` (9 preceding siblings ...)
  2011-08-23  8:28 ` [PATCH 10/11] xfs: wait for I/O completion when writing out pages in xfs_setattr_size Christoph Hellwig
@ 2011-08-23  8:28 ` Christoph Hellwig
  2011-09-01 19:22   ` Alex Elder
  10 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-08-23  8:28 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-remove-iocount --]
[-- Type: text/plain, Size: 8719 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>
Reviewed-by: Dave Chinner <dchinner@redhat.com>

Index: xfs/fs/xfs/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_aops.c	2011-08-23 04:40:00.124165548 +0200
+++ xfs/fs/xfs/xfs_aops.c	2011-08-23 04:40:04.074144199 +0200
@@ -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);
 }
 
@@ -298,7 +263,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;
@@ -558,7 +522,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-23 04:39:23.264365236 +0200
+++ xfs/fs/xfs/xfs_file.c	2011-08-23 04:40:04.077477488 +0200
@@ -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-23 04:39:23.277698496 +0200
+++ xfs/fs/xfs/xfs_super.c	2011-08-23 04:40:04.080810783 +0200
@@ -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-23 04:39:23.287698441 +0200
+++ xfs/fs/xfs/xfs_sync.c	2011-08-23 04:40:04.080810783 +0200
@@ -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-23 04:39:23.297698387 +0200
+++ xfs/fs/xfs/xfs_vnodeops.c	2011-08-23 04:40:04.087477393 +0200
@@ -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-23 04:39:29.900995938 +0200
+++ xfs/fs/xfs/xfs_aops.h	2011-08-23 04:40:04.087477393 +0200
@@ -61,9 +61,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-23 04:40:00.550829903 +0200
+++ xfs/fs/xfs/xfs_iops.c	2011-08-23 04:40:04.090810714 +0200
@@ -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-23 04:39:23.331031541 +0200
+++ xfs/fs/xfs/xfs_iget.c	2011-08-23 04:40:04.094144037 +0200
@@ -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-23 04:39:23.347698116 +0200
+++ xfs/fs/xfs/xfs_inode.h	2011-08-23 04:40:04.097477360 +0200
@@ -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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 08/11] xfs: defer AIO/DIO completions
  2011-08-23  8:28 ` [PATCH 08/11] xfs: defer AIO/DIO completions Christoph Hellwig
@ 2011-08-24  0:01   ` Dave Chinner
  2011-09-01 19:22   ` Alex Elder
  1 sibling, 0 replies; 27+ messages in thread
From: Dave Chinner @ 2011-08-24  0:01 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, Aug 23, 2011 at 04:28:10AM -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>

Much nicer ;)

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 01/11] xfs: remove delwri buffer handling from xfs_buf_iorequest
  2011-08-23  8:28 ` [PATCH 01/11] xfs: remove delwri buffer handling from xfs_buf_iorequest Christoph Hellwig
@ 2011-09-01 19:21   ` Alex Elder
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:21 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -0400, Christoph Hellwig wrote:
> We cannot ever reach xfs_buf_iorequest for a buffer with XBF_DELWRI set,
> given that all write handlers make sure that the buffer is remove from
> the delwri queue before, and we never do reads with the XBF_DELWRI flag
> set (which the code would not handle correctly anyway).
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 02/11] xfs: remove the unlock argument to xfs_buf_delwri_queue
  2011-08-23  8:28 ` [PATCH 02/11] xfs: remove the unlock argument to xfs_buf_delwri_queue Christoph Hellwig
@ 2011-09-01 19:22   ` Alex Elder
  2011-09-01 21:39     ` Christoph Hellwig
  0 siblings, 1 reply; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -0400, Christoph Hellwig wrote:
> We can just unlock the buffer in the caller, and the decrement of b_hold
> would also be needed in the !unlock, we just never hit that case currently
> given that the caller handles that case.

More specifically, the only way we'd hit that case would
involve an unqueued buffer (in xfs_buf_unlock()) getting
queued before bt_delwrite_lock could be acquired (in
xfs_buf_delwri_queue()).  But that can't happen because
the buffer is locked the entire time between the check
in xfs_buf_unlock() and the one in xfs_buf_delwri_queue().
(Right?)

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 03/11] xfs: move more delwri setup into xfs_buf_delwri_queue
  2011-08-23  8:28 ` [PATCH 03/11] xfs: move more delwri setup into xfs_buf_delwri_queue Christoph Hellwig
@ 2011-09-01 19:22   ` Alex Elder
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -0400, Christoph Hellwig wrote:
> Do not transfer a reference held by the caller to the buffer on the list,
> or decrement it in xfs_buf_delwri_queue, but instead grab a new reference
> if needed, and let the caller drop its own reference.  Also move setting
> of the XBF_DELWRI and XBF_ASYNC flags into xfs_buf_delwri_queue, and
> only do it if needed.  Note that for now xfs_buf_unlock already has
> XBF_DELWRI, but that will change in the following patches.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Definite improvement.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 04/11] xfs: call xfs_buf_delwri_queue directly
  2011-08-23  8:28 ` [PATCH 04/11] xfs: call xfs_buf_delwri_queue directly Christoph Hellwig
@ 2011-09-01 19:22   ` Alex Elder
  2011-09-01 21:46     ` Christoph Hellwig
  0 siblings, 1 reply; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -0400, Christoph Hellwig wrote:
> Unify the ways we add buffers to the delwri queue by always calling
> xfs_buf_delwri_queue directly.  The xfs_bdwrite functions is removed and
> opencoded in its callers, and the two places setting XBF_DELWRI while a
> buffer is locked and expecting xfs_buf_unlock to pick it up are converted
> to call xfs_buf_delwri_queue directly, too.  Also replace the
> XFS_BUF_UNDELAYWRITE macro with direct calls to xfs_buf_delwri_dequeue
> to make the explicit queuing/dequeuing more obvious.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

So instead of just marking XBF_DELWRI to flag to signal
that the buffer should be queued at unlock time, we now
just put it in the queue right away.  Seems reasonable.
Do you know why enqueueing it was delayed before?

In any case:
Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 05/11] xfs: let xfs_bwrite callers handle the xfs_buf_relse
  2011-08-23  8:28 ` [PATCH 05/11] xfs: let xfs_bwrite callers handle the xfs_buf_relse Christoph Hellwig
@ 2011-09-01 19:22   ` Alex Elder
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -0400, Christoph Hellwig wrote:
> Remove the xfs_buf_relse from xfs_bwrite and let the caller handle it to
> mirror the delwri and read paths.
> 
> Also remove the mount pointer passed to xfs_bwrite, which is superflous now
> that we have a mount pointer in the buftarg.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Looks good.  In once case you may have fixed a bug
where the buffer pointer was used despite its last
reference being dropped.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 06/11] xfs: use the "delwri" terminology consistently
  2011-08-23  8:28 ` [PATCH 06/11] xfs: use the "delwri" terminology consistently Christoph Hellwig
@ 2011-09-01 19:22   ` Alex Elder
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -0400, Christoph Hellwig wrote:
> And also remove the strange local lock and delwri list pointers in a few
> functions.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 07/11] xfs: remove dead ENODEV handling in xfs_destroy_ioend
  2011-08-23  8:28 ` [PATCH 07/11] xfs: remove dead ENODEV handling in xfs_destroy_ioend Christoph Hellwig
@ 2011-09-01 19:22   ` Alex Elder
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -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>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 08/11] xfs: defer AIO/DIO completions
  2011-08-23  8:28 ` [PATCH 08/11] xfs: defer AIO/DIO completions Christoph Hellwig
  2011-08-24  0:01   ` Dave Chinner
@ 2011-09-01 19:22   ` Alex Elder
  1 sibling, 0 replies; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -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>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 09/11] xfs: reduce ioend latency
  2011-08-23  8:28 ` [PATCH 09/11] xfs: reduce ioend latency Christoph Hellwig
@ 2011-09-01 19:22   ` Alex Elder
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -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>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 10/11] xfs: wait for I/O completion when writing out pages in xfs_setattr_size
  2011-08-23  8:28 ` [PATCH 10/11] xfs: wait for I/O completion when writing out pages in xfs_setattr_size Christoph Hellwig
@ 2011-09-01 19:22   ` Alex Elder
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -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>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 11/11] xfs: remove i_iocount
  2011-08-23  8:28 ` [PATCH 11/11] xfs: remove i_iocount Christoph Hellwig
@ 2011-09-01 19:22   ` Alex Elder
  0 siblings, 0 replies; 27+ messages in thread
From: Alex Elder @ 2011-09-01 19:22 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Tue, 2011-08-23 at 04:28 -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>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 02/11] xfs: remove the unlock argument to xfs_buf_delwri_queue
  2011-09-01 19:22   ` Alex Elder
@ 2011-09-01 21:39     ` Christoph Hellwig
  0 siblings, 0 replies; 27+ messages in thread
From: Christoph Hellwig @ 2011-09-01 21:39 UTC (permalink / raw)
  To: Alex Elder; +Cc: Christoph Hellwig, xfs

On Thu, Sep 01, 2011 at 02:22:04PM -0500, Alex Elder wrote:
> On Tue, 2011-08-23 at 04:28 -0400, Christoph Hellwig wrote:
> > We can just unlock the buffer in the caller, and the decrement of b_hold
> > would also be needed in the !unlock, we just never hit that case currently
> > given that the caller handles that case.
> 
> More specifically, the only way we'd hit that case would
> involve an unqueued buffer (in xfs_buf_unlock()) getting
> queued before bt_delwrite_lock could be acquired (in
> xfs_buf_delwri_queue()).  But that can't happen because
> the buffer is locked the entire time between the check
> in xfs_buf_unlock() and the one in xfs_buf_delwri_queue().
> (Right?)

Exactly.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 04/11] xfs: call xfs_buf_delwri_queue directly
  2011-09-01 19:22   ` Alex Elder
@ 2011-09-01 21:46     ` Christoph Hellwig
  2011-09-02  0:17       ` Dave Chinner
  0 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2011-09-01 21:46 UTC (permalink / raw)
  To: Alex Elder; +Cc: Christoph Hellwig, xfs

On Thu, Sep 01, 2011 at 02:22:15PM -0500, Alex Elder wrote:
> that the buffer should be queued at unlock time, we now
> just put it in the queue right away.  Seems reasonable.
> Do you know why enqueueing it was delayed before?

I don't know why - it's a carryover from IRIX days, so long before my
time.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 04/11] xfs: call xfs_buf_delwri_queue directly
  2011-09-01 21:46     ` Christoph Hellwig
@ 2011-09-02  0:17       ` Dave Chinner
  0 siblings, 0 replies; 27+ messages in thread
From: Dave Chinner @ 2011-09-02  0:17 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs, Alex Elder

On Thu, Sep 01, 2011 at 05:46:40PM -0400, Christoph Hellwig wrote:
> On Thu, Sep 01, 2011 at 02:22:15PM -0500, Alex Elder wrote:
> > that the buffer should be queued at unlock time, we now
> > just put it in the queue right away.  Seems reasonable.
> > Do you know why enqueueing it was delayed before?
> 
> I don't know why - it's a carryover from IRIX days,

Right. On Irix a buffer could simply have the BUF_DELWRI flag set on
it, and the buffer cache would take care of everything else. There
wasn't a specific delayed write list in Irix, instead there was a
fixed table of buffers (large!) that was scanned every 5 seconds
(20% scanned once per second) and the delwri buffers were gathered
by the scan.  Hence there were places in XFS where it simply set the
flag to mark a buffer delwri, and that's what this Linux code had to
catch.

> so long before my time.

The XFS code long predates me, too, but I spend a couple of years in
the trenches around the Irix buffer cache when I was at SGI so I at
least know why it was like that. ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2011-09-02  0:17 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-23  8:28 [PATCH 00/11] patch queue for Linux 3.2 Christoph Hellwig
2011-08-23  8:28 ` [PATCH 01/11] xfs: remove delwri buffer handling from xfs_buf_iorequest Christoph Hellwig
2011-09-01 19:21   ` Alex Elder
2011-08-23  8:28 ` [PATCH 02/11] xfs: remove the unlock argument to xfs_buf_delwri_queue Christoph Hellwig
2011-09-01 19:22   ` Alex Elder
2011-09-01 21:39     ` Christoph Hellwig
2011-08-23  8:28 ` [PATCH 03/11] xfs: move more delwri setup into xfs_buf_delwri_queue Christoph Hellwig
2011-09-01 19:22   ` Alex Elder
2011-08-23  8:28 ` [PATCH 04/11] xfs: call xfs_buf_delwri_queue directly Christoph Hellwig
2011-09-01 19:22   ` Alex Elder
2011-09-01 21:46     ` Christoph Hellwig
2011-09-02  0:17       ` Dave Chinner
2011-08-23  8:28 ` [PATCH 05/11] xfs: let xfs_bwrite callers handle the xfs_buf_relse Christoph Hellwig
2011-09-01 19:22   ` Alex Elder
2011-08-23  8:28 ` [PATCH 06/11] xfs: use the "delwri" terminology consistently Christoph Hellwig
2011-09-01 19:22   ` Alex Elder
2011-08-23  8:28 ` [PATCH 07/11] xfs: remove dead ENODEV handling in xfs_destroy_ioend Christoph Hellwig
2011-09-01 19:22   ` Alex Elder
2011-08-23  8:28 ` [PATCH 08/11] xfs: defer AIO/DIO completions Christoph Hellwig
2011-08-24  0:01   ` Dave Chinner
2011-09-01 19:22   ` Alex Elder
2011-08-23  8:28 ` [PATCH 09/11] xfs: reduce ioend latency Christoph Hellwig
2011-09-01 19:22   ` Alex Elder
2011-08-23  8:28 ` [PATCH 10/11] xfs: wait for I/O completion when writing out pages in xfs_setattr_size Christoph Hellwig
2011-09-01 19:22   ` Alex Elder
2011-08-23  8:28 ` [PATCH 11/11] xfs: remove i_iocount Christoph Hellwig
2011-09-01 19:22   ` Alex Elder

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.