On Fri, May 17, 2019 at 09:31:04AM +0200, Christoph Hellwig wrote:
> If we want to push the log to make progress on the items we'd need to
> return XFS_ITEM_PINNED instead of XFS_ITEM_LOCKED. Removing the
> method will do exactly that.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/xfs_dquot_item.c | 14 --------------
> 1 file changed, 14 deletions(-)
>
> diff --git a/fs/xfs/xfs_dquot_item.c b/fs/xfs/xfs_dquot_item.c
> index 486eea151fdb..a61a8a770d7f 100644
> --- a/fs/xfs/xfs_dquot_item.c
> +++ b/fs/xfs/xfs_dquot_item.c
> @@ -288,18 +288,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;
> -}
> -
Hmm, this one is a bit interesting because it's a potential change in
behavior and I'm not sure the comment above accurately reflects the
situation. In xfs_qm_scall_quotaoff(), we log the first quotaoff item
and commit it synchronously. I believe this means it immediately goes
into the AIL. Then we have to iterate inodes to drop all dquot
references and purge the dquot cache, which can do I/O by writing back
dquot bufs before we eventually log the quotaoff_end item. All in all
this can take a bit of time (and we have test scenarios that reproduce
quotaoff log deadlocks already).
I think this change can cause AIL processing concurrent to a quotaoff in
progress to potentially force the log on every pass. I would not expect
that to have a positive effect because a log force doesn't actually help
the quotaoff progress until the quotaoff_end is committed, and that
already occurs synchronously as well. I don't think it's wise to change
behavior here, at least not without some testing and analysis around how
this impacts those already somewhat flakey quota off operations.
Brian
> STATIC xfs_lsn_t
> xfs_qm_qoffend_logitem_committed(
> struct xfs_log_item *lip,
> @@ -327,7 +315,6 @@ 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,
> };
>
> /*
> @@ -336,7 +323,6 @@ static const struct xfs_item_ops xfs_qm_qoffend_logitem_ops = {
> 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,
> };
>
> /*
> --
> 2.20.1
>
