On Mon, Oct 14, 2024 at 01:24:19AM -0700, Christoph Hellwig wrote:
> On Thu, Oct 10, 2024 at 06:12:21PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Make chown's quota adjustments work with realtime files.
>
> Maybe explain how you made it work here?
> > @@ -1393,18 +1393,17 @@ xfs_qm_dqusage_adjust(
> >
> > ASSERT(ip->i_delayed_blks == 0);
> >
> > + lock_mode = xfs_ilock_data_map_shared(ip);
> > if (XFS_IS_REALTIME_INODE(ip)) {
> > - struct xfs_ifork *ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
> > -
> > error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> > - if (error)
> > + if (error) {
> > + xfs_iunlock(ip, lock_mode);
> > goto error0;
> > -
> > - xfs_bmap_count_leaves(ifp, &rtblks);
> > + }
>
> So this obviously was missing locking :)
Yep. Luckily you could never mount with rt+quota before, so there
weren't any corruption complaints. I guess that's one way to get to
zero bugs. :P
> > }
> > -
> > - nblks = (xfs_qcnt_t)ip->i_nblocks - rtblks;
> > + xfs_inode_count_blocks(tp, ip, &nblks, &rtblks);
> > xfs_iflags_clear(ip, XFS_IQUOTAUNCHECKED);
> > + xfs_iunlock(ip, lock_mode);
>
> But this now also forces lockign for !rt file systems, I guess
> we don't really care about an extra ilock roundtrip in chown,
> though.
Well, it's needed for correctness because you have to distinguish data
and rtblocks usage to keep the accounting correct. The only way to do
that is to walk the data fork mappings, which means ILOCK.
> The changes itself look good, but a useful commit log would be nice:
xfs: fix chown with rt quota
Make chown's quota adjustments work with realtime files. This is mostly
a matter of calling xfs_inode_count_blocks on a given file to figure out
the number of blocks allocated to the data device and to the realtime
device, and using those quantities to update the quota accounting when
the id changes. Delayed allocation reservations are moved from the old
dquot's incore reservation to the new dquot's incore reservation.
Note that there was a missing ILOCK bug in xfs_qm_dqusage_adjust that we
must fix before calling xfs_iread_extents. Prior to 2.6.37 the locking
was correct, but then someone removed the ILOCK as part of a cleanup.
Nobody noticed because nowhere in the git history have we ever supported
rt+quota so nobody can use this.
I'm leaving git breadcrumbs in case anyone is desperate enough to try to
backport the rtquota code to old kernels.
Not-Cc: <stable@xxxxxxxxxxxxxxx> # v2.6.37
Fixes: 52fda114249578 ("xfs: simplify xfs_qm_dqusage_adjust")
Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Heh, thanks. :)
--D
