Turn off XBF_READ_AHEAD in io completion
PATCH 1/1 Turn off XBF_READ_AHEAD in io completion
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.
xfs/linux-2.6/xfs_buf.c renamed from xfs/linux-2.6/xfs_buf.c_1.247 +1 -1
@@ -1032,7 +1032,7 @@ xfs_buf_ioend( xfs_buf_t *bp, int schedule) { - bp->b_flags &= ~(XBF_READ |XBF_WRITE ); + bp->b_flags &= ~(XBF_READ |XBF_WRITE |XBF_READ_AHEAD );if (bp->b_error ==0 ) bp->b_flags |=XBF_DONE ;
Quoted 4 lines · Lachlan McIlroy
Read-ahead of an inode cluster will set
XBF_READ_AHEADin 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.
Quoted 11 lines · Christoph Hellwig
Quoted 4 lines · Lachlan McIlroy
Read-ahead of an inode cluster will set
XBF_READ_AHEADin 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_ioapplyand keep the flags just for this I/O separate from those that are permanent and inbp->b_flags.
Okay, I've done that (new patch attached). It's certainly not as clean as the last patch.
xfs/linux-2.6/xfs_buf.c renamed from xfs/linux-2.6/xfs_buf.c_1.247 +12 -10
@@ -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)
xfs/linux-2.6/xfs_buf.h renamed from xfs/linux-2.6/xfs_buf.h_1.122 +5 -5
@@ -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)
xfs/linux-2.6/xfs_lrw.c renamed from xfs/linux-2.6/xfs_lrw.c_1.271 +2 -2
@@ -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 ; }
Quoted 2 lines · Lachlan McIlroy
Okay, I've done that (new patch attached). It's certainly not as clean as the last patch.
Yes, it doesn't really look like an improvement. I'd go with your previous patch and see if we can find a better way to sort out the buffer flags mess later.