| Message ID | 20230514152739.962109-15-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS | expand |
On Sun, May 14, 2023 at 8:28 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > This check is unneeded. Without it, sec_name() will returns the null > string "", then section_mismatch() will return immediately. > > Anyway, special section indices do not appear quite often in these > loops. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Could almost eliminate is_shndx_special after this. I think there's only one callsite after this? (I should check with the series applied; this patch fails to apply on mainline if that's relevant for potential merge conflicts). Does doing that additional work speed things up though? > --- > > scripts/mod/modpost.c | 16 ++++------------ > 1 file changed, 4 insertions(+), 12 deletions(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 0ef9d6964b6a..28db215ecc71 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1370,7 +1370,6 @@ static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) > static void section_rela(const char *modname, struct elf_info *elf, > Elf_Shdr *sechdr) > { > - Elf_Sym *sym; > Elf_Rela *rela; > Elf_Rela r; > unsigned int r_sym; > @@ -1413,11 +1412,8 @@ static void section_rela(const char *modname, struct elf_info *elf, > continue; > break; > } > - sym = elf->symtab_start + r_sym; > - /* Skip special sections */ > - if (is_shndx_special(sym->st_shndx)) > - continue; > - check_section_mismatch(modname, elf, sym, > + > + check_section_mismatch(modname, elf, elf->symtab_start + r_sym, > fsecndx, fromsec, r.r_offset, r.r_addend); > } > } > @@ -1425,7 +1421,6 @@ static void section_rela(const char *modname, struct elf_info *elf, > static void section_rel(const char *modname, struct elf_info *elf, > Elf_Shdr *sechdr) > { > - Elf_Sym *sym; > Elf_Rel *rel; > Elf_Rela r; > unsigned int r_sym; > @@ -1472,11 +1467,8 @@ static void section_rel(const char *modname, struct elf_info *elf, > default: > fatal("Please add code to calculate addend for this architecture\n"); > } > - sym = elf->symtab_start + r_sym; > - /* Skip special sections */ > - if (is_shndx_special(sym->st_shndx)) > - continue; > - check_section_mismatch(modname, elf, sym, > + > + check_section_mismatch(modname, elf, elf->symtab_start + r_sym, > fsecndx, fromsec, r.r_offset, r.r_addend); > } > } > -- > 2.39.2 >
On Wed, May 17, 2023 at 2:23 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Sun, May 14, 2023 at 8:28 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > This check is unneeded. Without it, sec_name() will returns the null > > string "", then section_mismatch() will return immediately. > > > > Anyway, special section indices do not appear quite often in these > > loops. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > Could almost eliminate is_shndx_special after this. I think there's > only one callsite after this? (I should check with the series applied; > this patch fails to apply on mainline if that's relevant for potential > merge conflicts). > > Does doing that additional work speed things up though? Either way, Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > > --- > > > > scripts/mod/modpost.c | 16 ++++------------ > > 1 file changed, 4 insertions(+), 12 deletions(-) > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > index 0ef9d6964b6a..28db215ecc71 100644 > > --- a/scripts/mod/modpost.c > > +++ b/scripts/mod/modpost.c > > @@ -1370,7 +1370,6 @@ static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) > > static void section_rela(const char *modname, struct elf_info *elf, > > Elf_Shdr *sechdr) > > { > > - Elf_Sym *sym; > > Elf_Rela *rela; > > Elf_Rela r; > > unsigned int r_sym; > > @@ -1413,11 +1412,8 @@ static void section_rela(const char *modname, struct elf_info *elf, > > continue; > > break; > > } > > - sym = elf->symtab_start + r_sym; > > - /* Skip special sections */ > > - if (is_shndx_special(sym->st_shndx)) > > - continue; > > - check_section_mismatch(modname, elf, sym, > > + > > + check_section_mismatch(modname, elf, elf->symtab_start + r_sym, > > fsecndx, fromsec, r.r_offset, r.r_addend); > > } > > } > > @@ -1425,7 +1421,6 @@ static void section_rela(const char *modname, struct elf_info *elf, > > static void section_rel(const char *modname, struct elf_info *elf, > > Elf_Shdr *sechdr) > > { > > - Elf_Sym *sym; > > Elf_Rel *rel; > > Elf_Rela r; > > unsigned int r_sym; > > @@ -1472,11 +1467,8 @@ static void section_rel(const char *modname, struct elf_info *elf, > > default: > > fatal("Please add code to calculate addend for this architecture\n"); > > } > > - sym = elf->symtab_start + r_sym; > > - /* Skip special sections */ > > - if (is_shndx_special(sym->st_shndx)) > > - continue; > > - check_section_mismatch(modname, elf, sym, > > + > > + check_section_mismatch(modname, elf, elf->symtab_start + r_sym, > > fsecndx, fromsec, r.r_offset, r.r_addend); > > } > > } > > -- > > 2.39.2 > > > > > -- > Thanks, > ~Nick Desaulniers
On Thu, May 18, 2023 at 6:23 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Sun, May 14, 2023 at 8:28 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > This check is unneeded. Without it, sec_name() will returns the null > > string "", then section_mismatch() will return immediately. > > > > Anyway, special section indices do not appear quite often in these > > loops. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > Could almost eliminate is_shndx_special after this. I think there's > only one callsite after this? There will be no callsite of is_shndx_special(). I will remove the definition as well. (I should check with the series applied; > this patch fails to apply on mainline if that's relevant for potential > merge conflicts). > > Does doing that additional work speed things up though? I do not know. As I wrote in the comment, special indices rarely appear in the relocation. I do not believe "let's bail out earlier" does not buy us. > > > --- > > > > scripts/mod/modpost.c | 16 ++++------------ > > 1 file changed, 4 insertions(+), 12 deletions(-) > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > index 0ef9d6964b6a..28db215ecc71 100644 > > --- a/scripts/mod/modpost.c > > +++ b/scripts/mod/modpost.c > > @@ -1370,7 +1370,6 @@ static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) > > static void section_rela(const char *modname, struct elf_info *elf, > > Elf_Shdr *sechdr) > > { > > - Elf_Sym *sym; > > Elf_Rela *rela; > > Elf_Rela r; > > unsigned int r_sym; > > @@ -1413,11 +1412,8 @@ static void section_rela(const char *modname, struct elf_info *elf, > > continue; > > break; > > } > > - sym = elf->symtab_start + r_sym; > > - /* Skip special sections */ > > - if (is_shndx_special(sym->st_shndx)) > > - continue; > > - check_section_mismatch(modname, elf, sym, > > + > > + check_section_mismatch(modname, elf, elf->symtab_start + r_sym, > > fsecndx, fromsec, r.r_offset, r.r_addend); > > } > > } > > @@ -1425,7 +1421,6 @@ static void section_rela(const char *modname, struct elf_info *elf, > > static void section_rel(const char *modname, struct elf_info *elf, > > Elf_Shdr *sechdr) > > { > > - Elf_Sym *sym; > > Elf_Rel *rel; > > Elf_Rela r; > > unsigned int r_sym; > > @@ -1472,11 +1467,8 @@ static void section_rel(const char *modname, struct elf_info *elf, > > default: > > fatal("Please add code to calculate addend for this architecture\n"); > > } > > - sym = elf->symtab_start + r_sym; > > - /* Skip special sections */ > > - if (is_shndx_special(sym->st_shndx)) > > - continue; > > - check_section_mismatch(modname, elf, sym, > > + > > + check_section_mismatch(modname, elf, elf->symtab_start + r_sym, > > fsecndx, fromsec, r.r_offset, r.r_addend); > > } > > } > > -- > > 2.39.2 > > > > > -- > Thanks, > ~Nick Desaulniers
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 0ef9d6964b6a..28db215ecc71 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1370,7 +1370,6 @@ static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) static void section_rela(const char *modname, struct elf_info *elf, Elf_Shdr *sechdr) { - Elf_Sym *sym; Elf_Rela *rela; Elf_Rela r; unsigned int r_sym; @@ -1413,11 +1412,8 @@ static void section_rela(const char *modname, struct elf_info *elf, continue; break; } - sym = elf->symtab_start + r_sym; - /* Skip special sections */ - if (is_shndx_special(sym->st_shndx)) - continue; - check_section_mismatch(modname, elf, sym, + + check_section_mismatch(modname, elf, elf->symtab_start + r_sym, fsecndx, fromsec, r.r_offset, r.r_addend); } } @@ -1425,7 +1421,6 @@ static void section_rela(const char *modname, struct elf_info *elf, static void section_rel(const char *modname, struct elf_info *elf, Elf_Shdr *sechdr) { - Elf_Sym *sym; Elf_Rel *rel; Elf_Rela r; unsigned int r_sym; @@ -1472,11 +1467,8 @@ static void section_rel(const char *modname, struct elf_info *elf, default: fatal("Please add code to calculate addend for this architecture\n"); } - sym = elf->symtab_start + r_sym; - /* Skip special sections */ - if (is_shndx_special(sym->st_shndx)) - continue; - check_section_mismatch(modname, elf, sym, + + check_section_mismatch(modname, elf, elf->symtab_start + r_sym, fsecndx, fromsec, r.r_offset, r.r_addend); } }
This check is unneeded. Without it, sec_name() will returns the null string "", then section_mismatch() will return immediately. Anyway, special section indices do not appear quite often in these loops. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/modpost.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)