| Message ID | 20200417150859.14734-9-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | xfs: flush related error handling cleanups | expand |
On Fri, Apr 17, 2020 at 11:08:55AM -0400, Brian Foster wrote: > The quotaoff intent item push handler unconditionally returns locked > status because it remains AIL resident until removed by the > quotafoff end intent. xfsaild_push_item() already returns pinned > status for items (generally intents) without a push handler. This is > effectively the same behavior for the purpose of quotaoff, so remove It's not the same. XFS_ITEM_PINNED results in a log force from the xfsaild, while XFS_ITEM_LOCKED items are just skipped. So this change will result in a log force every time the AIL push sees a quotaoff log item. Hence I think the code as it stands is correct as log forces will not speed up the removal of the quotaoff item from the AIL... Cheers, Dave.
On Mon, Apr 20, 2020 at 01:58:58PM +1000, Dave Chinner wrote: > On Fri, Apr 17, 2020 at 11:08:55AM -0400, Brian Foster wrote: > > The quotaoff intent item push handler unconditionally returns locked > > status because it remains AIL resident until removed by the > > quotafoff end intent. xfsaild_push_item() already returns pinned > > status for items (generally intents) without a push handler. This is > > effectively the same behavior for the purpose of quotaoff, so remove > > It's not the same. XFS_ITEM_PINNED results in a log force from the > xfsaild, while XFS_ITEM_LOCKED items are just skipped. So this > change will result in a log force every time the AIL push sees a > quotaoff log item. Hence I think the code as it stands is correct > as log forces will not speed up the removal of the quotaoff item > from the AIL... > Ah right.. I was thinking we had a count heuristic there but we potentially force the log once we hit at least one pinned item. I think this same thing might have come up when this was first refactored. I'll drop this one.. Brian > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com >
diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c index 5a7808299a32..582b3796a0c9 100644 --- a/fs/xfs/xfs_dquot_item.c +++ b/fs/xfs/xfs_dquot_item.c @@ -274,18 +274,6 @@ xfs_qm_qoff_logitem_format( xlog_finish_iovec(lv, vecp, sizeof(struct xfs_qoff_logitem)); } -/* - * There isn't much you can do to push a quotaoff item. It is simply - * stuck waiting for the log to be flushed to disk. - */ -STATIC uint -xfs_qm_qoff_logitem_push( - struct xfs_log_item *lip, - struct list_head *buffer_list) -{ - return XFS_ITEM_LOCKED; -} - STATIC xfs_lsn_t xfs_qm_qoffend_logitem_committed( struct xfs_log_item *lip, @@ -318,14 +306,12 @@ static const struct xfs_item_ops xfs_qm_qoffend_logitem_ops = { .iop_size = xfs_qm_qoff_logitem_size, .iop_format = xfs_qm_qoff_logitem_format, .iop_committed = xfs_qm_qoffend_logitem_committed, - .iop_push = xfs_qm_qoff_logitem_push, .iop_release = xfs_qm_qoff_logitem_release, }; static const struct xfs_item_ops xfs_qm_qoff_logitem_ops = { .iop_size = xfs_qm_qoff_logitem_size, .iop_format = xfs_qm_qoff_logitem_format, - .iop_push = xfs_qm_qoff_logitem_push, .iop_release = xfs_qm_qoff_logitem_release, };
The quotaoff intent item push handler unconditionally returns locked status because it remains AIL resident until removed by the quotafoff end intent. xfsaild_push_item() already returns pinned status for items (generally intents) without a push handler. This is effectively the same behavior for the purpose of quotaoff, so remove the unnecessary quotaoff push handler. Signed-off-by: Brian Foster <bfoster@redhat.com> --- fs/xfs/xfs_dquot_item.c | 14 -------------- 1 file changed, 14 deletions(-)