7:19 p.m.
-----Original Message-----
Hi Tao Liu,
Thanks for the update.
-----Original Message-----
> This patch indroduces mod_symname_hash, and its install/remove operations.
> Since symbol_search() has to return the lowest address symbol and
> symbol_search_next() returns the next lowest symbol, thus the installation
> should be sorted ascendingly.
>
> Install spn previous to sp:
>
> If sp is the start of bucket, and
> 1) spn->value is smaller than sp->value, or
> 2) spn->value equals sp->value and spn is smaller than sp.
>
> Install spn next to sp:
>
> 1) spn->value is larger than sp->value, or
> 2) spn->value equals to sp->value and spn is larger than sp,
> spn will stay behind of sp. And if
> 1) sp->name_hash_next is NULL or
> 2) sp->name_hash_next->value larger than spn->value
> spn will be next to sp.
>
> spn->value is the kernel address of the symbol and will not change.
> So we use it mainly to determine the sequence. When spn->value equals
> sp->value, they must be symbols within a kernel module, so we use spn
> and sp to determine the sequence.
If sp->value equals to spn->value, they are in the same module, and
spn should be larger than sp, according to mod_symtable_hash_install_range().
Then is it OK to insert spn when sp->value becomes larger than spn->value
like this?
sp = st->mod_symname_hash[index];
if (!sp) {
st->mod_symname_hash[index] = spn;
spn->name_hash_next = NULL;
return;
}
for (; sp; sp = sp->name_hash_next) {
if (sp->value > spn->value || !sp->name_hash_next) {
spn->name_hash_next = sp->name_hash_next;
sp->name_hash_next = spn;
return;
}
}
sorry, I was confused, this is a fixed one:
sp = st->mod_symname_hash[index];
if (!sp || sp->value > spn->value) {
spn->name_hash_next = sp;
st->mod_symname_hash[index] = spn;
return;
}
for (; sp; sp = sp->name_hash_next) {
if (!sp->name_hash_next || sp->name_hash_next->value >
spn->value) {
spn->name_hash_next = sp->name_hash_next;
sp->name_hash_next = spn;
return;
}
}
Thanks,
Kazu
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> defs.h | 1 +
> symbols.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 82 insertions(+)
>
> diff --git a/defs.h b/defs.h
> index cbd45e5..bbdca79 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2755,6 +2755,7 @@ struct symbol_table_data {
> double val_hash_searches;
> double val_hash_iterations;
> struct syment *symname_hash[SYMNAME_HASH];
> + struct syment *mod_symname_hash[SYMNAME_HASH];
> struct symbol_namespace kernel_namespace;
> struct syment *ext_module_symtable;
> struct syment *ext_module_symend;
> diff --git a/symbols.c b/symbols.c
> index 69dccdb..7fb2df7 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -1157,6 +1157,87 @@ symname_hash_install(struct syment *spn)
> }
> }
>
> +/*
> + * Install a single kernel module symbol into the mod_symname_hash.
> + */
> +static void
> +mod_symname_hash_install(struct syment *spn)
> +{
> + struct syment *sp, *tmp;
> + int index;
> +
> + if (!spn)
> + return;
> +
> + index = SYMNAME_HASH_INDEX(spn->name);
> +
> + sp = st->mod_symname_hash[index];
> +
> + if (!sp ||
> + (spn->value < sp->value) ||
> + (spn->value == sp->value && spn < sp)) {
> + /*
> + * Install spn to the previous position of sp.
> + */
> + st->mod_symname_hash[index] = spn;
> + spn->name_hash_next = sp;
> + } else {
> + for (; sp; sp = sp->name_hash_next) {
> + if ((spn->value > sp->value) ||
> + (spn->value == sp->value && spn > sp)) {
> + if (!sp->name_hash_next ||
> + spn->value < sp->name_hash_next->value) {
> + /*
> + * Install spn to the next position of sp.
> + */
> + tmp = sp->name_hash_next;
> + sp->name_hash_next = spn;
> + spn->name_hash_next = tmp;
> + break;
> + }
> + }
> + }
> + }
> +}
> +
> +static void
> +mod_symname_hash_remove(struct syment *spn)
> +{
> + struct syment *sp;
> + int index;
> +
> + if (!spn)
> + return;
> +
> + index = SYMNAME_HASH_INDEX(spn->name);
> +
> + if (st->mod_symname_hash[index] == spn)
> + st->mod_symname_hash[index] = spn->name_hash_next;
I think now we can return here,
> +
> + for (sp = st->mod_symname_hash[index]; sp; sp = sp->name_hash_next) {
> + if (sp->name_hash_next == spn)
> + sp->name_hash_next = spn->name_hash_next;
and here, too.
For the other patches, please wait for a while.
Thanks,
Kazu
> + }
> +}
> +
> +static void
> +mod_symtable_hash_install_range(struct syment *from, struct syment *to)
> +{
> + struct syment *sp;
> +
> + for (sp = from; sp <= to; sp++)
> + mod_symname_hash_install(sp);
> +}
> +
> +static void
> +mod_symtable_hash_remove_range(struct syment *from, struct syment *to)
> +{
> + struct syment *sp;
> +
> + for (sp = from; sp <= to; sp++)
> + mod_symname_hash_remove(sp);
> +}
> +
> /*
> * Static kernel symbol value search
> */
> --
> 2.29.2
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility