[DTrace-devel] [PATCH 1/7] htab reduction: kernpath

[Nick Alcock]

On 8 Oct 2021, Eugene Loh spake thusly:

> Small nit:  it would seem that cmd/ctf_module_dump.c no longer needs to 
> #define BUCKETS.  So that line can be removed.

Ack, though I've split it into a separate commit that just removes
ctf_module_dump entirely. (And another commit, also before this one,
that drops the dead-long-before-BPF-DTrace ctf.ko support and requires a
vmlinux.ctfa, saving a pile of complexity and refcounting of dt_modules
and the like. Will post for review in the next round of reviews.)

> Mostly, I have some higher level questions about why dtp->dt_nkernpaths 
> and dtp->dt_kernpathlist are maintained.
>
> It seems that dtp->dt_nkernpaths is used only to see if the htab is 

Originally they had many more purposes, but as you noticed (and I
didn't) they have become pretty much residual now. Dropping them is the
right thing to do.

> Actually, to test whether we need to initialize dtp->dt_kernpaths, 
> instead of checking dt_nkernpaths, how about simply checking 
> dtp->dt_kernpaths!=NULL.

YES.

> The other use of dt_nkernpaths is for destroying the kernpaths htab.  
> This is apparently also the only reason that dtp->dt_kernpathlist is 
> maintained.
[...]
>               Or, probably better, just allow callers to specify a 
> destructor (along with hval, cmp, etc.);  then have dt_htab_destroy() 
> iterate over entries, calling the destructor, and finally destroying the 
> htab?

I was thinking "we have to store the things somewhere", but, you're
right: if we add a destructor we can just use that and no longer need to
drag around ancillary lists everywhere, and the htab can (if desired) be
considered to be storing things itself (while still allowing you to
store it elsewhere and just stuff pointers to it into the htab, by just
not freeing the elements in a destructor).

This would simplify the speculations work too, but rather than change
that commit so much at this late date I'll just adjust it to account for
this.

The hard part is iterating over entries though. I might need to augment
the dt_htab_ops_t to add a .next, since the link pointers etc are
completely customizable.

-- 
NULL && (void)