Commit 90810b9e authored by Dave Chinner's avatar Dave Chinner Committed by Alex Elder

xfs: push stale, pinned buffers on trylock failures

As reported by Nick Piggin, XFS is suffering from long pauses under
highly concurrent workloads when hosted on ramdisks. The problem is
that an inode buffer is stuck in the pinned state in memory and as a
result either the inode buffer or one of the inodes within the
buffer is stopping the tail of the log from being moved forward.

The system remains in this state until a periodic log force issued
by xfssyncd causes the buffer to be unpinned. The main problem is
that these are stale buffers, and are hence held locked until the
transaction/checkpoint that marked them state has been committed to
disk. When the filesystem gets into this state, only the xfssyncd
can cause the async transactions to be committed to disk and hence
unpin the inode buffer.

This problem was encountered when scaling the busy extent list, but
only the blocking lock interface was fixed to solve the problem.
Extend the same fix to the buffer trylock operations - if we fail to
lock a pinned, stale buffer, then force the log immediately so that
when the next attempt to lock it comes around, it will have been
unpinned.
Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
parent c726de44
...@@ -488,29 +488,16 @@ _xfs_buf_find( ...@@ -488,29 +488,16 @@ _xfs_buf_find(
spin_unlock(&pag->pag_buf_lock); spin_unlock(&pag->pag_buf_lock);
xfs_perag_put(pag); xfs_perag_put(pag);
/* Attempt to get the semaphore without sleeping, if (xfs_buf_cond_lock(bp)) {
* if this does not work then we need to drop the /* failed, so wait for the lock if requested. */
* spinlock and do a hard attempt on the semaphore.
*/
if (down_trylock(&bp->b_sema)) {
if (!(flags & XBF_TRYLOCK)) { if (!(flags & XBF_TRYLOCK)) {
/* wait for buffer ownership */
xfs_buf_lock(bp); xfs_buf_lock(bp);
XFS_STATS_INC(xb_get_locked_waited); XFS_STATS_INC(xb_get_locked_waited);
} else { } else {
/* We asked for a trylock and failed, no need
* to look at file offset and length here, we
* know that this buffer at least overlaps our
* buffer and is locked, therefore our buffer
* either does not exist, or is this buffer.
*/
xfs_buf_rele(bp); xfs_buf_rele(bp);
XFS_STATS_INC(xb_busy_locked); XFS_STATS_INC(xb_busy_locked);
return NULL; return NULL;
} }
} else {
/* trylock worked */
XB_SET_OWNER(bp);
} }
if (bp->b_flags & XBF_STALE) { if (bp->b_flags & XBF_STALE) {
...@@ -876,10 +863,18 @@ xfs_buf_rele( ...@@ -876,10 +863,18 @@ xfs_buf_rele(
*/ */
/* /*
* Locks a buffer object, if it is not already locked. * Locks a buffer object, if it is not already locked. Note that this in
* Note that this in no way locks the underlying pages, so it is only * no way locks the underlying pages, so it is only useful for
* useful for synchronizing concurrent use of buffer objects, not for * synchronizing concurrent use of buffer objects, not for synchronizing
* synchronizing independent access to the underlying pages. * independent access to the underlying pages.
*
* If we come across a stale, pinned, locked buffer, we know that we are
* being asked to lock a buffer that has been reallocated. Because it is
* pinned, we know that the log has not been pushed to disk and hence it
* will still be locked. Rather than continuing to have trylock attempts
* fail until someone else pushes the log, push it ourselves before
* returning. This means that the xfsaild will not get stuck trying
* to push on stale inode buffers.
*/ */
int int
xfs_buf_cond_lock( xfs_buf_cond_lock(
...@@ -890,6 +885,8 @@ xfs_buf_cond_lock( ...@@ -890,6 +885,8 @@ xfs_buf_cond_lock(
locked = down_trylock(&bp->b_sema) == 0; locked = down_trylock(&bp->b_sema) == 0;
if (locked) if (locked)
XB_SET_OWNER(bp); XB_SET_OWNER(bp);
else if (atomic_read(&bp->b_pin_count) && (bp->b_flags & XBF_STALE))
xfs_log_force(bp->b_target->bt_mount, 0);
trace_xfs_buf_cond_lock(bp, _RET_IP_); trace_xfs_buf_cond_lock(bp, _RET_IP_);
return locked ? 0 : -EBUSY; return locked ? 0 : -EBUSY;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment