Commit 0d1120b9 authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Carlos Maiolino
Browse files

xfs: remove most in-flight buffer accounting 



The buffer cache keeps a bt_io_count per-CPU counter to track all
in-flight I/O, which is used to ensure no I/O is in flight when
unmounting the file system.

For most I/O we already keep track of inflight I/O at higher levels:

 - for synchronous I/O (xfs_buf_read/xfs_bwrite/xfs_buf_delwri_submit),
   the caller has a reference and waits for I/O completions using
   xfs_buf_iowait
 - for xfs_buf_delwri_submit_nowait the only caller (AIL writeback)
   tracks the log items that the buffer attached to

This only leaves only xfs_buf_readahead_map as a submitter of
asynchronous I/O that is not tracked by anything else.  Replace the
bt_io_count per-cpu counter with a more specific bt_readahead_count
counter only tracking readahead I/O.  This allows to simply increment
it when submitting readahead I/O and decrementing it when it completed,
and thus simplify xfs_buf_rele and remove the needed for the
XBF_NO_IOACCT flags and the XFS_BSTATE_IN_FLIGHT buffer state.

Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <djwong@kernel.org>
Signed-off-by: default avatarCarlos Maiolino <cem@kernel.org>
parent efc5f7a9

fs/xfs/xfs_buf.c

+15 −75
Original line number Diff line number Diff line
@@ -29,11 +29,6 @@ struct kmem_cache *xfs_buf_cache; 
/*

 * Locking orders

 *

 * xfs_buf_ioacct_inc:

 * xfs_buf_ioacct_dec:

 *	b_sema (caller holds)

 *	  b_lock

 *

 * xfs_buf_stale:

 *	b_sema (caller holds)

 *	  b_lock
@@ -81,51 +76,6 @@ xfs_buf_vmap_len( 
	return (bp->b_page_count * PAGE_SIZE);

}



/*

 * Bump the I/O in flight count on the buftarg if we haven't yet done so for

 * this buffer. The count is incremented once per buffer (per hold cycle)

 * because the corresponding decrement is deferred to buffer release. Buffers

 * can undergo I/O multiple times in a hold-release cycle and per buffer I/O

 * tracking adds unnecessary overhead. This is used for sychronization purposes

 * with unmount (see xfs_buftarg_drain()), so all we really need is a count of

 * in-flight buffers.

 *

 * Buffers that are never released (e.g., superblock, iclog buffers) must set

 * the XBF_NO_IOACCT flag before I/O submission. Otherwise, the buftarg count

 * never reaches zero and unmount hangs indefinitely.

 */

static inline void

xfs_buf_ioacct_inc(

	struct xfs_buf	*bp)

{

	if (bp->b_flags & XBF_NO_IOACCT)

		return;



	ASSERT(bp->b_flags & XBF_ASYNC);

	spin_lock(&bp->b_lock);

	if (!(bp->b_state & XFS_BSTATE_IN_FLIGHT)) {

		bp->b_state |= XFS_BSTATE_IN_FLIGHT;

		percpu_counter_inc(&bp->b_target->bt_io_count);

	}

	spin_unlock(&bp->b_lock);

}



/*

 * Clear the in-flight state on a buffer about to be released to the LRU or

 * freed and unaccount from the buftarg.

 */

static inline void

__xfs_buf_ioacct_dec(

	struct xfs_buf	*bp)

{

	lockdep_assert_held(&bp->b_lock);



	if (bp->b_state & XFS_BSTATE_IN_FLIGHT) {

		bp->b_state &= ~XFS_BSTATE_IN_FLIGHT;

		percpu_counter_dec(&bp->b_target->bt_io_count);

	}

}



/*

 * When we mark a buffer stale, we remove the buffer from the LRU and clear the

 * b_lru_ref count so that the buffer is freed immediately when the buffer
@@ -156,8 +106,6 @@ xfs_buf_stale( 
	 * status now to preserve accounting consistency.

	 */

	spin_lock(&bp->b_lock);

	__xfs_buf_ioacct_dec(bp);



	atomic_set(&bp->b_lru_ref, 0);

	if (!(bp->b_state & XFS_BSTATE_DISPOSE) &&

	    (list_lru_del_obj(&bp->b_target->bt_lru, &bp->b_lru)))
@@ -946,6 +894,7 @@ xfs_buf_readahead_map( 
	bp->b_ops = ops;

	bp->b_flags &= ~(XBF_WRITE | XBF_DONE);

	bp->b_flags |= flags;

	percpu_counter_inc(&target->bt_readahead_count);

	xfs_buf_submit(bp);

}
@@ -1002,10 +951,12 @@ xfs_buf_get_uncached( 
	struct xfs_buf		*bp;

	DEFINE_SINGLE_BUF_MAP(map, XFS_BUF_DADDR_NULL, numblks);



	/* there are currently no valid flags for xfs_buf_get_uncached */

	ASSERT(flags == 0);



	*bpp = NULL;



	/* flags might contain irrelevant bits, pass only what we care about */

	error = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT, &bp);

	error = _xfs_buf_alloc(target, &map, 1, flags, &bp);

	if (error)

		return error;
@@ -1059,7 +1010,6 @@ xfs_buf_rele_uncached( 
		spin_unlock(&bp->b_lock);

		return;

	}

	__xfs_buf_ioacct_dec(bp);

	spin_unlock(&bp->b_lock);

	xfs_buf_free(bp);

}
@@ -1078,19 +1028,11 @@ xfs_buf_rele_cached( 
	spin_lock(&bp->b_lock);

	ASSERT(bp->b_hold >= 1);

	if (bp->b_hold > 1) {

		/*

		 * Drop the in-flight state if the buffer is already on the LRU

		 * and it holds the only reference. This is racy because we

		 * haven't acquired the pag lock, but the use of _XBF_IN_FLIGHT

		 * ensures the decrement occurs only once per-buf.

		 */

		if (--bp->b_hold == 1 && !list_empty(&bp->b_lru))

			__xfs_buf_ioacct_dec(bp);

		bp->b_hold--;

		goto out_unlock;

	}



	/* we are asked to drop the last reference */

	__xfs_buf_ioacct_dec(bp);

	if (!(bp->b_flags & XBF_STALE) && atomic_read(&bp->b_lru_ref)) {

		/*

		 * If the buffer is added to the LRU, keep the reference to the
@@ -1370,6 +1312,8 @@ __xfs_buf_ioend( 
			bp->b_ops->verify_read(bp);

		if (!bp->b_error)

			bp->b_flags |= XBF_DONE;

		if (bp->b_flags & XBF_READ_AHEAD)

			percpu_counter_dec(&bp->b_target->bt_readahead_count);

	} else {

		if (!bp->b_error) {

			bp->b_flags &= ~XBF_WRITE_FAIL;
@@ -1658,9 +1602,6 @@ xfs_buf_submit( 
	 */

	bp->b_error = 0;



	if (bp->b_flags & XBF_ASYNC)

		xfs_buf_ioacct_inc(bp);



	if ((bp->b_flags & XBF_WRITE) && !xfs_buf_verify_write(bp)) {

		xfs_force_shutdown(bp->b_mount, SHUTDOWN_CORRUPT_INCORE);

		xfs_buf_ioend(bp);
@@ -1786,9 +1727,8 @@ xfs_buftarg_wait( 
	struct xfs_buftarg	*btp)

{

	/*

	 * First wait on the buftarg I/O count for all in-flight buffers to be

	 * released. This is critical as new buffers do not make the LRU until

	 * they are released.

	 * First wait for all in-flight readahead buffers to be released.  This is

	 * critical as new buffers do not make the LRU until they are released.

	 *

	 * Next, flush the buffer workqueue to ensure all completion processing

	 * has finished. Just waiting on buffer locks is not sufficient for
@@ -1797,7 +1737,7 @@ xfs_buftarg_wait( 
	 * all reference counts have been dropped before we start walking the

	 * LRU list.

	 */

	while (percpu_counter_sum(&btp->bt_io_count))

	while (percpu_counter_sum(&btp->bt_readahead_count))

		delay(100);

	flush_workqueue(btp->bt_mount->m_buf_workqueue);

}
@@ -1914,8 +1854,8 @@ xfs_destroy_buftarg( 
	struct xfs_buftarg	*btp)

{

	shrinker_free(btp->bt_shrinker);

	ASSERT(percpu_counter_sum(&btp->bt_io_count) == 0);

	percpu_counter_destroy(&btp->bt_io_count);

	ASSERT(percpu_counter_sum(&btp->bt_readahead_count) == 0);

	percpu_counter_destroy(&btp->bt_readahead_count);

	list_lru_destroy(&btp->bt_lru);

}
@@ -1969,7 +1909,7 @@ xfs_init_buftarg( 


	if (list_lru_init(&btp->bt_lru))

		return -ENOMEM;

	if (percpu_counter_init(&btp->bt_io_count, 0, GFP_KERNEL))

	if (percpu_counter_init(&btp->bt_readahead_count, 0, GFP_KERNEL))

		goto out_destroy_lru;



	btp->bt_shrinker =
@@ -1983,7 +1923,7 @@ xfs_init_buftarg( 
	return 0;



out_destroy_io_count:

	percpu_counter_destroy(&btp->bt_io_count);

	percpu_counter_destroy(&btp->bt_readahead_count);

out_destroy_lru:

	list_lru_destroy(&btp->bt_lru);

	return -ENOMEM;

fs/xfs/xfs_buf.h

+1 −4
Original line number Diff line number Diff line
@@ -27,7 +27,6 @@ struct xfs_buf; 
#define XBF_READ	 (1u << 0) /* buffer intended for reading from device */

#define XBF_WRITE	 (1u << 1) /* buffer intended for writing to device */

#define XBF_READ_AHEAD	 (1u << 2) /* asynchronous read-ahead */

#define XBF_NO_IOACCT	 (1u << 3) /* bypass I/O accounting (non-LRU bufs) */

#define XBF_ASYNC	 (1u << 4) /* initiator will not wait for completion */

#define XBF_DONE	 (1u << 5) /* all pages in the buffer uptodate */

#define XBF_STALE	 (1u << 6) /* buffer has been staled, do not find it */
@@ -58,7 +57,6 @@ typedef unsigned int xfs_buf_flags_t; 
	{ XBF_READ,		"READ" }, \

	{ XBF_WRITE,		"WRITE" }, \

	{ XBF_READ_AHEAD,	"READ_AHEAD" }, \

	{ XBF_NO_IOACCT,	"NO_IOACCT" }, \

	{ XBF_ASYNC,		"ASYNC" }, \

	{ XBF_DONE,		"DONE" }, \

	{ XBF_STALE,		"STALE" }, \
@@ -77,7 +75,6 @@ typedef unsigned int xfs_buf_flags_t; 
 * Internal state flags.

 */

#define XFS_BSTATE_DISPOSE	 (1 << 0)	/* buffer being discarded */

#define XFS_BSTATE_IN_FLIGHT	 (1 << 1)	/* I/O in flight */



struct xfs_buf_cache {

	struct rhashtable	bc_hash;
@@ -116,7 +113,7 @@ struct xfs_buftarg { 
	struct shrinker		*bt_shrinker;

	struct list_lru		bt_lru;



	struct percpu_counter	bt_io_count;

	struct percpu_counter	bt_readahead_count;

	struct ratelimit_state	bt_ioerror_rl;



	/* Atomic write unit values */

fs/xfs/xfs_buf_mem.c

+1 −1
Original line number Diff line number Diff line
@@ -117,7 +117,7 @@ xmbuf_free( 
	struct xfs_buftarg	*btp)

{

	ASSERT(xfs_buftarg_is_mem(btp));

	ASSERT(percpu_counter_sum(&btp->bt_io_count) == 0);

	ASSERT(percpu_counter_sum(&btp->bt_readahead_count) == 0);



	trace_xmbuf_free(btp);

fs/xfs/xfs_mount.c

+2 −5
Original line number Diff line number Diff line
@@ -181,14 +181,11 @@ xfs_readsb( 


	/*

	 * Allocate a (locked) buffer to hold the superblock. This will be kept

	 * around at all times to optimize access to the superblock. Therefore,

	 * set XBF_NO_IOACCT to make sure it doesn't hold the buftarg count

	 * elevated.

	 * around at all times to optimize access to the superblock.

	 */

reread:

	error = xfs_buf_read_uncached(mp->m_ddev_targp, XFS_SB_DADDR,

				      BTOBB(sector_size), XBF_NO_IOACCT, &bp,

				      buf_ops);

				      BTOBB(sector_size), 0, &bp, buf_ops);

	if (error) {

		if (loud)

			xfs_warn(mp, "SB validate failed with error %d.", error);

fs/xfs/xfs_rtalloc.c

+1 −1
Original line number Diff line number Diff line
@@ -1407,7 +1407,7 @@ xfs_rtmount_readsb( 


	/* m_blkbb_log is not set up yet */

	error = xfs_buf_read_uncached(mp->m_rtdev_targp, XFS_RTSB_DADDR,

			mp->m_sb.sb_blocksize >> BBSHIFT, XBF_NO_IOACCT, &bp,

			mp->m_sb.sb_blocksize >> BBSHIFT, 0, &bp,

			&xfs_rtsb_buf_ops);

	if (error) {

		xfs_warn(mp, "rt sb validate failed with error %d.", error);