Re: [Crash-utility] [PATCH v2] Improve the performance of symbol searching for kernel modules

-----Original Message----- > On Mon, Aug 23, 2021 at 08:24:28AM +0000, HAGIO KAZUHITO(萩尾 一仁) wrote: > > Hi Tao Liu, > > > > -----Original Message----- > > > Currently the sequence for crash searching a symbol is: 1) kernel symname > > > hash table, 2) iterate all kernel symbols, 3) iterate all kernel modules > > > and their symbols. In the worst case, if a non-exist symbol been searched, > > > all 3 stages will be went through. The time consuming status for each stage > > > is like: > > > > > > stage 1 stage 2 stage 3 > > > 0.007000(ms) 0.593000(ms) 2.421000(ms) > > > > Just to clarify, which parts of code are meant by the stage 1 and 2? > > > > Do you have a result with the patch? > > > > > > > > stage 3 takes too much time when comparing to stage 1. So let's introduce a > > > symname hash table for kernel modules to improve the performance of symbol > > > searching. > > > > I see that 3) is relatively time-consuming, but I have not had a delay > > due to the symbol search. Which command did you observe a delay? > > Hello Kazu, > > The target function is symbols.c:symbol_search. The code involved in the 3 stages are: > > 1) sp_hashed = symname_hash_search(s); > 2) for (sp = sp_hashed ? sp_hashed : st->symtable; sp < st->symend; sp++) { > if (STREQ(s, sp->name)) > return(sp); > 3) for (i = 0; i < st->mods_installed; i++) { > lm = &st->load_modules[i]; > .... > } OK, I see. (BTW, at a glance I'm not sure why the 2nd stage is needed... sp_hashed was already checked with STREQ in symname_hash_search if found, otherwise st->synmane_hash should include all kernel symbols..)

> > I measured each stage's time by adding the following code to symbol_search: > > struct load_module *lm; > int pseudos, search_init; > + double T1, T2, T3; > + clock_t start, end; > > + start = clock(); > sp_hashed = symname_hash_search(s); > + end = clock(); > + T1 = (double)(end - start) * 1000.0 / CLOCKS_PER_SEC; > > + start = clock(); > for (sp = sp_hashed ? sp_hashed : st->symtable; sp < st->symend; sp++) { > if (STREQ(s, sp->name)) > return(sp); > } > + end = clock(); > + T2 = (double)(end - start) * 1000.0 / CLOCKS_PER_SEC; > > pseudos = (strstr(s, "_MODULE_START_") || strstr(s, "_MODULE_END_")); > search_init = FALSE; > > + start = clock(); > for (i = 0; i < st->mods_installed; i++) { > lm = &st->load_modules[i]; > if (lm->mod_flags & MOD_INIT) > .... > return(sp); > } > } > + end = clock(); > + T3 = (double)(end - start) * 1000.0 / CLOCKS_PER_SEC; > + printf("%lf %lf %lf\n", T1, T2, T3); > > if (!search_init) > return((struct syment *)NULL); > > crash> sym blah > T1:0.008000 T2:0.562000 T3:2.435000 > symbol not found: blah > possible alternatives: > (none found) > > With the v2 patch applied and the time measurement code added: > > crash> sym blah > T1:0.003000 T2:0.545000 T3:0.017000 > symbol not found: blah > possible alternatives: > (none found) > > We can see T3 performs better. Thanks for the info, that's good. One more, do you have any slow command that will be especially improved by the patch? I'm ok with the sym command taking 3ms :-) I'd like to know why the patch should be applied or how much the worst command etc. will be improved, to see the importance of it.

Thanks, Kazu -- Crash-utility mailing list Crash-utility(a)redhat.com https://listman.redhat.com/mailman/listinfo/crash-utility