On Wednesday, January 1st, 2025 at 8:56 AM, Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Sat, Dec 21, 2024 at 01:23:45AM +0000, Ihor Solodrai wrote:
>
> SNIP
>
> > -static int btf_encoder__add_saved_funcs(bool skip_encoding_inconsistent_proto)
> > +static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_encoding_inconsistent_proto)
> > {
> > struct btf_encoder_func_state **saved_fns, *s;
> > - struct btf_encoder *e = NULL;
> > - int i = 0, j, nr_saved_fns = 0;
> > + int err = 0, i = 0, j, nr_saved_fns = 0;
> >
> > - /* Retrieve function states from each encoder, combine them
> > + /* Retrieve function states from the encoder, combine them
> > * and sort by name, addr.
> > */
> > - btf_encoders__for_each_encoder(e) {
> > - list_for_each_entry(s, &e->func_states, node)
> > - nr_saved_fns++;
> > + list_for_each_entry(s, &encoder->func_states, node) {
> > + nr_saved_fns++;
> > }
> >
> > if (nr_saved_fns == 0)
> > - return 0;
> > + goto out;
> >
> > saved_fns = calloc(nr_saved_fns, sizeof(*saved_fns));
> > - btf_encoders__for_each_encoder(e) {
> > - list_for_each_entry(s, &e->func_states, node)
> > - saved_fns[i++] = s;
> > + if (!saved_fns) {
> > + err = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + list_for_each_entry(s, &encoder->func_states, node) {
> > + saved_fns[i++] = s;
> > }
> > qsort(saved_fns, nr_saved_fns, sizeof(*saved_fns), saved_functions_cmp);
> >
> > @@ -1377,11 +1313,10 @@ static int btf_encoder__add_saved_funcs(bool skip_encoding_inconsistent_proto)
> >
> > /* Now that we are done with function states, free them. */
> > free(saved_fns);
> > - btf_encoders__for_each_encoder(e) {
> > - btf_encoder__delete_saved_funcs(e);
> > - }
> > + btf_encoder__delete_saved_funcs(encoder);
>
>
> is this call necessary? there's btf_encoder__delete call right after
> same for elf_functions_list__clear in btf_encoder__encode
At this point we know that the function information is no longer
needed, so we can free up some memory.
I remember when I wrote a "context" patch [1] (now discarded), there
was a significant difference in MAX RSS between freeing this right
away and delaying until encoding is finished. Now it might not be as
significant, but still there is no reason to keep the stuff we know is
not used later.
[1] https://lore.kernel.org/dwarves/20241213223641.564002-8-ihor.solodrai@xxxxx/
>
> thanks,
> jirka
>
> > - return 0;
> > +out:
> > + return err;
> > }
>
>
> SNIP
