Re: [PATCH] Turn off XBF_READ_AHEAD in io completion

[Lachlan McIlroy <lachlan@sgi.com>]

[-- Attachment #1: Type: text/plain, Size: 705 bytes --]

Christoph Hellwig wrote:
> On Thu, Nov 01, 2007 at 05:19:35PM +1100, Lachlan McIlroy wrote:
>> Read-ahead of an inode cluster will set XBF_READ_AHEAD in the buffer.
>> If we don't remove the flag it will still be set when we flush the
>> buffer back to disk.  Not sure if leaving this flag set causes any
>> serious problems but it does trigger an assert.
> 
> It might be better if such temporary flags never actually make it to
> bp->b_flags.  Just pass down a flags variable all the way to
> _xfs_buf_ioapply and keep the flags just for this I/O separate from
> those that are permanent and in bp->b_flags.
> 

Okay, I've done that (new patch attached).  It's certainly not as
clean as the last patch.

[-- Attachment #2: readahead.diff --]
[-- Type: text/x-patch, Size: 4511 bytes --]

--- fs/xfs/linux-2.6/xfs_buf.c_1.247	2007-10-29 16:01:29.000000000 +1100
+++ fs/xfs/linux-2.6/xfs_buf.c	2007-11-02 13:39:48.000000000 +1100
@@ -1020,7 +1020,7 @@ xfs_buf_iodone_work(
 	    (bp->b_flags & (XBF_ORDERED|XBF_ASYNC)) == (XBF_ORDERED|XBF_ASYNC)) {
 		XB_TRACE(bp, "ordered_retry", bp->b_iodone);
 		bp->b_flags &= ~XBF_ORDERED;
-		xfs_buf_iorequest(bp);
+		xfs_buf_iorequest(bp, bp->b_flags);
 	} else if (bp->b_iodone)
 		(*(bp->b_iodone))(bp);
 	else if (bp->b_flags & XBF_ASYNC)
@@ -1082,9 +1082,9 @@ xfs_buf_iostart(
 	}
 
 	bp->b_flags &= ~(XBF_READ | XBF_WRITE | XBF_ASYNC | XBF_DELWRI | \
-			XBF_READ_AHEAD | _XBF_RUN_QUEUES);
+			_XBF_RUN_QUEUES);
 	bp->b_flags |= flags & (XBF_READ | XBF_WRITE | XBF_ASYNC | \
-			XBF_READ_AHEAD | _XBF_RUN_QUEUES);
+			_XBF_RUN_QUEUES);
 
 	BUG_ON(bp->b_bn == XFS_BUF_DADDR_NULL);
 
@@ -1093,7 +1093,7 @@ xfs_buf_iostart(
 	 * a shutdown situation, for example).
 	 */
 	status = (flags & XBF_WRITE) ?
-		xfs_buf_iostrategy(bp) : xfs_buf_iorequest(bp);
+		xfs_buf_iostrategy(bp, flags) : xfs_buf_iorequest(bp, flags);
 
 	/* Wait for I/O if we are not an async request.
 	 * Note: async I/O request completion will release the buffer,
@@ -1172,7 +1172,8 @@ xfs_buf_bio_end_io(
 
 STATIC void
 _xfs_buf_ioapply(
-	xfs_buf_t		*bp)
+	xfs_buf_t		*bp,
+	xfs_buf_flags_t		flags)
 {
 	int			i, rw, map_i, total_nr_pages, nr_pages;
 	struct bio		*bio;
@@ -1194,7 +1195,7 @@ _xfs_buf_ioapply(
 		rw = (bp->b_flags & XBF_WRITE) ? WRITE_SYNC : READ_SYNC;
 	} else {
 		rw = (bp->b_flags & XBF_WRITE) ? WRITE :
-		     (bp->b_flags & XBF_READ_AHEAD) ? READA : READ;
+		     (flags & XBF_READ_AHEAD) ? READA : READ;
 	}
 
 	/* Special code path for reading a sub page size buffer in --
@@ -1279,7 +1280,8 @@ submit_io:
 
 int
 xfs_buf_iorequest(
-	xfs_buf_t		*bp)
+	xfs_buf_t		*bp,
+	xfs_buf_flags_t		flags)
 {
 	XB_TRACE(bp, "iorequest", 0);
 
@@ -1299,7 +1301,7 @@ xfs_buf_iorequest(
 	 * all the I/O from calling xfs_buf_ioend too early.
 	 */
 	atomic_set(&bp->b_io_remaining, 1);
-	_xfs_buf_ioapply(bp);
+	_xfs_buf_ioapply(bp, flags);
 	_xfs_buf_ioend(bp, 0);
 
 	xfs_buf_rele(bp);
@@ -1775,7 +1777,7 @@ xfsbufd(
 			ASSERT(target == bp->b_target);
 
 			list_del_init(&bp->b_list);
-			xfs_buf_iostrategy(bp);
+			xfs_buf_iostrategy(bp, bp->b_flags);
 			count++;
 		}
 
@@ -1819,7 +1821,7 @@ xfs_flush_buftarg(
 		else
 			list_del_init(&bp->b_list);
 
-		xfs_buf_iostrategy(bp);
+		xfs_buf_iostrategy(bp, bp->b_flags);
 	}
 
 	if (wait)
--- fs/xfs/linux-2.6/xfs_buf.h_1.122	2007-11-02 13:34:39.000000000 +1100
+++ fs/xfs/linux-2.6/xfs_buf.h	2007-11-02 13:38:35.000000000 +1100
@@ -191,14 +191,14 @@ extern void xfs_buf_unlock(xfs_buf_t *);
 extern void xfs_buf_ioend(xfs_buf_t *,	int);
 extern void xfs_buf_ioerror(xfs_buf_t *, int);
 extern int xfs_buf_iostart(xfs_buf_t *, xfs_buf_flags_t);
-extern int xfs_buf_iorequest(xfs_buf_t *);
+extern int xfs_buf_iorequest(xfs_buf_t *, xfs_buf_flags_t);
 extern int xfs_buf_iowait(xfs_buf_t *);
 extern void xfs_buf_iomove(xfs_buf_t *, size_t, size_t, xfs_caddr_t,
 				xfs_buf_rw_t);
 
-static inline int xfs_buf_iostrategy(xfs_buf_t *bp)
+static inline int xfs_buf_iostrategy(xfs_buf_t *bp, xfs_buf_flags_t flags)
 {
-	return bp->b_strat ? bp->b_strat(bp) : xfs_buf_iorequest(bp);
+	return bp->b_strat ? bp->b_strat(bp) : xfs_buf_iorequest(bp, flags);
 }
 
 static inline int xfs_buf_geterror(xfs_buf_t *bp)
@@ -380,7 +380,7 @@ static inline int XFS_bwrite(xfs_buf_t *
 		bp->b_flags |= _XBF_RUN_QUEUES;
 
 	xfs_buf_delwri_dequeue(bp);
-	xfs_buf_iostrategy(bp);
+	xfs_buf_iostrategy(bp, bp->b_flags);
 	if (iowait) {
 		error = xfs_buf_iowait(bp);
 		xfs_buf_relse(bp);
@@ -395,7 +395,7 @@ static inline int xfs_bdwrite(void *mp, 
 	return xfs_buf_iostart(bp, XBF_DELWRI | XBF_ASYNC);
 }
 
-#define XFS_bdstrat(bp) xfs_buf_iorequest(bp)
+#define XFS_bdstrat(bp) xfs_buf_iorequest(bp, (bp)->b_flags)
 
 #define xfs_iowait(bp)	xfs_buf_iowait(bp)
 
--- fs/xfs/linux-2.6/xfs_lrw.c_1.271	2007-11-09 11:58:53.000000000 +1100
+++ fs/xfs/linux-2.6/xfs_lrw.c	2007-11-09 11:57:50.000000000 +1100
@@ -878,7 +878,7 @@ xfs_bdstrat_cb(struct xfs_buf *bp)
 
 	mp = XFS_BUF_FSPRIVATE3(bp, xfs_mount_t *);
 	if (!XFS_FORCED_SHUTDOWN(mp)) {
-		xfs_buf_iorequest(bp);
+		xfs_buf_iorequest(bp, bp->b_flags);
 		return 0;
 	} else {
 		xfs_buftrace("XFS__BDSTRAT IOERROR", bp);
@@ -912,7 +912,7 @@ xfsbdstrat(
 		 * if (XFS_BUF_IS_GRIO(bp)) {
 		 */
 
-		xfs_buf_iorequest(bp);
+		xfs_buf_iorequest(bp, bp->b_flags);
 		return 0;
 	}