Re: [PATCH 27/30] libxfs: work around do_div() not handling 32 bit numerators

Dave Chinner
2013-11-05T01:56:01Z

On Mon, Nov 04, 2013 at 01:16:43AM -0800, Christoph Hellwig wrote:
> On Wed, Oct 30, 2013 at 03:31:18PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > The libxfs dquot buffer code uses do_div() with a 32 bit numerator.
> > This gives incorrect results as do_div() passes the numerator by
> > reference as a pointer to a 64 bit value. Hence it does the division
> > using 32 bits of garbage gives the wrong result. The kernel code
> > handles 32 bit numerators just fine, so this patch is a temporary
> > workaround in the dquot buffer code until we fix do_div() to handle
> > 32 bit numerators correctly.
>
> The right fix is to simply stop using do_div here, both in kernel and
> userspace.

Oh, right, I was thinking that BBTOB() resulted in a u64 value like
BTOBB() and BTOBBT() do. It doesn't have a built in cast, so takes
whatever type the source variable has.

> > xfs_calc_dquots_per_chunk(
> > struct xfs_mount *mp,
> > unsigned int nbblks) /* basic block units */
> > {
> > - unsigned int ndquots;
> > + uint64_t ndquots;
> >
> > ASSERT(nbblks > 0);
> > ndquots = BBTOB(nbblks);
> > do_div(ndquots, sizeof(xfs_dqblk_t));
> >
> > - return ndquots;
> > + return (int)ndquots;
>
> ndquots = BBTOB(nbblks) / sizeof(xfs_dqblk_t);

Yup, that should work.

> Also the current xfs_calc_dquots_per_chunk doesn't use the mp argument
> anyway, but all callers do. I think in the end we'd want to take that
> in from the calers end end up with a version that includes the
> check from the quotainfo. But for getting userspace up and running my
> above minimal version should do it.

Yes, changed to fix. I'll drive further cleanups from the kernel
side here...

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com