| Message ID | 20201031223131.3398153-2-jolsa@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | [1/2] btf_encoder: Move find_all_percpu_vars in generic collect_symbols | expand |
This looks good to me. Thanks, Jiri. Acked-by: Hao Luo <haoluo@google.com> On Sat, Oct 31, 2020 at 3:31 PM Jiri Olsa <jolsa@kernel.org> wrote: > > Moving find_all_percpu_vars under generic collect_symbols > function that walks over symbols and calls collect_percpu_var. > > We will add another collect function that needs to go through > all the symbols, so it's better we go through them just once. > > There's no functional change intended. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > btf_encoder.c | 124 +++++++++++++++++++++++++++----------------------- > 1 file changed, 67 insertions(+), 57 deletions(-) > > diff --git a/btf_encoder.c b/btf_encoder.c > index 4c92908beab2..1866bb16a8ba 100644 > --- a/btf_encoder.c > +++ b/btf_encoder.c > @@ -250,75 +250,85 @@ 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 collect_percpu_var(struct btf_elf *btfe, GElf_Sym *sym) > { > - uint32_t core_id; > - GElf_Sym sym; > + const char *sym_name; > + uint64_t addr; > + uint32_t size; > > - /* cache variables' addresses, preparing for searching in symtab. */ > - percpu_var_cnt = 0; > + /* compare a symbol's shndx to determine if it's a percpu variable */ > + if (elf_sym__section(sym) != btfe->percpu_shndx) > + return 0; > + if (elf_sym__type(sym) != STT_OBJECT) > + return 0; > > - /* search within symtab for percpu variables */ > - elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) { > - const char *sym_name; > - uint64_t addr; > - uint32_t size; > + addr = elf_sym__value(sym); > + /* > + * Store only those symbols that have allocated space in the percpu section. > + * This excludes the following three types of symbols: > + * > + * 1. __ADDRESSABLE(sym), which are forcely emitted as symbols. > + * 2. __UNIQUE_ID(prefix), which are introduced to generate unique ids. > + * 3. __exitcall(fn), functions which are labeled as exit calls. > + * > + * In addition, the variables defined using DEFINE_PERCPU_FIRST are > + * also not included, which currently includes: > + * > + * 1. fixed_percpu_data > + */ > + if (!addr) > + return 0; > > - /* compare a symbol's shndx to determine if it's a percpu variable */ > - if (elf_sym__section(&sym) != btfe->percpu_shndx) > - continue; > - if (elf_sym__type(&sym) != STT_OBJECT) > - continue; > + size = elf_sym__size(sym); > + if (!size) > + return 0; /* ignore zero-sized symbols */ > > - addr = elf_sym__value(&sym); > - /* > - * Store only those symbols that have allocated space in the percpu section. > - * This excludes the following three types of symbols: > - * > - * 1. __ADDRESSABLE(sym), which are forcely emitted as symbols. > - * 2. __UNIQUE_ID(prefix), which are introduced to generate unique ids. > - * 3. __exitcall(fn), functions which are labeled as exit calls. > - * > - * In addition, the variables defined using DEFINE_PERCPU_FIRST are > - * also not included, which currently includes: > - * > - * 1. fixed_percpu_data > - */ > - if (!addr) > - continue; > + sym_name = elf_sym__name(sym, btfe->symtab); > + if (!btf_name_valid(sym_name)) { > + dump_invalid_symbol("Found symbol of invalid name when encoding btf", > + sym_name, btf_elf__verbose, btf_elf__force); > + if (btf_elf__force) > + return 0; > + return -1; > + } > > - size = elf_sym__size(&sym); > - if (!size) > - continue; /* ignore zero-sized symbols */ > + if (btf_elf__verbose) > + printf("Found per-CPU symbol '%s' at address 0x%lx\n", sym_name, addr); > > - sym_name = elf_sym__name(&sym, btfe->symtab); > - if (!btf_name_valid(sym_name)) { > - dump_invalid_symbol("Found symbol of invalid name when encoding btf", > - sym_name, btf_elf__verbose, btf_elf__force); > - if (btf_elf__force) > - continue; > - return -1; > - } > + if (percpu_var_cnt == MAX_PERCPU_VAR_CNT) { > + fprintf(stderr, "Reached the limit of per-CPU variables: %d\n", > + MAX_PERCPU_VAR_CNT); > + return -1; > + } > + percpu_vars[percpu_var_cnt].addr = addr; > + percpu_vars[percpu_var_cnt].sz = size; > + percpu_vars[percpu_var_cnt].name = sym_name; > + percpu_var_cnt++; > > - if (btf_elf__verbose) > - printf("Found per-CPU symbol '%s' at address 0x%lx\n", sym_name, addr); > + return 0; > +} > + > +static int collect_symbols(struct btf_elf *btfe, bool collect_percpu_vars) > +{ > + uint32_t core_id; > + GElf_Sym sym; > > - if (percpu_var_cnt == MAX_PERCPU_VAR_CNT) { > - fprintf(stderr, "Reached the limit of per-CPU variables: %d\n", > - MAX_PERCPU_VAR_CNT); > + /* cache variables' addresses, preparing for searching in symtab. */ > + percpu_var_cnt = 0; > + > + /* search within symtab for percpu variables */ > + elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) { > + if (collect_percpu_vars && collect_percpu_var(btfe, &sym)) > return -1; > - } > - percpu_vars[percpu_var_cnt].addr = addr; > - percpu_vars[percpu_var_cnt].sz = size; > - percpu_vars[percpu_var_cnt].name = sym_name; > - percpu_var_cnt++; > } > > - if (percpu_var_cnt) > - qsort(percpu_vars, percpu_var_cnt, sizeof(percpu_vars[0]), percpu_var_cmp); > + if (collect_percpu_vars) { > + if (percpu_var_cnt) > + qsort(percpu_vars, percpu_var_cnt, sizeof(percpu_vars[0]), percpu_var_cmp); > > - if (btf_elf__verbose) > - printf("Found %d per-CPU variables!\n", percpu_var_cnt); > + if (btf_elf__verbose) > + printf("Found %d per-CPU variables!\n", percpu_var_cnt); > + } > return 0; > } > > @@ -347,7 +357,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > if (!btfe) > return -1; > > - if (!skip_encoding_vars && find_all_percpu_vars(btfe)) > + if (collect_symbols(btfe, !skip_encoding_vars)) > goto out; > > has_index_type = false; > -- > 2.26.2 >
Em Mon, Nov 02, 2020 at 10:29:22AM -0800, Hao Luo escreveu: > This looks good to me. Thanks, Jiri. > > Acked-by: Hao Luo <haoluo@google.com> Thanks, applied, will wait for a v2 for the other. > On Sat, Oct 31, 2020 at 3:31 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > Moving find_all_percpu_vars under generic collect_symbols > > function that walks over symbols and calls collect_percpu_var. > > > > We will add another collect function that needs to go through > > all the symbols, so it's better we go through them just once. > > > > There's no functional change intended. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > btf_encoder.c | 124 +++++++++++++++++++++++++++----------------------- > > 1 file changed, 67 insertions(+), 57 deletions(-) > > > > diff --git a/btf_encoder.c b/btf_encoder.c > > index 4c92908beab2..1866bb16a8ba 100644 > > --- a/btf_encoder.c > > +++ b/btf_encoder.c > > @@ -250,75 +250,85 @@ 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 collect_percpu_var(struct btf_elf *btfe, GElf_Sym *sym) > > { > > - uint32_t core_id; > > - GElf_Sym sym; > > + const char *sym_name; > > + uint64_t addr; > > + uint32_t size; > > > > - /* cache variables' addresses, preparing for searching in symtab. */ > > - percpu_var_cnt = 0; > > + /* compare a symbol's shndx to determine if it's a percpu variable */ > > + if (elf_sym__section(sym) != btfe->percpu_shndx) > > + return 0; > > + if (elf_sym__type(sym) != STT_OBJECT) > > + return 0; > > > > - /* search within symtab for percpu variables */ > > - elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) { > > - const char *sym_name; > > - uint64_t addr; > > - uint32_t size; > > + addr = elf_sym__value(sym); > > + /* > > + * Store only those symbols that have allocated space in the percpu section. > > + * This excludes the following three types of symbols: > > + * > > + * 1. __ADDRESSABLE(sym), which are forcely emitted as symbols. > > + * 2. __UNIQUE_ID(prefix), which are introduced to generate unique ids. > > + * 3. __exitcall(fn), functions which are labeled as exit calls. > > + * > > + * In addition, the variables defined using DEFINE_PERCPU_FIRST are > > + * also not included, which currently includes: > > + * > > + * 1. fixed_percpu_data > > + */ > > + if (!addr) > > + return 0; > > > > - /* compare a symbol's shndx to determine if it's a percpu variable */ > > - if (elf_sym__section(&sym) != btfe->percpu_shndx) > > - continue; > > - if (elf_sym__type(&sym) != STT_OBJECT) > > - continue; > > + size = elf_sym__size(sym); > > + if (!size) > > + return 0; /* ignore zero-sized symbols */ > > > > - addr = elf_sym__value(&sym); > > - /* > > - * Store only those symbols that have allocated space in the percpu section. > > - * This excludes the following three types of symbols: > > - * > > - * 1. __ADDRESSABLE(sym), which are forcely emitted as symbols. > > - * 2. __UNIQUE_ID(prefix), which are introduced to generate unique ids. > > - * 3. __exitcall(fn), functions which are labeled as exit calls. > > - * > > - * In addition, the variables defined using DEFINE_PERCPU_FIRST are > > - * also not included, which currently includes: > > - * > > - * 1. fixed_percpu_data > > - */ > > - if (!addr) > > - continue; > > + sym_name = elf_sym__name(sym, btfe->symtab); > > + if (!btf_name_valid(sym_name)) { > > + dump_invalid_symbol("Found symbol of invalid name when encoding btf", > > + sym_name, btf_elf__verbose, btf_elf__force); > > + if (btf_elf__force) > > + return 0; > > + return -1; > > + } > > > > - size = elf_sym__size(&sym); > > - if (!size) > > - continue; /* ignore zero-sized symbols */ > > + if (btf_elf__verbose) > > + printf("Found per-CPU symbol '%s' at address 0x%lx\n", sym_name, addr); > > > > - sym_name = elf_sym__name(&sym, btfe->symtab); > > - if (!btf_name_valid(sym_name)) { > > - dump_invalid_symbol("Found symbol of invalid name when encoding btf", > > - sym_name, btf_elf__verbose, btf_elf__force); > > - if (btf_elf__force) > > - continue; > > - return -1; > > - } > > + if (percpu_var_cnt == MAX_PERCPU_VAR_CNT) { > > + fprintf(stderr, "Reached the limit of per-CPU variables: %d\n", > > + MAX_PERCPU_VAR_CNT); > > + return -1; > > + } > > + percpu_vars[percpu_var_cnt].addr = addr; > > + percpu_vars[percpu_var_cnt].sz = size; > > + percpu_vars[percpu_var_cnt].name = sym_name; > > + percpu_var_cnt++; > > > > - if (btf_elf__verbose) > > - printf("Found per-CPU symbol '%s' at address 0x%lx\n", sym_name, addr); > > + return 0; > > +} > > + > > +static int collect_symbols(struct btf_elf *btfe, bool collect_percpu_vars) > > +{ > > + uint32_t core_id; > > + GElf_Sym sym; > > > > - if (percpu_var_cnt == MAX_PERCPU_VAR_CNT) { > > - fprintf(stderr, "Reached the limit of per-CPU variables: %d\n", > > - MAX_PERCPU_VAR_CNT); > > + /* cache variables' addresses, preparing for searching in symtab. */ > > + percpu_var_cnt = 0; > > + > > + /* search within symtab for percpu variables */ > > + elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) { > > + if (collect_percpu_vars && collect_percpu_var(btfe, &sym)) > > return -1; > > - } > > - percpu_vars[percpu_var_cnt].addr = addr; > > - percpu_vars[percpu_var_cnt].sz = size; > > - percpu_vars[percpu_var_cnt].name = sym_name; > > - percpu_var_cnt++; > > } > > > > - if (percpu_var_cnt) > > - qsort(percpu_vars, percpu_var_cnt, sizeof(percpu_vars[0]), percpu_var_cmp); > > + if (collect_percpu_vars) { > > + if (percpu_var_cnt) > > + qsort(percpu_vars, percpu_var_cnt, sizeof(percpu_vars[0]), percpu_var_cmp); > > > > - if (btf_elf__verbose) > > - printf("Found %d per-CPU variables!\n", percpu_var_cnt); > > + if (btf_elf__verbose) > > + printf("Found %d per-CPU variables!\n", percpu_var_cnt); > > + } > > return 0; > > } > > > > @@ -347,7 +357,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, > > if (!btfe) > > return -1; > > > > - if (!skip_encoding_vars && find_all_percpu_vars(btfe)) > > + if (collect_symbols(btfe, !skip_encoding_vars)) > > goto out; > > > > has_index_type = false; > > -- > > 2.26.2 > >
diff --git a/btf_encoder.c b/btf_encoder.c index 4c92908beab2..1866bb16a8ba 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -250,75 +250,85 @@ 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 collect_percpu_var(struct btf_elf *btfe, GElf_Sym *sym) { - uint32_t core_id; - GElf_Sym sym; + const char *sym_name; + uint64_t addr; + uint32_t size; - /* cache variables' addresses, preparing for searching in symtab. */ - percpu_var_cnt = 0; + /* compare a symbol's shndx to determine if it's a percpu variable */ + if (elf_sym__section(sym) != btfe->percpu_shndx) + return 0; + if (elf_sym__type(sym) != STT_OBJECT) + return 0; - /* search within symtab for percpu variables */ - elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) { - const char *sym_name; - uint64_t addr; - uint32_t size; + addr = elf_sym__value(sym); + /* + * Store only those symbols that have allocated space in the percpu section. + * This excludes the following three types of symbols: + * + * 1. __ADDRESSABLE(sym), which are forcely emitted as symbols. + * 2. __UNIQUE_ID(prefix), which are introduced to generate unique ids. + * 3. __exitcall(fn), functions which are labeled as exit calls. + * + * In addition, the variables defined using DEFINE_PERCPU_FIRST are + * also not included, which currently includes: + * + * 1. fixed_percpu_data + */ + if (!addr) + return 0; - /* compare a symbol's shndx to determine if it's a percpu variable */ - if (elf_sym__section(&sym) != btfe->percpu_shndx) - continue; - if (elf_sym__type(&sym) != STT_OBJECT) - continue; + size = elf_sym__size(sym); + if (!size) + return 0; /* ignore zero-sized symbols */ - addr = elf_sym__value(&sym); - /* - * Store only those symbols that have allocated space in the percpu section. - * This excludes the following three types of symbols: - * - * 1. __ADDRESSABLE(sym), which are forcely emitted as symbols. - * 2. __UNIQUE_ID(prefix), which are introduced to generate unique ids. - * 3. __exitcall(fn), functions which are labeled as exit calls. - * - * In addition, the variables defined using DEFINE_PERCPU_FIRST are - * also not included, which currently includes: - * - * 1. fixed_percpu_data - */ - if (!addr) - continue; + sym_name = elf_sym__name(sym, btfe->symtab); + if (!btf_name_valid(sym_name)) { + dump_invalid_symbol("Found symbol of invalid name when encoding btf", + sym_name, btf_elf__verbose, btf_elf__force); + if (btf_elf__force) + return 0; + return -1; + } - size = elf_sym__size(&sym); - if (!size) - continue; /* ignore zero-sized symbols */ + if (btf_elf__verbose) + printf("Found per-CPU symbol '%s' at address 0x%lx\n", sym_name, addr); - sym_name = elf_sym__name(&sym, btfe->symtab); - if (!btf_name_valid(sym_name)) { - dump_invalid_symbol("Found symbol of invalid name when encoding btf", - sym_name, btf_elf__verbose, btf_elf__force); - if (btf_elf__force) - continue; - return -1; - } + if (percpu_var_cnt == MAX_PERCPU_VAR_CNT) { + fprintf(stderr, "Reached the limit of per-CPU variables: %d\n", + MAX_PERCPU_VAR_CNT); + return -1; + } + percpu_vars[percpu_var_cnt].addr = addr; + percpu_vars[percpu_var_cnt].sz = size; + percpu_vars[percpu_var_cnt].name = sym_name; + percpu_var_cnt++; - if (btf_elf__verbose) - printf("Found per-CPU symbol '%s' at address 0x%lx\n", sym_name, addr); + return 0; +} + +static int collect_symbols(struct btf_elf *btfe, bool collect_percpu_vars) +{ + uint32_t core_id; + GElf_Sym sym; - if (percpu_var_cnt == MAX_PERCPU_VAR_CNT) { - fprintf(stderr, "Reached the limit of per-CPU variables: %d\n", - MAX_PERCPU_VAR_CNT); + /* cache variables' addresses, preparing for searching in symtab. */ + percpu_var_cnt = 0; + + /* search within symtab for percpu variables */ + elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) { + if (collect_percpu_vars && collect_percpu_var(btfe, &sym)) return -1; - } - percpu_vars[percpu_var_cnt].addr = addr; - percpu_vars[percpu_var_cnt].sz = size; - percpu_vars[percpu_var_cnt].name = sym_name; - percpu_var_cnt++; } - if (percpu_var_cnt) - qsort(percpu_vars, percpu_var_cnt, sizeof(percpu_vars[0]), percpu_var_cmp); + if (collect_percpu_vars) { + if (percpu_var_cnt) + qsort(percpu_vars, percpu_var_cnt, sizeof(percpu_vars[0]), percpu_var_cmp); - if (btf_elf__verbose) - printf("Found %d per-CPU variables!\n", percpu_var_cnt); + if (btf_elf__verbose) + printf("Found %d per-CPU variables!\n", percpu_var_cnt); + } return 0; } @@ -347,7 +357,7 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force, if (!btfe) return -1; - if (!skip_encoding_vars && find_all_percpu_vars(btfe)) + if (collect_symbols(btfe, !skip_encoding_vars)) goto out; has_index_type = false;
Moving find_all_percpu_vars under generic collect_symbols function that walks over symbols and calls collect_percpu_var. We will add another collect function that needs to go through all the symbols, so it's better we go through them just once. There's no functional change intended. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- btf_encoder.c | 124 +++++++++++++++++++++++++++----------------------- 1 file changed, 67 insertions(+), 57 deletions(-)