[Crash-utility] [PATCH v5 1/6] Implement install and remove operations for mod_symname_hash

[HAGIO KAZUHITO(萩尾 一仁)]

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;
                }
        }

> 
> Signed-off-by: Tao Liu <ltao at 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