| Message ID | 20200326131703.23246-2-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: a couple AIL pushing trylock fixes | expand |
On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > A dquot flush currently blocks on the buffer lock for the underlying > dquot buffer. In turn, this causes xfsaild to block rather than > continue processing other items in the meantime. Update > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > are handled, and return -EAGAIN if the lock fails. Fix up any > callers that don't currently handle the error properly. Looks good, seems like the two remaining xfs_qm_dqflush have sensible -EAGAIN handling. Reviewed-by: Christoph Hellwig <hch@lst.de>
On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > A dquot flush currently blocks on the buffer lock for the underlying > dquot buffer. In turn, this causes xfsaild to block rather than > continue processing other items in the meantime. Update > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > are handled, and return -EAGAIN if the lock fails. Fix up any > callers that don't currently handle the error properly. > > Signed-off-by: Brian Foster <bfoster@redhat.com> Is xfs_qm_dquot_isolate returning LRU_RETRY an acceptable resolution (as opposed to, say, LRU_SKIP) for xfs_qm_dqflush returning -EAGAIN? --D > --- > fs/xfs/xfs_dquot.c | 6 +++--- > fs/xfs/xfs_dquot_item.c | 3 ++- > fs/xfs/xfs_qm.c | 14 +++++++++----- > 3 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > index 711376ca269f..af2c8e5ceea0 100644 > --- a/fs/xfs/xfs_dquot.c > +++ b/fs/xfs/xfs_dquot.c > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > * Get the buffer containing the on-disk dquot > */ > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > - &xfs_dquot_buf_ops); > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > + &bp, &xfs_dquot_buf_ops); > if (error) > goto out_unlock; > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > out_unlock: > xfs_dqfunlock(dqp); > - return -EIO; > + return error; > } > > /* > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > index cf65e2e43c6e..baad1748d0d1 100644 > --- a/fs/xfs/xfs_dquot_item.c > +++ b/fs/xfs/xfs_dquot_item.c > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > if (!xfs_buf_delwri_queue(bp, buffer_list)) > rval = XFS_ITEM_FLUSHING; > xfs_buf_relse(bp); > - } > + } else if (error == -EAGAIN) > + rval = XFS_ITEM_LOCKED; > > spin_lock(&lip->li_ailp->ail_lock); > out_unlock: > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c > index de1d2c606c14..68c778d25c48 100644 > --- a/fs/xfs/xfs_qm.c > +++ b/fs/xfs/xfs_qm.c > @@ -121,12 +121,11 @@ xfs_qm_dqpurge( > { > struct xfs_mount *mp = dqp->q_mount; > struct xfs_quotainfo *qi = mp->m_quotainfo; > + int error = -EAGAIN; > > xfs_dqlock(dqp); > - if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) { > - xfs_dqunlock(dqp); > - return -EAGAIN; > - } > + if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) > + goto out_unlock; > > dqp->dq_flags |= XFS_DQ_FREEING; > > @@ -139,7 +138,6 @@ xfs_qm_dqpurge( > */ > if (XFS_DQ_IS_DIRTY(dqp)) { > struct xfs_buf *bp = NULL; > - int error; > > /* > * We don't care about getting disk errors here. We need > @@ -149,6 +147,8 @@ xfs_qm_dqpurge( > if (!error) { > error = xfs_bwrite(bp); > xfs_buf_relse(bp); > + } else if (error == -EAGAIN) { > + goto out_unlock; > } > xfs_dqflock(dqp); > } > @@ -174,6 +174,10 @@ xfs_qm_dqpurge( > > xfs_qm_dqdestroy(dqp); > return 0; > + > +out_unlock: > + xfs_dqunlock(dqp); > + return error; > } > > /* > -- > 2.21.1 >
On Fri, Mar 27, 2020 at 08:45:28AM -0700, Darrick J. Wong wrote: > On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > > A dquot flush currently blocks on the buffer lock for the underlying > > dquot buffer. In turn, this causes xfsaild to block rather than > > continue processing other items in the meantime. Update > > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > > are handled, and return -EAGAIN if the lock fails. Fix up any > > callers that don't currently handle the error properly. > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > Is xfs_qm_dquot_isolate returning LRU_RETRY an acceptable resolution (as > opposed to, say, LRU_SKIP) for xfs_qm_dqflush returning -EAGAIN? > Hmm.. this is reclaim so I suppose LRU_SKIP would be more appropriate than retry (along with more consistent with the other trylock failures in that function). Ok with something like the following? @@ -461,7 +461,11 @@ xfs_qm_dquot_isolate( spin_unlock(lru_lock); error = xfs_qm_dqflush(dqp, &bp); - if (error) + if (error == -EAGAIN) { + xfs_dqunlock(dqp); + spin_lock(lru_lock); + goto out_miss_busy; + } else if (error) goto out_unlock_dirty; xfs_buf_delwri_queue(bp, &isol->buffers); Brian > --D > > > --- > > fs/xfs/xfs_dquot.c | 6 +++--- > > fs/xfs/xfs_dquot_item.c | 3 ++- > > fs/xfs/xfs_qm.c | 14 +++++++++----- > > 3 files changed, 14 insertions(+), 9 deletions(-) > > > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > > index 711376ca269f..af2c8e5ceea0 100644 > > --- a/fs/xfs/xfs_dquot.c > > +++ b/fs/xfs/xfs_dquot.c > > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > > * Get the buffer containing the on-disk dquot > > */ > > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > > - &xfs_dquot_buf_ops); > > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > > + &bp, &xfs_dquot_buf_ops); > > if (error) > > goto out_unlock; > > > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > > > out_unlock: > > xfs_dqfunlock(dqp); > > - return -EIO; > > + return error; > > } > > > > /* > > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > > index cf65e2e43c6e..baad1748d0d1 100644 > > --- a/fs/xfs/xfs_dquot_item.c > > +++ b/fs/xfs/xfs_dquot_item.c > > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > > if (!xfs_buf_delwri_queue(bp, buffer_list)) > > rval = XFS_ITEM_FLUSHING; > > xfs_buf_relse(bp); > > - } > > + } else if (error == -EAGAIN) > > + rval = XFS_ITEM_LOCKED; > > > > spin_lock(&lip->li_ailp->ail_lock); > > out_unlock: > > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c > > index de1d2c606c14..68c778d25c48 100644 > > --- a/fs/xfs/xfs_qm.c > > +++ b/fs/xfs/xfs_qm.c > > @@ -121,12 +121,11 @@ xfs_qm_dqpurge( > > { > > struct xfs_mount *mp = dqp->q_mount; > > struct xfs_quotainfo *qi = mp->m_quotainfo; > > + int error = -EAGAIN; > > > > xfs_dqlock(dqp); > > - if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) { > > - xfs_dqunlock(dqp); > > - return -EAGAIN; > > - } > > + if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) > > + goto out_unlock; > > > > dqp->dq_flags |= XFS_DQ_FREEING; > > > > @@ -139,7 +138,6 @@ xfs_qm_dqpurge( > > */ > > if (XFS_DQ_IS_DIRTY(dqp)) { > > struct xfs_buf *bp = NULL; > > - int error; > > > > /* > > * We don't care about getting disk errors here. We need > > @@ -149,6 +147,8 @@ xfs_qm_dqpurge( > > if (!error) { > > error = xfs_bwrite(bp); > > xfs_buf_relse(bp); > > + } else if (error == -EAGAIN) { > > + goto out_unlock; > > } > > xfs_dqflock(dqp); > > } > > @@ -174,6 +174,10 @@ xfs_qm_dqpurge( > > > > xfs_qm_dqdestroy(dqp); > > return 0; > > + > > +out_unlock: > > + xfs_dqunlock(dqp); > > + return error; > > } > > > > /* > > -- > > 2.21.1 > > >
On Fri, Mar 27, 2020 at 12:44:40PM -0400, Brian Foster wrote: > On Fri, Mar 27, 2020 at 08:45:28AM -0700, Darrick J. Wong wrote: > > On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > > > A dquot flush currently blocks on the buffer lock for the underlying > > > dquot buffer. In turn, this causes xfsaild to block rather than > > > continue processing other items in the meantime. Update > > > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > > > are handled, and return -EAGAIN if the lock fails. Fix up any > > > callers that don't currently handle the error properly. > > > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > > > Is xfs_qm_dquot_isolate returning LRU_RETRY an acceptable resolution (as > > opposed to, say, LRU_SKIP) for xfs_qm_dqflush returning -EAGAIN? > > > > Hmm.. this is reclaim so I suppose LRU_SKIP would be more appropriate > than retry (along with more consistent with the other trylock failures > in that function). Ok with something like the following? > > @@ -461,7 +461,11 @@ xfs_qm_dquot_isolate( > spin_unlock(lru_lock); > > error = xfs_qm_dqflush(dqp, &bp); > - if (error) > + if (error == -EAGAIN) { > + xfs_dqunlock(dqp); > + spin_lock(lru_lock); > + goto out_miss_busy; > + } else if (error) > goto out_unlock_dirty; Then again, is it safe to skip from here once we've cycled the lru_lock? Brian > > xfs_buf_delwri_queue(bp, &isol->buffers); > > Brian > > > --D > > > > > --- > > > fs/xfs/xfs_dquot.c | 6 +++--- > > > fs/xfs/xfs_dquot_item.c | 3 ++- > > > fs/xfs/xfs_qm.c | 14 +++++++++----- > > > 3 files changed, 14 insertions(+), 9 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > > > index 711376ca269f..af2c8e5ceea0 100644 > > > --- a/fs/xfs/xfs_dquot.c > > > +++ b/fs/xfs/xfs_dquot.c > > > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > > > * Get the buffer containing the on-disk dquot > > > */ > > > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > > > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > > > - &xfs_dquot_buf_ops); > > > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > > > + &bp, &xfs_dquot_buf_ops); > > > if (error) > > > goto out_unlock; > > > > > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > > > > > out_unlock: > > > xfs_dqfunlock(dqp); > > > - return -EIO; > > > + return error; > > > } > > > > > > /* > > > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > > > index cf65e2e43c6e..baad1748d0d1 100644 > > > --- a/fs/xfs/xfs_dquot_item.c > > > +++ b/fs/xfs/xfs_dquot_item.c > > > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > > > if (!xfs_buf_delwri_queue(bp, buffer_list)) > > > rval = XFS_ITEM_FLUSHING; > > > xfs_buf_relse(bp); > > > - } > > > + } else if (error == -EAGAIN) > > > + rval = XFS_ITEM_LOCKED; > > > > > > spin_lock(&lip->li_ailp->ail_lock); > > > out_unlock: > > > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c > > > index de1d2c606c14..68c778d25c48 100644 > > > --- a/fs/xfs/xfs_qm.c > > > +++ b/fs/xfs/xfs_qm.c > > > @@ -121,12 +121,11 @@ xfs_qm_dqpurge( > > > { > > > struct xfs_mount *mp = dqp->q_mount; > > > struct xfs_quotainfo *qi = mp->m_quotainfo; > > > + int error = -EAGAIN; > > > > > > xfs_dqlock(dqp); > > > - if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) { > > > - xfs_dqunlock(dqp); > > > - return -EAGAIN; > > > - } > > > + if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) > > > + goto out_unlock; > > > > > > dqp->dq_flags |= XFS_DQ_FREEING; > > > > > > @@ -139,7 +138,6 @@ xfs_qm_dqpurge( > > > */ > > > if (XFS_DQ_IS_DIRTY(dqp)) { > > > struct xfs_buf *bp = NULL; > > > - int error; > > > > > > /* > > > * We don't care about getting disk errors here. We need > > > @@ -149,6 +147,8 @@ xfs_qm_dqpurge( > > > if (!error) { > > > error = xfs_bwrite(bp); > > > xfs_buf_relse(bp); > > > + } else if (error == -EAGAIN) { > > > + goto out_unlock; > > > } > > > xfs_dqflock(dqp); > > > } > > > @@ -174,6 +174,10 @@ xfs_qm_dqpurge( > > > > > > xfs_qm_dqdestroy(dqp); > > > return 0; > > > + > > > +out_unlock: > > > + xfs_dqunlock(dqp); > > > + return error; > > > } > > > > > > /* > > > -- > > > 2.21.1 > > > > > >
On Fri, Mar 27, 2020 at 12:44:40PM -0400, Brian Foster wrote: > On Fri, Mar 27, 2020 at 08:45:28AM -0700, Darrick J. Wong wrote: > > On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > > > A dquot flush currently blocks on the buffer lock for the underlying > > > dquot buffer. In turn, this causes xfsaild to block rather than > > > continue processing other items in the meantime. Update > > > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > > > are handled, and return -EAGAIN if the lock fails. Fix up any > > > callers that don't currently handle the error properly. > > > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > > > Is xfs_qm_dquot_isolate returning LRU_RETRY an acceptable resolution (as > > opposed to, say, LRU_SKIP) for xfs_qm_dqflush returning -EAGAIN? > > > > Hmm.. this is reclaim so I suppose LRU_SKIP would be more appropriate > than retry (along with more consistent with the other trylock failures > in that function). Ok with something like the following? > > @@ -461,7 +461,11 @@ xfs_qm_dquot_isolate( > spin_unlock(lru_lock); > > error = xfs_qm_dqflush(dqp, &bp); > - if (error) > + if (error == -EAGAIN) { > + xfs_dqunlock(dqp); > + spin_lock(lru_lock); > + goto out_miss_busy; > + } else if (error) > goto out_unlock_dirty; > > xfs_buf_delwri_queue(bp, &isol->buffers); Yeah, looks good to me. --D > > Brian > > > --D > > > > > --- > > > fs/xfs/xfs_dquot.c | 6 +++--- > > > fs/xfs/xfs_dquot_item.c | 3 ++- > > > fs/xfs/xfs_qm.c | 14 +++++++++----- > > > 3 files changed, 14 insertions(+), 9 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > > > index 711376ca269f..af2c8e5ceea0 100644 > > > --- a/fs/xfs/xfs_dquot.c > > > +++ b/fs/xfs/xfs_dquot.c > > > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > > > * Get the buffer containing the on-disk dquot > > > */ > > > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > > > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > > > - &xfs_dquot_buf_ops); > > > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > > > + &bp, &xfs_dquot_buf_ops); > > > if (error) > > > goto out_unlock; > > > > > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > > > > > out_unlock: > > > xfs_dqfunlock(dqp); > > > - return -EIO; > > > + return error; > > > } > > > > > > /* > > > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > > > index cf65e2e43c6e..baad1748d0d1 100644 > > > --- a/fs/xfs/xfs_dquot_item.c > > > +++ b/fs/xfs/xfs_dquot_item.c > > > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > > > if (!xfs_buf_delwri_queue(bp, buffer_list)) > > > rval = XFS_ITEM_FLUSHING; > > > xfs_buf_relse(bp); > > > - } > > > + } else if (error == -EAGAIN) > > > + rval = XFS_ITEM_LOCKED; > > > > > > spin_lock(&lip->li_ailp->ail_lock); > > > out_unlock: > > > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c > > > index de1d2c606c14..68c778d25c48 100644 > > > --- a/fs/xfs/xfs_qm.c > > > +++ b/fs/xfs/xfs_qm.c > > > @@ -121,12 +121,11 @@ xfs_qm_dqpurge( > > > { > > > struct xfs_mount *mp = dqp->q_mount; > > > struct xfs_quotainfo *qi = mp->m_quotainfo; > > > + int error = -EAGAIN; > > > > > > xfs_dqlock(dqp); > > > - if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) { > > > - xfs_dqunlock(dqp); > > > - return -EAGAIN; > > > - } > > > + if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) > > > + goto out_unlock; > > > > > > dqp->dq_flags |= XFS_DQ_FREEING; > > > > > > @@ -139,7 +138,6 @@ xfs_qm_dqpurge( > > > */ > > > if (XFS_DQ_IS_DIRTY(dqp)) { > > > struct xfs_buf *bp = NULL; > > > - int error; > > > > > > /* > > > * We don't care about getting disk errors here. We need > > > @@ -149,6 +147,8 @@ xfs_qm_dqpurge( > > > if (!error) { > > > error = xfs_bwrite(bp); > > > xfs_buf_relse(bp); > > > + } else if (error == -EAGAIN) { > > > + goto out_unlock; > > > } > > > xfs_dqflock(dqp); > > > } > > > @@ -174,6 +174,10 @@ xfs_qm_dqpurge( > > > > > > xfs_qm_dqdestroy(dqp); > > > return 0; > > > + > > > +out_unlock: > > > + xfs_dqunlock(dqp); > > > + return error; > > > } > > > > > > /* > > > -- > > > 2.21.1 > > > > > >
On Fri, Mar 27, 2020 at 12:46:44PM -0400, Brian Foster wrote: > On Fri, Mar 27, 2020 at 12:44:40PM -0400, Brian Foster wrote: > > On Fri, Mar 27, 2020 at 08:45:28AM -0700, Darrick J. Wong wrote: > > > On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > > > > A dquot flush currently blocks on the buffer lock for the underlying > > > > dquot buffer. In turn, this causes xfsaild to block rather than > > > > continue processing other items in the meantime. Update > > > > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > > > > are handled, and return -EAGAIN if the lock fails. Fix up any > > > > callers that don't currently handle the error properly. > > > > > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > > > > > Is xfs_qm_dquot_isolate returning LRU_RETRY an acceptable resolution (as > > > opposed to, say, LRU_SKIP) for xfs_qm_dqflush returning -EAGAIN? > > > > > > > Hmm.. this is reclaim so I suppose LRU_SKIP would be more appropriate > > than retry (along with more consistent with the other trylock failures > > in that function). Ok with something like the following? > > > > @@ -461,7 +461,11 @@ xfs_qm_dquot_isolate( > > spin_unlock(lru_lock); > > > > error = xfs_qm_dqflush(dqp, &bp); > > - if (error) > > + if (error == -EAGAIN) { > > + xfs_dqunlock(dqp); > > + spin_lock(lru_lock); > > + goto out_miss_busy; > > + } else if (error) > > goto out_unlock_dirty; > > Then again, is it safe to skip from here once we've cycled the lru_lock? DOH. Yeah, I missed that we cycled the lru lock and therefore have to LRU_RETRY. So I guess the original patch was fine: Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > Brian > > > > > xfs_buf_delwri_queue(bp, &isol->buffers); > > > > Brian > > > > > --D > > > > > > > --- > > > > fs/xfs/xfs_dquot.c | 6 +++--- > > > > fs/xfs/xfs_dquot_item.c | 3 ++- > > > > fs/xfs/xfs_qm.c | 14 +++++++++----- > > > > 3 files changed, 14 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > > > > index 711376ca269f..af2c8e5ceea0 100644 > > > > --- a/fs/xfs/xfs_dquot.c > > > > +++ b/fs/xfs/xfs_dquot.c > > > > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > > > > * Get the buffer containing the on-disk dquot > > > > */ > > > > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > > > > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > > > > - &xfs_dquot_buf_ops); > > > > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > > > > + &bp, &xfs_dquot_buf_ops); > > > > if (error) > > > > goto out_unlock; > > > > > > > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > > > > > > > out_unlock: > > > > xfs_dqfunlock(dqp); > > > > - return -EIO; > > > > + return error; > > > > } > > > > > > > > /* > > > > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > > > > index cf65e2e43c6e..baad1748d0d1 100644 > > > > --- a/fs/xfs/xfs_dquot_item.c > > > > +++ b/fs/xfs/xfs_dquot_item.c > > > > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > > > > if (!xfs_buf_delwri_queue(bp, buffer_list)) > > > > rval = XFS_ITEM_FLUSHING; > > > > xfs_buf_relse(bp); > > > > - } > > > > + } else if (error == -EAGAIN) > > > > + rval = XFS_ITEM_LOCKED; > > > > > > > > spin_lock(&lip->li_ailp->ail_lock); > > > > out_unlock: > > > > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c > > > > index de1d2c606c14..68c778d25c48 100644 > > > > --- a/fs/xfs/xfs_qm.c > > > > +++ b/fs/xfs/xfs_qm.c > > > > @@ -121,12 +121,11 @@ xfs_qm_dqpurge( > > > > { > > > > struct xfs_mount *mp = dqp->q_mount; > > > > struct xfs_quotainfo *qi = mp->m_quotainfo; > > > > + int error = -EAGAIN; > > > > > > > > xfs_dqlock(dqp); > > > > - if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) { > > > > - xfs_dqunlock(dqp); > > > > - return -EAGAIN; > > > > - } > > > > + if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) > > > > + goto out_unlock; > > > > > > > > dqp->dq_flags |= XFS_DQ_FREEING; > > > > > > > > @@ -139,7 +138,6 @@ xfs_qm_dqpurge( > > > > */ > > > > if (XFS_DQ_IS_DIRTY(dqp)) { > > > > struct xfs_buf *bp = NULL; > > > > - int error; > > > > > > > > /* > > > > * We don't care about getting disk errors here. We need > > > > @@ -149,6 +147,8 @@ xfs_qm_dqpurge( > > > > if (!error) { > > > > error = xfs_bwrite(bp); > > > > xfs_buf_relse(bp); > > > > + } else if (error == -EAGAIN) { > > > > + goto out_unlock; > > > > } > > > > xfs_dqflock(dqp); > > > > } > > > > @@ -174,6 +174,10 @@ xfs_qm_dqpurge( > > > > > > > > xfs_qm_dqdestroy(dqp); > > > > return 0; > > > > + > > > > +out_unlock: > > > > + xfs_dqunlock(dqp); > > > > + return error; > > > > } > > > > > > > > /* > > > > -- > > > > 2.21.1 > > > > > > > > > >
On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > A dquot flush currently blocks on the buffer lock for the underlying > dquot buffer. In turn, this causes xfsaild to block rather than > continue processing other items in the meantime. Update > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > are handled, and return -EAGAIN if the lock fails. Fix up any > callers that don't currently handle the error properly. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > --- > fs/xfs/xfs_dquot.c | 6 +++--- > fs/xfs/xfs_dquot_item.c | 3 ++- > fs/xfs/xfs_qm.c | 14 +++++++++----- > 3 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > index 711376ca269f..af2c8e5ceea0 100644 > --- a/fs/xfs/xfs_dquot.c > +++ b/fs/xfs/xfs_dquot.c > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > * Get the buffer containing the on-disk dquot > */ > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > - &xfs_dquot_buf_ops); > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > + &bp, &xfs_dquot_buf_ops); > if (error) > goto out_unlock; > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > out_unlock: > xfs_dqfunlock(dqp); > - return -EIO; > + return error; > } > > /* > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > index cf65e2e43c6e..baad1748d0d1 100644 > --- a/fs/xfs/xfs_dquot_item.c > +++ b/fs/xfs/xfs_dquot_item.c > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > if (!xfs_buf_delwri_queue(bp, buffer_list)) > rval = XFS_ITEM_FLUSHING; > xfs_buf_relse(bp); > - } > + } else if (error == -EAGAIN) > + rval = XFS_ITEM_LOCKED; Doesn't xfs_inode_item_push() also have this problem in that it doesn't handle -EAGAIN properly? Also, we can get -EIO, -EFSCORRUPTED, etc here. They probably shouldn't return XFS_ITEM_SUCCESS, either.... Otherwise seems OK. Cheers, Dave.
On Mon, Mar 30, 2020 at 09:46:02AM +1100, Dave Chinner wrote: > On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > > A dquot flush currently blocks on the buffer lock for the underlying > > dquot buffer. In turn, this causes xfsaild to block rather than > > continue processing other items in the meantime. Update > > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > > are handled, and return -EAGAIN if the lock fails. Fix up any > > callers that don't currently handle the error properly. > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > --- > > fs/xfs/xfs_dquot.c | 6 +++--- > > fs/xfs/xfs_dquot_item.c | 3 ++- > > fs/xfs/xfs_qm.c | 14 +++++++++----- > > 3 files changed, 14 insertions(+), 9 deletions(-) > > > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > > index 711376ca269f..af2c8e5ceea0 100644 > > --- a/fs/xfs/xfs_dquot.c > > +++ b/fs/xfs/xfs_dquot.c > > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > > * Get the buffer containing the on-disk dquot > > */ > > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > > - &xfs_dquot_buf_ops); > > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > > + &bp, &xfs_dquot_buf_ops); > > if (error) > > goto out_unlock; > > > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > > > out_unlock: > > xfs_dqfunlock(dqp); > > - return -EIO; > > + return error; > > } > > > > /* > > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > > index cf65e2e43c6e..baad1748d0d1 100644 > > --- a/fs/xfs/xfs_dquot_item.c > > +++ b/fs/xfs/xfs_dquot_item.c > > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > > if (!xfs_buf_delwri_queue(bp, buffer_list)) > > rval = XFS_ITEM_FLUSHING; > > xfs_buf_relse(bp); > > - } > > + } else if (error == -EAGAIN) > > + rval = XFS_ITEM_LOCKED; > > Doesn't xfs_inode_item_push() also have this problem in that it > doesn't handle -EAGAIN properly? ... and now I see this is the second patch in the series... Cheers, Dave.
On Mon, Mar 30, 2020 at 09:46:02AM +1100, Dave Chinner wrote: > On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > > A dquot flush currently blocks on the buffer lock for the underlying > > dquot buffer. In turn, this causes xfsaild to block rather than > > continue processing other items in the meantime. Update > > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > > are handled, and return -EAGAIN if the lock fails. Fix up any > > callers that don't currently handle the error properly. > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > --- > > fs/xfs/xfs_dquot.c | 6 +++--- > > fs/xfs/xfs_dquot_item.c | 3 ++- > > fs/xfs/xfs_qm.c | 14 +++++++++----- > > 3 files changed, 14 insertions(+), 9 deletions(-) > > > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > > index 711376ca269f..af2c8e5ceea0 100644 > > --- a/fs/xfs/xfs_dquot.c > > +++ b/fs/xfs/xfs_dquot.c > > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > > * Get the buffer containing the on-disk dquot > > */ > > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > > - &xfs_dquot_buf_ops); > > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > > + &bp, &xfs_dquot_buf_ops); > > if (error) > > goto out_unlock; > > > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > > > out_unlock: > > xfs_dqfunlock(dqp); > > - return -EIO; > > + return error; > > } > > > > /* > > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > > index cf65e2e43c6e..baad1748d0d1 100644 > > --- a/fs/xfs/xfs_dquot_item.c > > +++ b/fs/xfs/xfs_dquot_item.c > > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > > if (!xfs_buf_delwri_queue(bp, buffer_list)) > > rval = XFS_ITEM_FLUSHING; > > xfs_buf_relse(bp); > > - } > > + } else if (error == -EAGAIN) > > + rval = XFS_ITEM_LOCKED; > > Doesn't xfs_inode_item_push() also have this problem in that it > doesn't handle -EAGAIN properly? > > Also, we can get -EIO, -EFSCORRUPTED, etc here. They probably > shouldn't return XFS_ITEM_SUCCESS, either.... > Good point. I'm actually not sure what we should return in that case given the item return codes all seem to assume a valid state. We could define an XFS_ITEM_ERROR return, but I'm not sure it's worth it for what is currently stat/tracepoint logic in the caller. Perhaps a broader rework of error handling in this context is in order that would lift generic (fatal) error handling into xfsaild. E.g., I see that xfs_qm_dqflush() is inconsistent by itself in that the item is removed from the AIL if we're already shut down, but not if that function invokes the shutdown; we shutdown if the direct xfs_dqblk_verify() call fails but not if the read verifier (which also looks like it calls xfs_dqblk_verify() on every on-disk dquot) returns -EFSCORRUPTED, etc. It might make some sense to let iop_push() return negative error codes if that facilitates consistent error handling... Brian > Otherwise seems OK. > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com >
On Mon, Mar 30, 2020 at 08:15:44AM -0400, Brian Foster wrote: > On Mon, Mar 30, 2020 at 09:46:02AM +1100, Dave Chinner wrote: > > On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > > > A dquot flush currently blocks on the buffer lock for the underlying > > > dquot buffer. In turn, this causes xfsaild to block rather than > > > continue processing other items in the meantime. Update > > > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > > > are handled, and return -EAGAIN if the lock fails. Fix up any > > > callers that don't currently handle the error properly. > > > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > > --- > > > fs/xfs/xfs_dquot.c | 6 +++--- > > > fs/xfs/xfs_dquot_item.c | 3 ++- > > > fs/xfs/xfs_qm.c | 14 +++++++++----- > > > 3 files changed, 14 insertions(+), 9 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > > > index 711376ca269f..af2c8e5ceea0 100644 > > > --- a/fs/xfs/xfs_dquot.c > > > +++ b/fs/xfs/xfs_dquot.c > > > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > > > * Get the buffer containing the on-disk dquot > > > */ > > > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > > > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > > > - &xfs_dquot_buf_ops); > > > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > > > + &bp, &xfs_dquot_buf_ops); > > > if (error) > > > goto out_unlock; > > > > > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > > > > > out_unlock: > > > xfs_dqfunlock(dqp); > > > - return -EIO; > > > + return error; > > > } > > > > > > /* > > > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > > > index cf65e2e43c6e..baad1748d0d1 100644 > > > --- a/fs/xfs/xfs_dquot_item.c > > > +++ b/fs/xfs/xfs_dquot_item.c > > > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > > > if (!xfs_buf_delwri_queue(bp, buffer_list)) > > > rval = XFS_ITEM_FLUSHING; > > > xfs_buf_relse(bp); > > > - } > > > + } else if (error == -EAGAIN) > > > + rval = XFS_ITEM_LOCKED; > > > > Doesn't xfs_inode_item_push() also have this problem in that it > > doesn't handle -EAGAIN properly? > > > > Also, we can get -EIO, -EFSCORRUPTED, etc here. They probably > > shouldn't return XFS_ITEM_SUCCESS, either.... > > > > Good point. I'm actually not sure what we should return in that case > given the item return codes all seem to assume a valid state. We could > define an XFS_ITEM_ERROR return, but I'm not sure it's worth it for what > is currently stat/tracepoint logic in the caller. Perhaps a broader > rework of error handling in this context is in order that would lift > generic (fatal) error handling into xfsaild. Yeah, that's where my thoughts were heading as well. > E.g., I see that > xfs_qm_dqflush() is inconsistent by itself in that the item is removed > from the AIL if we're already shut down, but not if that function > invokes the shutdown; we shutdown if the direct xfs_dqblk_verify() call > fails but not if the read verifier (which also looks like it calls > xfs_dqblk_verify() on every on-disk dquot) returns -EFSCORRUPTED, etc. > It might make some sense to let iop_push() return negative error codes > if that facilitates consistent error handling... Yes, it's a bit of a mess. I suspect that what we should be doing here is pulling the failed buffer write retry code up into the main push loop. That is, we can set LI_FAILED on log items that fail to flush, either directly at submit time, or at IO completion for write errors. Then we can have the main AIL loop set LI_FAILED on push failures, and also the main loop detect LI_FAILED directly and call a new ->iop_resubmit() function rather than having to handle that the resubmit cases as special cases in every ->iop_push() path. That seems like a much cleaner way of handling submission failure and retries for all log item types that need it compared to the way we currently handle it for buffers... Cheers, Dave.
On Tue, Mar 31, 2020 at 11:04:09AM +1100, Dave Chinner wrote: > On Mon, Mar 30, 2020 at 08:15:44AM -0400, Brian Foster wrote: > > On Mon, Mar 30, 2020 at 09:46:02AM +1100, Dave Chinner wrote: > > > On Thu, Mar 26, 2020 at 09:17:02AM -0400, Brian Foster wrote: > > > > A dquot flush currently blocks on the buffer lock for the underlying > > > > dquot buffer. In turn, this causes xfsaild to block rather than > > > > continue processing other items in the meantime. Update > > > > xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers > > > > are handled, and return -EAGAIN if the lock fails. Fix up any > > > > callers that don't currently handle the error properly. > > > > > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > > > --- > > > > fs/xfs/xfs_dquot.c | 6 +++--- > > > > fs/xfs/xfs_dquot_item.c | 3 ++- > > > > fs/xfs/xfs_qm.c | 14 +++++++++----- > > > > 3 files changed, 14 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c > > > > index 711376ca269f..af2c8e5ceea0 100644 > > > > --- a/fs/xfs/xfs_dquot.c > > > > +++ b/fs/xfs/xfs_dquot.c > > > > @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( > > > > * Get the buffer containing the on-disk dquot > > > > */ > > > > error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, > > > > - mp->m_quotainfo->qi_dqchunklen, 0, &bp, > > > > - &xfs_dquot_buf_ops); > > > > + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, > > > > + &bp, &xfs_dquot_buf_ops); > > > > if (error) > > > > goto out_unlock; > > > > > > > > @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( > > > > > > > > out_unlock: > > > > xfs_dqfunlock(dqp); > > > > - return -EIO; > > > > + return error; > > > > } > > > > > > > > /* > > > > diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c > > > > index cf65e2e43c6e..baad1748d0d1 100644 > > > > --- a/fs/xfs/xfs_dquot_item.c > > > > +++ b/fs/xfs/xfs_dquot_item.c > > > > @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( > > > > if (!xfs_buf_delwri_queue(bp, buffer_list)) > > > > rval = XFS_ITEM_FLUSHING; > > > > xfs_buf_relse(bp); > > > > - } > > > > + } else if (error == -EAGAIN) > > > > + rval = XFS_ITEM_LOCKED; > > > > > > Doesn't xfs_inode_item_push() also have this problem in that it > > > doesn't handle -EAGAIN properly? > > > > > > Also, we can get -EIO, -EFSCORRUPTED, etc here. They probably > > > shouldn't return XFS_ITEM_SUCCESS, either.... > > > > > > > Good point. I'm actually not sure what we should return in that case > > given the item return codes all seem to assume a valid state. We could > > define an XFS_ITEM_ERROR return, but I'm not sure it's worth it for what > > is currently stat/tracepoint logic in the caller. Perhaps a broader > > rework of error handling in this context is in order that would lift > > generic (fatal) error handling into xfsaild. > > Yeah, that's where my thoughts were heading as well. > > > E.g., I see that > > xfs_qm_dqflush() is inconsistent by itself in that the item is removed > > from the AIL if we're already shut down, but not if that function > > invokes the shutdown; we shutdown if the direct xfs_dqblk_verify() call > > fails but not if the read verifier (which also looks like it calls > > xfs_dqblk_verify() on every on-disk dquot) returns -EFSCORRUPTED, etc. > > It might make some sense to let iop_push() return negative error codes > > if that facilitates consistent error handling... > > Yes, it's a bit of a mess. I suspect that what we should be doing > here is pulling the failed buffer write retry code up into the main > push loop. That is, we can set LI_FAILED on log items that fail to > flush, either directly at submit time, or at IO completion for write > errors. > > Then we can have the main AIL loop set LI_FAILED on push failures, > and also the main loop detect LI_FAILED directly and call a new > ->iop_resubmit() function rather than having to handle that the > resubmit cases as special cases in every ->iop_push() path. > I'm not sure we want to use LI_FAILED in failure to flush (i.e. push failure) situations because it's currently used specifically to indicate that a particular item requires resubmit when it already has been successfully flushed. This avoids the need for a post I/O error push to retry an already locked flush lock (and flush attempt) and subsequently cause the item to remain stuck on the AIL. It still might make sense to refactor the existing LI_FAILED implementation into ->iop_resubmit() callbacks for those items that use it, though. That also doesn't preclude refactoring some sort of generic push failure error handling into xfsaild for the sake of consistency. It's just not immediately clear to me what it should look like. Perhaps I'll poke at it a bit once I get the next rfc of the relog work settled and posted (soon).. Brian > That seems like a much cleaner way of handling submission failure > and retries for all log item types that need it compared to the way > we currently handle it for buffers... > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com >
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c index 711376ca269f..af2c8e5ceea0 100644 --- a/fs/xfs/xfs_dquot.c +++ b/fs/xfs/xfs_dquot.c @@ -1105,8 +1105,8 @@ xfs_qm_dqflush( * Get the buffer containing the on-disk dquot */ error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp, dqp->q_blkno, - mp->m_quotainfo->qi_dqchunklen, 0, &bp, - &xfs_dquot_buf_ops); + mp->m_quotainfo->qi_dqchunklen, XBF_TRYLOCK, + &bp, &xfs_dquot_buf_ops); if (error) goto out_unlock; @@ -1177,7 +1177,7 @@ xfs_qm_dqflush( out_unlock: xfs_dqfunlock(dqp); - return -EIO; + return error; } /* diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c index cf65e2e43c6e..baad1748d0d1 100644 --- a/fs/xfs/xfs_dquot_item.c +++ b/fs/xfs/xfs_dquot_item.c @@ -189,7 +189,8 @@ xfs_qm_dquot_logitem_push( if (!xfs_buf_delwri_queue(bp, buffer_list)) rval = XFS_ITEM_FLUSHING; xfs_buf_relse(bp); - } + } else if (error == -EAGAIN) + rval = XFS_ITEM_LOCKED; spin_lock(&lip->li_ailp->ail_lock); out_unlock: diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c index de1d2c606c14..68c778d25c48 100644 --- a/fs/xfs/xfs_qm.c +++ b/fs/xfs/xfs_qm.c @@ -121,12 +121,11 @@ xfs_qm_dqpurge( { struct xfs_mount *mp = dqp->q_mount; struct xfs_quotainfo *qi = mp->m_quotainfo; + int error = -EAGAIN; xfs_dqlock(dqp); - if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) { - xfs_dqunlock(dqp); - return -EAGAIN; - } + if ((dqp->dq_flags & XFS_DQ_FREEING) || dqp->q_nrefs != 0) + goto out_unlock; dqp->dq_flags |= XFS_DQ_FREEING; @@ -139,7 +138,6 @@ xfs_qm_dqpurge( */ if (XFS_DQ_IS_DIRTY(dqp)) { struct xfs_buf *bp = NULL; - int error; /* * We don't care about getting disk errors here. We need @@ -149,6 +147,8 @@ xfs_qm_dqpurge( if (!error) { error = xfs_bwrite(bp); xfs_buf_relse(bp); + } else if (error == -EAGAIN) { + goto out_unlock; } xfs_dqflock(dqp); } @@ -174,6 +174,10 @@ xfs_qm_dqpurge( xfs_qm_dqdestroy(dqp); return 0; + +out_unlock: + xfs_dqunlock(dqp); + return error; } /*
A dquot flush currently blocks on the buffer lock for the underlying dquot buffer. In turn, this causes xfsaild to block rather than continue processing other items in the meantime. Update xfs_qm_dqflush() to trylock the buffer, similar to how inode buffers are handled, and return -EAGAIN if the lock fails. Fix up any callers that don't currently handle the error properly. Signed-off-by: Brian Foster <bfoster@redhat.com> --- fs/xfs/xfs_dquot.c | 6 +++--- fs/xfs/xfs_dquot_item.c | 3 ++- fs/xfs/xfs_qm.c | 14 +++++++++----- 3 files changed, 14 insertions(+), 9 deletions(-)