Agree with Andrii. The function name 'config' is too generic and
'config_per_var' is confusing to me. But the rest looked good. Thanks
for taking a look at this.
Hao
On Tue, Oct 27, 2020 at 4:12 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Mon, Oct 26, 2020 at 5:07 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> >
> > Moving find_all_percpu_vars under generic onfig function
> > that walks over symbols and calls config_percpu_var.
> >
> > We will add another config function that needs to go
> > through all the symbols, so it's better they go through
> > them just once.
> >
> > There's no functional change intended.
> >
> > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > ---
> > btf_encoder.c | 126 ++++++++++++++++++++++++++------------------------
> > 1 file changed, 66 insertions(+), 60 deletions(-)
> >
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index 2a6455be4c52..2dd26c904039 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -250,7 +250,64 @@ static bool percpu_var_exists(uint64_t addr, uint32_t *sz, const char **name)
> > return true;
> > }
> >
> > -static int find_all_percpu_vars(struct btf_elf *btfe)
> > +static int config_percpu_var(struct btf_elf *btfe, GElf_Sym *sym)
>
> I find the "config" name completely misleading. How about
> "collect_percpu_var" or something along those lines?
>
> > +{
> > + const char *sym_name;
> > + uint64_t addr;
> > + uint32_t size;
> > +
>
> [...]
>
> > +}
> > +
> > +static int config(struct btf_elf *btfe, bool do_percpu_vars)
>
> same here, config is generic and misrepresenting what we are doing
> here. E.g., collect_symbols would probably be more clear.
>
> > {
> > uint32_t core_id;
> > GElf_Sym sym;
>
> [...]
