Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 06 Jun 2018 07:35:12 -0400
From:      Ravi Pokala <rpokala@freebsd.org>
To:        Mateusz Guzik <mjg@FreeBSD.org>, <src-committers@freebsd.org>, <svn-src-all@freebsd.org>, <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r334702 - head/sys/sys
Message-ID:  <6E6E92B2-7536-4281-8EAF-72823E84902E@panasas.com>
In-Reply-To: <201806060508.w56586c9053686@repo.freebsd.org>
References:  <201806060508.w56586c9053686@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi Mateusz,

=EF=BB=BF-----Original Message-----
From: <owner-src-committers@freebsd.org> on behalf of Mateusz Guzik <mjg@Fr=
eeBSD.org>
Date: 2018-06-06, Wednesday at 01:08
To: <src-committers@freebsd.org>, <svn-src-all@freebsd.org>, <svn-src-head@=
freebsd.org>
Subject: svn commit: r334702 - head/sys/sys

> Author: mjg
> Date: Wed Jun  6 05:08:05 2018
> New Revision: 334702
> URL: https://svnweb.freebsd.org/changeset/base/334702
>=20
> Log:
>   malloc: elaborate on r334545 due to frequent questions

I was one of the questioners. :-) Thank you for explaining the conditional =
logic.

> ...
> + * Passing the flag down requires malloc to blindly zero the entire obje=
ct.
> + * In practice a lot of the zeroing can be avoided if most of the object
> + * gets explicitly initialized after the allocation. Letting the compile=
r
> + * zero in place gives it the opportunity to take advantage of this stat=
e.

This part, I still don't understand. :-(

The call to bzero() is still for the full length passed in, so how does thi=
s help?

> ...
> + *	_malloc_item =3D malloc(_size, type, (flags) &~ M_ZERO);
> + *	if (((flags) & M_WAITOK) !=3D 0 || _malloc_item !=3D 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 know=
s (the check for the flag being set) is always true" is tautological.

> ...
> + * If the flag is not set, the compiler knows the left size is always fa=
lse
> + * 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 i=
s a block with local variables which need to be contained.)

>  	void *_malloc_item;						\
> @@ -193,7 +228,8 @@ void	*malloc(size_t size, struct malloc_type *type, i=
n
>  	if (__builtin_constant_p(size) && __builtin_constant_p(flags) &&\
>  	    ((flags) & M_ZERO) !=3D 0) {					\
>  		_malloc_item =3D malloc(_size, type, (flags) &~ M_ZERO);	\
> -		if (((flags) & M_WAITOK) !=3D 0 || _malloc_item !=3D NULL)	\
> +		if (((flags) & M_WAITOK) !=3D 0 ||			\
> +		    __predict_true(_malloc_item !=3D NULL))		\
>  			bzero(_malloc_item, _size);			\
>  	} else {							\
>  		_malloc_item =3D malloc(_size, type, flags);		\

This confuses me too. If the constant-size/constant-flags/M_ZERO-is-set tes=
t fails, then it falls down to calling malloc(). Which we are in the middle =
of defining. So what does that expand to?

Thanks,

Ravi (rpokala@)





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6E6E92B2-7536-4281-8EAF-72823E84902E>