Re: [PATCH] kernel: rounddown helper function

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 05 Aug 2010 13:56:36 -0400
Eric Paris <eparis@redhat.com> wrote:

> > > I'm more used to seeing it like
> > > 
> > > #define DIV_ROUND_DOWN(n, d)	(((n) / (d)) * (d))
> > > 
> > > but since multiply/divide/modulus are usually slower, your (SELinux) way is better,
> > > I suppose.
> > > 
> > > and the usual caveats apply:  don't use these macros with expressions (nor with y
> > > or d == 0).
> > 
> > Yes, it really shouldn't reference its argument twice.  And that's easy
> > to fix.
> 
> Are you suggesting something like
> 
> #define rounddown(n, d) ({ typeof(n) __n = (n); __n - (__n % (d)); })

looks good.

> If that's what you are hoping for, would you also like to see a patch
> doing the same thing for roundup() ?

Sure.  I doubt if anything accidentally depends on the curent behavior,
although that would be amusing.

> > A fancy version would detect constant-power-of-two and do an `& (d - 1)'
> > instead of the modulus.  But probably the compiler does optimisatons in
> > that case - for unsigned types, at least.
> 
> I don't think we really need to.  My quick test shows:
> 
> #define rounddown(n, d) ({typeof((n)) __n = (n); (__n - (__n % (d)));})
> 
> int round7(unsigned int a)
> {
> 	return rounddown(a, 7);
> }
> 
> int round4(unsigned int a)
> {
> 	return rounddown(a, 4);
> }
> 
> 0000000000400504 <round7>:
>   400504:       b9 07 00 00 00          mov    $0x7,%ecx
>   400509:       89 f8                   mov    %edi,%eax
>   40050b:       31 d2                   xor    %edx,%edx
>   40050d:       f7 f1                   div    %ecx
>   40050f:       89 f8                   mov    %edi,%eax
>   400511:       29 d0                   sub    %edx,%eax
>   400513:       c3                      retq
> 
> 0000000000400514 <round4>:
>   400514:       89 f8                   mov    %edi,%eax
>   400516:       83 e0 fc                and    $0xfffffffffffffffc,%eax
>   400519:       c3                      retq
> 

OK, thanks for checking.