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

[Ian Rogers]

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.

Thanks,
Ian