Re: [PATCH v5 00/17] Reference count checker and related fixes

[Adrian Hunter]

On 4/04/23 20:35, Ian Rogers wrote:
> On Tue, Apr 4, 2023 at 10:26 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>
>> On 4/04/23 18:58, Ian Rogers wrote:
>>> Ping. It would be nice to have this landed or at least the first 10
>>> patches that refactor the map API and are the bulk of the
>>> lines-of-code changed. Having those landed would make it easier to
>>> rebase in the future, but I also think the whole series is ready to
>>> go.
>>
>> I was wondering if the handling of dynamic data like struct map makes
>> any sense at present.  Perhaps someone can reassure me.
>>
>> A struct map can be updated when an MMAP event is processed.  So it
>> seems like anything racing with event processing is already broken, and
>> reference counting / locking cannot help - unless there is also
>> copy-on-write (which there isn't at present)?
>>
>> For struct maps, referencing it while simultaneously processing
>> events seems to make even less sense?
> 
> Agreed. The point of this work isn't to reimplement the maps/map APIs
> but to add a layer of reference count checking. A refactor to change
> the implementation without reference counts can delete the reference
> count checking and I think that is great! I'm trying to get the code
> base, in its current shape, to be more correct guided by sanitizers.
> Unfortunately the sanitizers come from a C++ RAII world where
> maintaining reference counts is somewhat trivial, we have to work
> harder as done here.
> 
> A similar thing to refactoring maps is changing symbol. The rb_node
> there accounts for 3*8 bytes of pointer, but is just to sort the
> symbol by address. A sorted array would suffice as well complexity
> wise, freeing 16-bytes per symbol, and is already done for symbols
> sorted by name.

Ok, just stuff to keep in mind.