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

[HAGIO KAZUHITO(萩尾 一仁)]

-----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 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
> 
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://listman.redhat.com/mailman/listinfo/crash-utility