On 31/07/2024 14:26, Arnaldo Carvalho de Melo wrote:
> On Wed, Jul 31, 2024 at 09:57:25AM +0100, Alan Maguire wrote:
>> On 30/07/2024 23:43, Matthew Maurer wrote:
>>> Without this, even with `--lang_exclude=rust` set, running on `vmlinux`
>>> with `CONFIG_RUST` enabled will lead to errors like:
>>> die__process_function: tag not supported 0x2f (template_type_parameter)!
>>> because the filtering doesn't happen until finalization, but unsupported
>>> tags are reported during loading.
>>>
>>> As an added bonus, this should speed up processing of large objects with
>>> filtered CUs, as their details will no longer be walked.
>>>
>>
>> One question on this; if we are always doing early filtering like this,
>> should the explicit cu__filter() call be removed from pahole_stealer()?
>
> When I saw the introduction of an extra callback to be used inside the
> dwarf_loader I thought that it would be used only for this specific
> language filtering feature, i.e. a defensive approach at implementing
> this to avoid unintended side effects of doing all filtering at that
> point, maybe some other feature somehow depends on the cu__filter()
> being called where it was so far.
>
> But then it is being used for all filtering, so it seems just a way to
> reduce the patch size...
>
> So I'd keep the cu->early_cu_filter() but would use it only for the
> language filtering feature, wdyt?
So if I understand correctly,
if (languages.exclude)
conf_load.early_cu_filter = cu__filter;
? Seems reasonable to me. Thanks!
Alan
> - Arnaldo
>
>>> Signed-off-by: Matthew Maurer <mmaurer@xxxxxxxxxx>
>>> ---
>>> dwarf_loader.c | 10 ++++++++++
>>> dwarves.h | 1 +
>>> pahole.c | 1 +
>>> 3 files changed, 12 insertions(+)
>>>
>>> diff --git a/dwarf_loader.c b/dwarf_loader.c
>>> index b832c93..c48dfef 100644
>>> --- a/dwarf_loader.c
>>> +++ b/dwarf_loader.c
>>> @@ -2854,6 +2854,16 @@ static int die__process(Dwarf_Die *die, struct cu *cu, struct conf_load *conf)
>>>
>>> cu->language = attr_numeric(die, DW_AT_language);
>>>
>>> + if (conf->early_cu_filter)
>>> + cu = (conf->early_cu_filter)(cu); S
>
> Also we can have it more compactly as:
>
> + if (conf->early_cu_filter)
> + cu = conf->early_cu_filter(cu);
>
> No?
>
>>> +
>>> + /*
>>> + * If we filtered this CU out, we still want to keep iterating, but
>>> + * there's no need to walk the rest of the CU info.
>>> + */
>>> + if (cu == NULL)
>>> + return DWARF_CB_OK;
>>> +
>>> if (dwarf_child(die, &child) == 0) {
>>> int err = die__process_unit(&child, cu, conf);
>>> if (err)
>>> diff --git a/dwarves.h b/dwarves.h
>>> index f5ae79f..92d102b 100644
>>> --- a/dwarves.h
>>> +++ b/dwarves.h
>>> @@ -72,6 +72,7 @@ struct conf_load {
>>> enum load_steal_kind (*steal)(struct cu *cu,
>>> struct conf_load *conf,
>>> void *thr_data);
>>> + struct cu * (*early_cu_filter)(struct cu *cu);
>>> int (*thread_exit)(struct conf_load *conf, void *thr_data);
>>> void *cookie;
>>> char *format_path;
>>> diff --git a/pahole.c b/pahole.c
>>> index 954498d..937b0a1 100644
>>> --- a/pahole.c
>>> +++ b/pahole.c
>>> @@ -3765,6 +3765,7 @@ int main(int argc, char *argv[])
>>> memset(tab, ' ', sizeof(tab) - 1);
>>>
>>> conf_load.steal = pahole_stealer;
>>> + conf_load.early_cu_filter = cu__filter;
>>> conf_load.thread_exit = pahole_thread_exit;
>>>
>>> if (conf_load.reproducible_build) {
