Date: Wed, 6 Jun 2018 15:01:56 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Ravi Pokala <rpokala@freebsd.org> Cc: Mateusz Guzik <mjg@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r334702 - head/sys/sys Message-ID: <CAGudoHF9Kw6gFyNkJADzCCg0vvSq-o%2BrWwxBX15cKszDSV5KiA@mail.gmail.com> In-Reply-To: <6E6E92B2-7536-4281-8EAF-72823E84902E@panasas.com> References: <201806060508.w56586c9053686@repo.freebsd.org> <6E6E92B2-7536-4281-8EAF-72823E84902E@panasas.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jun 6, 2018 at 1:35 PM, Ravi Pokala <rpokala@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>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHF9Kw6gFyNkJADzCCg0vvSq-o%2BrWwxBX15cKszDSV5KiA>