Re: [PATCH v3 2/6] xfs: introduce xfs_dialloc_roll()

Christoph Hellwig
2020-12-07T13:44:39Z

> + /*
> + * We want the quota changes to be associated with the next transaction,
> + * NOT this one. So, detach the dqinfo from this and attach it to the
> + * next transaction.
> + */
> + if (tp->t_dqinfo) {
> + dqinfo = tp->t_dqinfo;
> + tp->t_dqinfo = NULL;
> + tflags = tp->t_flags & XFS_TRANS_DQ_DIRTY;
> + tp->t_flags &= ~(XFS_TRANS_DQ_DIRTY);

No need for the braces here.

> + if (error) {
> + xfs_buf_relse(agibp);
> + return error;
> + }

I haven't looked over the other patches if there is a good reason for
it, but to me keeping the xfs_buf_relse in the caller would seem more
natural.

>
> +/* XXX: will be removed in the following patch */
> +int

I don't think the comment is very helpful. As of this patch the
declaration is obviously needed, and that is all that matters.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>