On 05/11/2020 10:13 AM, Jose Fonseca wrote:
Hi,
To give everybody a bit of background context, this email comes from
https://gitlab.freedesktop.org/mesa/mesa/-/issues/2911 .
The short story is that Gallium components (but not Mesa) used to have
their malloc/free calls intercepted, to satisfy certain needs: 1) memory
debugging on Windows, 2) memory accounting on embedded systems. But
with the unification of Gallium into Mesa, the gallium vs non-gallium
division got blurred, leading to some mallocs being intercepted but not
the respective frees, and vice-versa.
I admit that trying to intercept mallocs/frees for some components and
not others is error prone. We could get this going on again, it's
doable, but it's possible it would keep come causing troubles, for us or
others, over and over again.
The two needs mentioned above were mostly VMware's needs. So I've
reevaluated, and I /think/ that with some trickery we satisfy those two
needs differently. (Without wide spread source code changes.)
On the other hand, VMware is probably not the only one to have such
needs. In fact Vulkan spec added memory callbacks precisely with the
same use cases as ours, as seen
https://www.khronos.org/registry/vulkan/specs/1.2/html/chap10.html#memory-host which
states:
/Vulkan provides applications the opportunity to perform host memory
allocations on behalf of the Vulkan implementation. If this feature
is not used, the implementation will perform its own memory
allocations. Since most memory allocations are off the critical
path, this is not meant as a performance feature. *Rather, this can
be useful for certain embedded systems, for debugging purposes (e.g.
putting a guard page after all host allocations), or for memory
allocation logging.*/
And I know there were people interested in having Mesa drivers on
embedded devices on the past (the old Tunsten Graphics having even been
multiple times hired to do so), and I'm pretty sure they exist again.
Therefore, rather than shying away from memory allocation abstractions
now, I wonder if now it's not the time to actually double down on them
and ensure we do so comprehensively throughout the whole mesa, all drivers?
*
*
After all Mesa traditionally always had MALLOC*/CALLOC*/FREE wrappers
around malloc/free. As so many other projects do.
More concretely, I'd like to propose that we:
* ensure all components use MALLOC*/CALLOC*/FREE and never
malloc/calloc/free directly (unless interfacing with a 3rd party
which expects memory to be allocated/freed with malloc/free directly)
* Perhaps consider renaming MALLOC -> _mesa_malloc etc while we're at it
* introduce a mechanisms to quickly catch any mistaken use of
malloc/calloc/free, regardless compiler/OS used:
o #define malloc/free/calloc as malloc_do_not_use/free_do_not_use
to trigger compilation errors, except on files which explicely
opt out of this (source files which need to interface with 3rd
party, or source files which implement the callbacks)
o Add a cookie to MALLOC/CALLOC/FREE memory to ensure it's not
inadvertently mixed with malloc/calloc/free
The end goal is that we should be able to have a memory allocation
abstraction which can be used for all the needs above: memory debugging,
memory accounting, and satisfying Vulkan host memory callbacks.
Some might retort: why not just play some tricks with the linker, and
intercept all malloc/free calls, without actually having to modify any
source code?
Yes, that's indeed technically feasible. And is precisely the sort of
trick I was planing to resort to satisfy VMware needs without having to
further modify the source code. But for these tricks to work, it is
absolutely /imperative/ that one statically links C++ library and STL.
The problem being that if one doesn't then there's an imbalance: the
malloc/free/new/delete calls done in inline code on C++ headers will be
intercepted, where as malloc/free/new/delete calls done in code from the
shared object which is not inlined will not, causing havoc. This is OK
for us VMware (we do it already for many other reasons, including
avoiding DLL hell.) But I doubt it will be palatable for everybody
else, particularly Linux distros, to have everything statically linked.
So effectively, if one really wants to implement Vulkan host memory
callbacks, the best way is to explicitly use malloc/free abstractions,
instead of the malloc/free directly.
So before we put more time on pursuing either the "all" or "nothing"
approaches, I'd like to get a feel for where people's preferences are.
Jose
I was tinkering with this on Friday. My initial idea is to use an
opt-in approach for memory tracking/debugging. That is, where we care
about tracking/debugging we use explicit alternates to malloc/free/etc.
malloc/free/MALLOC/CALLOC_STRUCT/etc are used in thousands of places in
Mesa/gallium and touching all of them seems like a hard way to go -
maybe not so much technical as people just not liking it for various
reasons.
I prototyped a u_trackmem.h header with u_trackmem_malloc(), functions,
etc. That header also does "#define malloc dont_call_malloc_here" to
try to prevent use of malloc/free in the .c code. u_trackmem.h would
typically have to be the last #include in a .c file.
I think redefining malloc/free in the .h file is better than the -D
compiler option because as soon as we include a .h file which uses
malloc/free we get a big mess.
u_trackmem.h attached.
-Brian
/**************************************************************************
*
* Copyright 2020 VMware, Inc.
* All Rights Reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the
* "Software"), to deal in the Software without restriction, including
* without limitation the rights to use, copy, modify, merge, publish,
* distribute, sub license, and/or sell copies of the Software, and to
* permit persons to whom the Software is furnished to do so, subject to
* the following conditions:
*
* The above copyright notice and this permission notice (including the
* next paragraph) shall be included in all copies or substantial portions
* of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
* OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
* IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
* ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*
**************************************************************************/
/*
* Functions for tracking memory usage.
*
* Some modules, like LLVMpipe, need memory tracking (how much memory is used)
* in some applications (VMware).
*
* Where memory tracking might be needed, use these functions instead of
* regular malloc/free. These functions may simply be implemented with
*
* When memo
*/
#ifndef U_TRACKMEM_H
#define U_TRACKMEM_H
#include <stddef.h> // for size_t
/*
* These are the tracked memory functions. We may plug in conventional
* malloc, free, etc. or special tracking/debug functions.
*/
struct u_trackmem_funcs {
void *(*malloc_fn)(size_t size);
void *(*calloc_fn)(size_t num, size_t size);
void *(*realloc_fn)(void *ptr, size_t size);
void (*free_fn)(void *p);
void *(*align_malloc_fn)(size_t size, size_t alignment);
void *(*align_calloc_fn)(size_t size, size_t alignment);
void *(*align_realloc_fn)(void *p, size_t oldsize,
size_t newsize, size_t alignment);
void (*align_free_fn)(void *p);
void *(*memdup_fn)(void *src, size_t size);
};
/*
* Clients of trackmem do malloc/free with:
* foo = u_trackmem.malloc(16);
* u_trackmem.free(foo);
* etc.
*/
extern struct u_trackmem_funcs u_trackmem;
/* Plug in alternate allocation funcs */
extern void u_trackmem_init(const struct u_trackmem_funcs *funcs);
#if 0
// in the .c file:
{
if (funcs) {
u_trackmem = *funcs;
} else {
/* plug in defaults */
u_trackmem.malloc_fn = malloc;
u_trackmem.calloc_fn = calloc;
u_trackmem.realloc_fn = realloc;
u_trackmem.free_fn = free;
u_trackmem.align_malloc_fn = align_malloc;
u_trackmem.align_calloc_fn = align_calloc;
u_trackmem.align_realloc_fn = align_realloc;
u_trackmem.align_free_fn = align_free;
u_trackmem.memdup_fn = memdup;
}
}
#endif
static inline void *
u_trackmem_malloc(size_t size)
{
return u_trackmem.malloc_fn(size);
}
static inline void *
u_trackmem_calloc(size_t num, size_t size)
{
return u_trackmem.calloc_fn(num, size);
}
static inline void *
u_trackmem_realloc(void *ptr, size_t size)
{
return u_trackmem.realloc_fn(ptr, size);
}
static inline void
u_trackmem_free(void *p)
{
u_trackmem.free_fn(p);
}
static inline void *
u_trackmem_align_malloc(size_t size, size_t alignment)
{
return u_trackmem.align_malloc_fn(size, alignment);
}
static inline void *
u_trackmem_align_calloc(size_t size, size_t alignment)
{
return u_trackmem.align_calloc_fn(size, alignment);
}
static inline void
u_trackmem_align_free(void *p)
{
return u_trackmem.align_free_fn(p);
}
static inline void *
u_trackmem_align_realloc(void *p, size_t oldsize,
size_t newsize, size_t alignment)
{
return u_trackmem.align_realloc_fn(p, oldsize, newsize, alignment);
}
/* Helper funcs */
#define U_TRACKMEM_MALLOC_STRUCT(T) (struct T *) u_trackmem_malloc(sizeof(struct T))
#define U_TRACKMEM_CALLOC_STRUCT(T) (struct T *) u_trackmem_calloc(1, sizeof(struct T))
/*
* These prevent using malloc, free, etc. in the .c file that includes
* this header.
*/
#define malloc dont_call_malloc_here
#define calloc dont_call_calloc_here
#define realloc dont_call_realloc_here
#define free dont_call_free_here
#undef MALLOC
#undef CALLOC
#undef FREE
#undef REALLOC
#undef MALLOC_STRUCT
#undef CALLOC_STRUCT
#undef CALLOC_VARIANT_LENGTH_STRUCT
#undef align_malloc
#undef align_free
#undef align_realloc
#endif /* U_TRACKMEM_H */
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
