On Tue, May 08, 2012 at 09:28:55AM -0500, Eric Sandeen wrote:
> On 5/8/12 5:48 AM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > xfstest 270 was causing quota reservations way beyond what was sane
> > (ten to hundreds of TB) for a 4GB filesystem. There's a sign problem
> > in the error handling path of xfs_bmapi_reserve_delalloc() because
> > xfs_trans_unreserve_quota_nblks() simple negates the value passed -
> > which doesn't work for an unsigned variable. This causes
> > reservations of close to 2^32 block instead of removing a
> > reservation of a handful of blocks.
> >
> > Fix the same problem in the other xfs_trans_unreserve_quota_nblks()
> > callers where unsigned integer variables are used, too.
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Ouch!
>
> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> as far as it goes, but a couple thoughts:
>
> 1) Should the cast be done in the macro so new callers don't get tripped up?
> 2) Should we just remove the ninos argument from the macro? It's always
> passed as 0 (and could potentially suffer the same problem)
>
> something like:
3) I'm going to remove the problematic macro altogether.
I just wanted to get this fix out there while I look at the other
quota problems I've found. Hint: xfstests _check_quota_usage()
doesn't actually work *at all* on XFS (always passes without
actually checking quotas), and so it has been hiding problems with
either the tests that use it or the quota code for some time....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|