Re: [PATCH 01/17] xfs: fix xquota chown transactionality wrt delalloc blocks

From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: linux-xfs@vger.kernel.org, hch@infradead.org,
	david@fromorbit.com, bfoster@redhat.com
Subject: Re: [PATCH 01/17] xfs: fix xquota chown transactionality wrt delalloc blocks
Date: Mon, 1 Feb 2021 12:14:25 +0000	[thread overview]
Message-ID: <20210201121425.GA3271714@infradead.org> (raw)
In-Reply-To: <161214503452.139387.3026688558992670901.stgit@magnolia>

On Sun, Jan 31, 2021 at 06:03:54PM -0800, Darrick J. Wong wrote:
> @@ -1785,6 +1785,28 @@ xfs_qm_vop_chown(
>  	xfs_trans_mod_dquot(tp, newdq, bfield, ip->i_d.di_nblocks);
>  	xfs_trans_mod_dquot(tp, newdq, XFS_TRANS_DQ_ICOUNT, 1);
>  
> +	/*
> +	 * Back when we made quota reservations for the chown, we reserved the
> +	 * ondisk blocks + delalloc blocks with the new dquot.  Now that we've
> +	 * switched the dquots, decrease the new dquot's block reservation
> +	 * (having already bumped up the real counter) so that we don't have
> +	 * any reservation to give back when we commit.
> +	 */
> +	xfs_trans_mod_dquot(tp, newdq, XFS_TRANS_DQ_RES_BLKS,
> +			-ip->i_delayed_blks);
> +
> +	/*
> +	 * Give the incore reservation for delalloc blocks back to the old
> +	 * dquot.  We don't normally handle delalloc quota reservations
> +	 * transactionally, so just lock the dquot and subtract from the
> +	 * reservation.  We've dirtied the transaction, so it's too late to
> +	 * turn back now.
> +	 */
> +	tp->t_flags |= XFS_TRANS_DIRTY;
> +	xfs_dqlock(prevdq);
> +	prevdq->q_blk.reserved -= ip->i_delayed_blks;
> +	xfs_dqunlock(prevdq);

Any particular reason for this order of operations vs grouping it with
the existing prevdq and newdq sections?

Otherwise loooks fine:

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