Re: svn commit: r334545 - in head/sys: contrib/zstd/lib/freebsd kern netinet/libalias sys
On Tue, Jun 05, 2018 at 06:35:37PM +0200, Mateusz Guzik wrote:
M> > On 06/02/2018 17:20, Mateusz Guzik wrote:
M> > > +#ifdef _KERNEL
M> > > +#define malloc(size, type, flags) ({\
M> > > + void *_malloc_item; \
M> > > + size_t _size = (size); \
M> > > + if (__builtin_constant_p(size) && __builtin_constant_p(flags) &&\
M> > > + ((flags) & M_ZERO)) { \
M> > > + _malloc_item = malloc(_size, type, (flags) &~ M_ZERO); \
M> > > + if (((flags) & M_WAITOK) || _malloc_item != NULL) \
M> > > + bzero(_malloc_item, _size); \
M> > > + } else {\
M> > > + _malloc_item = malloc(_size, type, flags); \
M> > > + } \
M> > > + _malloc_item; \
M> > > +})
M> > > +#endif
M> >
M> > Mateusz,
M> >
M> > Thank you for this and for all of your performance work. It is all very
M> > interesting stuff.
M> >
M> >
M> Thank you for the kind words. It is positive feedback like this which
M> keeps me going!
Btw, what was the point of checking flags || result? Most places in kernel
ignore flags and just test result regerdless of M_WAITOK/M_NOWAIT.
The result is already in a register, why do you think checking for absense
of M_WAITOK is faster that checking for !NULL _malloc_item?
--
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r334545 - in head/sys: contrib/zstd/lib/freebsd kern netinet/libalias sys
On Wed, Jun 06, 2018 at 12:56:47AM +0200, Mateusz Guzik wrote: M> > M> Thank you for the kind words. It is positive feedback like this which M> > M> keeps me going! M> > M> > Btw, what was the point of checking flags || result? Most places in kernel M> > ignore flags and just test result regerdless of M_WAITOK/M_NOWAIT. M> > M> > The result is already in a register, why do you think checking for absense M> > of M_WAITOK is faster that checking for !NULL _malloc_item? M> > M> M> This part is only reachable if flags are known at compilation time. If they M> contain M> M_WAITOK, the flag check will get elided along (we know for a fact it M> passes) M> and subsequently the NULL check will be short circuited, iow for known M> M_WAITOK|M_ZERO flags this is: M> M> _malloc_item = malloc(_size, type, flags & ~ M_ZERO); M> bzero(_malloc_item, _size); Thanks a lot for explanation! -- Gleb Smirnoff ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r334545 - in head/sys: contrib/zstd/lib/freebsd kern netinet/libalias sys
On Wed, Jun 6, 2018 at 12:52 AM, Gleb Smirnoff wrote:
> On Tue, Jun 05, 2018 at 06:35:37PM +0200, Mateusz Guzik wrote:
> M> > On 06/02/2018 17:20, Mateusz Guzik wrote:
> M> > > +#ifdef _KERNEL
> M> > > +#define malloc(size, type, flags) ({
> \
> M> > > + void *_malloc_item;
>\
> M> > > + size_t _size = (size);
> \
> M> > > + if (__builtin_constant_p(size) && __builtin_constant_p(flags)
> &&\
> M> > > + ((flags) & M_ZERO)) {
>\
> M> > > + _malloc_item = malloc(_size, type, (flags) &~
> M_ZERO); \
> M> > > + if (((flags) & M_WAITOK) || _malloc_item != NULL)
>\
> M> > > + bzero(_malloc_item, _size);
>\
> M> > > + } else {
> \
> M> > > + _malloc_item = malloc(_size, type, flags);
> \
> M> > > + }
>\
> M> > > + _malloc_item;
>\
> M> > > +})
> M> > > +#endif
> M> >
> M> > Mateusz,
> M> >
> M> > Thank you for this and for all of your performance work. It is all
> very
> M> > interesting stuff.
> M> >
> M> >
> M> Thank you for the kind words. It is positive feedback like this which
> M> keeps me going!
>
> Btw, what was the point of checking flags || result? Most places in kernel
> ignore flags and just test result regerdless of M_WAITOK/M_NOWAIT.
>
> The result is already in a register, why do you think checking for absense
> of M_WAITOK is faster that checking for !NULL _malloc_item?
>
This part is only reachable if flags are known at compilation time. If they
contain
M_WAITOK, the flag check will get elided along (we know for a fact it
passes)
and subsequently the NULL check will be short circuited, iow for known
M_WAITOK|M_ZERO flags this is:
_malloc_item = malloc(_size, type, flags & ~ M_ZERO);
bzero(_malloc_item, _size);
--
Mateusz Guzik
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r334545 - in head/sys: contrib/zstd/lib/freebsd kern netinet/libalias sys
On Tue, Jun 5, 2018 at 5:38 PM, Eric van Gyzen wrote:
> On 06/02/2018 17:20, Mateusz Guzik wrote:
> > +#ifdef _KERNEL
> > +#define malloc(size, type, flags) ({\
> > + void *_malloc_item; \
> > + size_t _size = (size); \
> > + if (__builtin_constant_p(size) && __builtin_constant_p(flags) &&\
> > + ((flags) & M_ZERO)) { \
> > + _malloc_item = malloc(_size, type, (flags) &~ M_ZERO); \
> > + if (((flags) & M_WAITOK) || _malloc_item != NULL) \
> > + bzero(_malloc_item, _size); \
> > + } else {\
> > + _malloc_item = malloc(_size, type, flags); \
> > + } \
> > + _malloc_item; \
> > +})
> > +#endif
>
> Mateusz,
>
> Thank you for this and for all of your performance work. It is all very
> interesting stuff.
>
>
Thank you for the kind words. It is positive feedback like this which
keeps me going!
> Coverity complains about this line:
>
> if (((flags) & M_WAITOK) || _malloc_item != NULL)
>
> saying:
>
> The expression
> 1 /* (2 | 0x100) & 2 */ || _malloc_item != NULL
> is suspicious because it performs a Boolean operation
> on a constant other than 0 or 1.
>
> Would you mind adding != 0 to appease Coverity?
>
Please go ahead.
--
Mateusz Guzik
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r334545 - in head/sys: contrib/zstd/lib/freebsd kern netinet/libalias sys
On 06/02/2018 17:20, Mateusz Guzik wrote:
> +#ifdef _KERNEL
> +#define malloc(size, type, flags) ({\
> + void *_malloc_item; \
> + size_t _size = (size); \
> + if (__builtin_constant_p(size) && __builtin_constant_p(flags) &&\
> + ((flags) & M_ZERO)) { \
> + _malloc_item = malloc(_size, type, (flags) &~ M_ZERO); \
> + if (((flags) & M_WAITOK) || _malloc_item != NULL) \
> + bzero(_malloc_item, _size); \
> + } else {\
> + _malloc_item = malloc(_size, type, flags); \
> + } \
> + _malloc_item; \
> +})
> +#endif
Mateusz,
Thank you for this and for all of your performance work. It is all very
interesting stuff.
Coverity complains about this line:
if (((flags) & M_WAITOK) || _malloc_item != NULL)
saying:
The expression
1 /* (2 | 0x100) & 2 */ || _malloc_item != NULL
is suspicious because it performs a Boolean operation
on a constant other than 0 or 1.
Would you mind adding != 0 to appease Coverity?
Thanks,
Eric
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r334545 - in head/sys: contrib/zstd/lib/freebsd kern netinet/libalias sys
Author: mjg
Date: Sat Jun 2 22:20:09 2018
New Revision: 334545
URL: https://svnweb.freebsd.org/changeset/base/334545
Log:
malloc: try to use builtins for zeroing at the callsite
Plenty of allocation sites pass M_ZERO and sizes which are small and known
at compilation time. Handling them internally in malloc loses this information
and results in avoidable calls to memset.
Instead, let the compiler take the advantage of it whenever possible.
Discussed with: jeff
Modified:
head/sys/contrib/zstd/lib/freebsd/stdlib.h
head/sys/kern/kern_malloc.c
head/sys/netinet/libalias/alias_mod.h
head/sys/sys/malloc.h
Modified: head/sys/contrib/zstd/lib/freebsd/stdlib.h
==
--- head/sys/contrib/zstd/lib/freebsd/stdlib.h Sat Jun 2 22:12:57 2018
(r334544)
+++ head/sys/contrib/zstd/lib/freebsd/stdlib.h Sat Jun 2 22:20:09 2018
(r334545)
@@ -35,6 +35,7 @@
MALLOC_DECLARE(M_ZSTD);
+#undef malloc
#definemalloc(x) (malloc)((x), M_ZSTD, M_WAITOK)
#definefree(x) (free)((x), M_ZSTD)
#definecalloc(a, b)(mallocarray)((a), (b), M_ZSTD, M_WAITOK |
M_ZERO)
Modified: head/sys/kern/kern_malloc.c
==
--- head/sys/kern/kern_malloc.c Sat Jun 2 22:12:57 2018(r334544)
+++ head/sys/kern/kern_malloc.c Sat Jun 2 22:20:09 2018(r334545)
@@ -549,7 +549,7 @@ malloc_dbg(caddr_t *vap, size_t *sizep, struct malloc_
* the allocation fails.
*/
void *
-malloc(size_t size, struct malloc_type *mtp, int flags)
+(malloc)(size_t size, struct malloc_type *mtp, int flags)
{
int indx;
caddr_t va;
Modified: head/sys/netinet/libalias/alias_mod.h
==
--- head/sys/netinet/libalias/alias_mod.h Sat Jun 2 22:12:57 2018
(r334544)
+++ head/sys/netinet/libalias/alias_mod.h Sat Jun 2 22:20:09 2018
(r334545)
@@ -41,6 +41,7 @@ MALLOC_DECLARE(M_ALIAS);
/* Use kernel allocator. */
#if defined(_SYS_MALLOC_H_)
+#undef malloc
#definemalloc(x) malloc(x, M_ALIAS, M_NOWAIT|M_ZERO)
#definecalloc(n, x)mallocarray((n), (x), M_ALIAS, M_NOWAIT|M_ZERO)
#definefree(x) free(x, M_ALIAS)
Modified: head/sys/sys/malloc.h
==
--- head/sys/sys/malloc.h Sat Jun 2 22:12:57 2018(r334544)
+++ head/sys/sys/malloc.h Sat Jun 2 22:20:09 2018(r334545)
@@ -38,6 +38,9 @@
#define_SYS_MALLOC_H_
#include
+#ifdef _KERNEL
+#include
+#endif
#include
#include
#include
@@ -183,6 +186,22 @@ void free(void *addr, struct malloc_type *type);
void free_domain(void *addr, struct malloc_type *type);
void *malloc(size_t size, struct malloc_type *type, int flags) __malloc_like
__result_use_check __alloc_size(1);
+#ifdef _KERNEL
+#definemalloc(size, type, flags) ({
\
+ void *_malloc_item; \
+ size_t _size = (size); \
+ if (__builtin_constant_p(size) && __builtin_constant_p(flags) &&\
+ ((flags) & M_ZERO)) { \
+ _malloc_item = malloc(_size, type, (flags) &~ M_ZERO); \
+ if (((flags) & M_WAITOK) || _malloc_item != NULL) \
+ bzero(_malloc_item, _size); \
+ } else {\
+ _malloc_item = malloc(_size, type, flags); \
+ } \
+ _malloc_item; \
+})
+#endif
+
void *malloc_domain(size_t size, struct malloc_type *type, int domain,
int flags) __malloc_like __result_use_check __alloc_size(1);
void *mallocarray(size_t nmemb, size_t size, struct malloc_type *type,
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
