[PATCH v4 0/4] Improve kernel modules symbol searching performance
by Tao Liu
This patch set improves symbol_search/symbol_exists performance by introducing hash
table for kernel modules symbols.
v3 -> v4:
1) Removed unused variables in function symbol_search/symbol_exists.
2) Replaced 'assert' with syment_is_installed check function.
3) Add Reviewed-by sign for the first 3 patches.
Tao Liu (4):
Improve the performance of symbol searching for kernel modules
Get the absolute value of SYMNAME_HASH_INDEX
Set name_hash_next field to be NULL for the newly installed elements
Add check if an syment element is installed one more time
defs.h | 3 +-
kernel.c | 1 +
symbols.c | 192 ++++++++++++++++++++++++++++++------------------------
3 files changed, 109 insertions(+), 87 deletions(-)
--
2.29.2
2 years, 11 months
Re: [Crash-utility] [PATCH v4 4/4] Add check if an syment element is installed one more time
by lijiang
> Date: Sat, 18 Sep 2021 15:59:32 +0800
> From: Tao Liu <ltao(a)redhat.com>
> To: crash-utility(a)redhat.com
> Subject: [Crash-utility] [PATCH v4 4/4] Add check if an syment element
> is installed one more time
> Message-ID: <20210918075932.132339-5-ltao(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"
>
> symname_hash_install won't check if spn has been installed before. If
> it does, the second install will corrupt the hash table as well as
> spn->cnt counting. This patch adds the check to avoid such risks.
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> symbols.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/symbols.c b/symbols.c
> index f7157b1..6d12c55 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -1147,6 +1147,20 @@ mod_symtable_hash_remove_range(struct syment *from, struct syment *to)
> symname_hash_remove(st->mod_symname_hash, sp);
> }
>
> +static inline int
> +syment_is_installed(struct syment *table[], struct syment *spn)
> +{
> + struct syment *sp;
> + int index;
> +
> + index = SYMNAME_HASH_INDEX(spn->name);
> + for (sp = table[index]; sp; sp = sp->name_hash_next) {
> + if (sp == spn)
> + return TRUE;
> + }
> + return FALSE;
> +}
> +
> /*
> * Install a single static kernel symbol into the symname_hash.
> */
> @@ -1156,7 +1170,7 @@ symname_hash_install(struct syment *table[], struct syment *spn)
> struct syment *sp;
> int index;
>
> - if (!spn)
> + if (!spn || syment_is_installed(table, spn))
> return;
Here, why don't we call the existing symname_hash_search() and
redefine a new syment_is_installed()? Could you please describe more
details?
Thanks.
Lianbo
>
> index = SYMNAME_HASH_INDEX(spn->name);
> --
> 2.29.2
2 years, 11 months