On Mon, Dec 07, 2020 at 02:43:50PM +0100, Christoph Hellwig wrote:
> > + /*
> > + * 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.
Ok, will fix.
>
> > + 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.
This part is inherited from Dave's original patch, but I guess I could
move this to all callers if needed, no strong opinion from myself.
>
> >
> > +/* 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.
Ok, will remove it.
>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
>
