svn commit: r334702 - head/sys/sys

[Ravi Pokala]

Hi Mateusz,

-----Original Message-----
From: <owner-src-committers at freebsd.org> on behalf of Mateusz Guzik <mjg at FreeBSD.org>
Date: 2018-06-06, Wednesday at 01:08
To: <src-committers at freebsd.org>, <svn-src-all at freebsd.org>, <svn-src-head at 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
> 
> 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 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?

> ...
> + *	_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.

> ...
> + * 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.)

>  	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?

Thanks,

Ravi (rpokala@)