On Mon, Aug 29, 2022 at 4:18 PM Delyan Kratunov <delyank@xxxxxx> wrote: > > > > > It is not very precise, but until those maps are gone it delays > > release of the allocator (we can empty all percpu caches to save > > memory once bpf_map pinning the allocator is gone, because allocations > > are not going to be served). But it allows unit_free to be relatively > > less costly as long as those 'candidate' maps are around. > > Yes, we considered this but it's much easier to get to pathological behaviors, by > just loading and unloading programs that can access an allocator in a loop. The > freelists being empty help but it's still quite easy to hold a lot of memory for > nothing. > > The pointer walk was proposed to prune most such pathological cases while still being > conservative enough to be easy to implement. Only races with the pointer walk can > extend the lifetime unnecessarily. I'm getting lost in this thread. Here is my understanding so far: We don't free kernel kptrs from map in release_uref, but we should for local kptrs, since such objs are not much different from timers. So release_uref will xchg all such kptrs and free them into the allocator without touching allocator's refcnt. So there is no concurrency issue that Kumar was concerned about. We might need two arrays though. prog->used_allocators[] and map->used_allocators[] The verifier would populate both at load time. At prog unload dec refcnt in one array. At map free dec refcnt in the other array. Map-in-map insert/delete of new map would copy allocators[] from outer map. As the general suggestion to solve this problem I think we really need to avoid run-time refcnt changes at alloc/free even when they're per-cpu 'fast'.
