Messages in this thread | | | Date | Tue, 28 Mar 2017 20:50:45 +0800 | From | zhouchengming <> | Subject | Re: [PATCH] reduce the time of finding symbols for module |
| |
On 2017/3/28 19:16, Miroslav Benes wrote: > On Tue, 28 Mar 2017, zhouchengming wrote: > >> On 2017/3/28 17:00, Miroslav Benes wrote: >>> >>> Hi, >>> >>> On Tue, 28 Mar 2017, Zhou Chengming wrote: >>> >>>> It's reported that the time of insmoding a klp.ko for one of our >>>> out-tree modules is too long. >>>> >>>> ~ time sudo insmod klp.ko >>>> real 0m23.799s >>>> user 0m0.036s >>>> sys 0m21.256s >>> >>> Is this stable through several (>=10) runs? 23 seconds are really >>> suspicious. Yes, there is a linear search through all the kallsyms in >>> kallsyms_on_each_symbol(), but there are something like 70k symbols on my >>> machine (that is, way less than 1M). 23 seconds are somewhat unexpected. >>> >> >> Yes, it's stable through several runs. >> >> I think the big reason is that our out-tree module used a lot of static local >> variables. We can see '.rela.kpatch.dynrelas' contains many entries, so it >> will >> waste a lot of time if we use kallsyms_on_each_symbol() to find these symbols >> of module. > > Ok, it means that you have a lot of relocation records which reference > your out-of-tree module. Then for each such entry klp_resolve_symbol() > is called and then klp_find_object_symbol() to actually resolve it. So if > you have 20k entries, you walk through vmlinux kallsyms table 20k times. > It is unneeded and that is why your fix works. > > But if there were 20k modules loaded, the problem would still be there. >
Yes, vmlinux kallsyms table is too big, but modules loaded are always few.
> I think it would be really nice to fix kallsyms :). Replace ordinary array > and the linear search with a hash table. > >> Relocation section '.rela.kpatch.funcs' at offset 0x382e0 contains 3 entries: >> Offset Info Type Sym. Value Sym. Name + >> Addend >> 000000000000 003300000101 R_AARCH64_ABS64 0000000000000000 value_show + 0 >> 000000000020 000b00000101 R_AARCH64_ABS64 0000000000000000 .kpatch.strings >> + 8 >> 000000000028 000b00000101 R_AARCH64_ABS64 0000000000000000 .kpatch.strings >> + 0 > > Hm, we do not have aarch64 support in upstream (yet). There is even no > dynamic ftrace with regs yet (if I am not mistaken). > >> Relocation section '.rela.kpatch.dynrelas' at offset 0x38328 contains 2562 >> entries: >> Offset Info Type Sym. Value Sym. Name + >> Addend >> 000000000000 003300000101 R_AARCH64_ABS64 0000000000000000 value_show + 14 >> 000000000018 000b00000101 R_AARCH64_ABS64 0000000000000000 .kpatch.strings >> + 13 >> 000000000020 000b00000101 R_AARCH64_ABS64 0000000000000000 .kpatch.strings >> + 0 >> 000000000040 003300000101 R_AARCH64_ABS64 0000000000000000 value_show + 20 >> 000000000058 000b00000101 R_AARCH64_ABS64 0000000000000000 .kpatch.strings >> + 13 >> 000000000060 000b00000101 R_AARCH64_ABS64 0000000000000000 .kpatch.strings >> + 0 >> >>> If it is a problem, can we fix kallsyms_on_each_symbol() and replace the >>> linear search with something better? All users would benefit... >>> >> >> Yes, it's better if we can improve the linear search, but I can't think of >> that... > > I don't understand. Fixing kallsyms is of course much more work but > everyone would benefit from that. > > If there is an agreement, we could accept your solution as temporary. In > such case, please prefix the subject with 'livepatch: ' and use capital > letter in 'Reduce'. Please also improve the changelog and describe where > the problem really is. >
Ok, if there are no oppositions, I will send a patch-v2 with improved changelog. This is really a temporary solution, and others can go on to fix the kallsyms if needed later.
Thanks.
> Thanks, > Miroslav > > . >
| |