svn commit: r334702 - head/sys/sys

[Mateusz Guzik]

On Wed, Jun 6, 2018 at 1:35 PM, Ravi Pokala <rpokala at freebsd.org> wrote:

> > + * Passing the flag down requires malloc to blindly zero the entire
> object.
> > + * In practice a lot of the zeroing can be avoided if most of the object
> > + * gets explicitly initialized after the allocation. Letting the
> compiler
> > + * zero in place gives it the opportunity to take advantage of this
> state.
>
> This part, I still don't understand. :-(
>
> The call to bzero() is still for the full length passed in, so how does
> this help?
>
>
bzero is:
#define bzero(buf, len) __builtin_memset((buf), 0, (len))

> ...
> > + *   _malloc_item = malloc(_size, type, (flags) &~ M_ZERO);
> > + *   if (((flags) & M_WAITOK) != 0 || _malloc_item != NULL)
> > + *           bzero(_malloc_item, _size);
> > + *
> > + * If the flag is set, the compiler knows the left side is always true,
> > + * therefore the entire statement is true and the callsite is:
>
> I think you mean "... the *right* side is always true ...", since the left
> side is the check for the flag being set. "If the flag is set, compiler
> knows (the check for the flag being set) is always true" is tautological.
>

It explains how __builtin_constant_p(flags) being true allows the compiler
to optimize out the flags-based check.

I don't understand why this particular use runs into so much confusion.
Just above it there is a M_ZERO check relying on the same property and
receiving no attention.

> ...
> > + * If the flag is not set, the compiler knows the left size is always
> false
> > + * and the NULL check is needed, therefore the callsite is:
>
> Same issue here.
>
> > ...
> >  #ifdef _KERNEL
> >  #define      malloc(size, type, flags) ({
>       \
>
> Now that I'm taking another look at this, I'm confused as to why the
> entire macro expansion is inside parentheses? (The braces make sense, since
> this is a block with local variables which need to be contained.)
>
>
It is to return the value (the last expression).


> >       void *_malloc_item;                                             \
> > @@ -193,7 +228,8 @@ void      *malloc(size_t size, struct malloc_type
> *type, in
> >       if (__builtin_constant_p(size) && __builtin_constant_p(flags) &&\
> >           ((flags) & M_ZERO) != 0) {                                  \
> >               _malloc_item = malloc(_size, type, (flags) &~ M_ZERO);  \
> > -             if (((flags) & M_WAITOK) != 0 || _malloc_item != NULL)  \
> > +             if (((flags) & M_WAITOK) != 0 ||                        \
> > +                 __predict_true(_malloc_item != NULL))               \
> >                       bzero(_malloc_item, _size);                     \
> >       } else {                                                        \
> >               _malloc_item = malloc(_size, type, flags);              \
>
> This confuses me too. If the constant-size/constant-flags/M_ZERO-is-set
> test fails, then it falls down to calling malloc(). Which we are in the
> middle of defining. So what does that expand to?
>
>
Expansion is not recursive, so this is an actual call to malloc.

-- 
Mateusz Guzik <mjguzik gmail.com>