<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div class="BodyFragment"> <div class="PlainText"> <blockquote style="border-left: 3px solid rgb(200, 200, 200); border-top-color: rgb(200, 200, 200); border-right-color: rgb(200, 200, 200); border-bottom-color: rgb(200, 200, 200); padding-left: 1ex; margin-left: 0.8ex; color: rgb(102, 102, 102);"> On 05/11/2020 10:13 AM, Jose Fonseca wrote: > Hi, > > To give everybody a bit of background context, this email comes from > <a href="https://gitlab.freedesktop.org/mesa/mesa/-/issues/2911">https://gitlab.freedesktop.org/mesa/mesa/-/issues/2911</a> . > > 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 > <a href="https://www.khronos.org/registry/vulkan/specs/1.2/html/chap10.html#memory-host"> https://www.khronos.org/registry/vulkan/specs/1.2/html/chap10.html#memory-host</a> 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. <div>-Brian </div> </blockquote> <div> </div> <div>Yes, I was leaning towards "all" or "nothing" approaches, but on 2nd thought, I don't see anything wrong with this approach. A partial solution is always easier to decide, as it doesn't require unanimity. And the more modules use u_trackmem.h, the more we approach the complete coverage. </div> <div> </div> <div>The only thing to watch out is that u_trackmem.h makes it difficult to do mallocs/free on header files, but that is virtually never the case on Mesa nowadays, so it should be good. And even if we want to do it, it should be possible by splitting u_trackmem.h in two (where one header doesn't have the #define overrides.)</div> <div> </div> <div>Jose</div> <div> </div> <div> </div> </div> </div> </body> </html>