[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170329000333.zmafvp4uk34m3em6@jeyu>
Date: Tue, 28 Mar 2017 17:03:33 -0700
From: Jessica Yu <jeyu@...hat.com>
To: Miroslav Benes <mbenes@...e.cz>
Cc: zhouchengming <zhouchengming1@...wei.com>,
live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
jpoimboe@...hat.com, jikos@...nel.org, pmladek@...e.com,
huawei.libin@...wei.com
Subject: Re: [PATCH] reduce the time of finding symbols for module
+++ Miroslav Benes [28/03/17 13:16 +0200]:
>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.
>
>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).
I'm curious, how was this tested? Since there is no dynamic ftrace
with regs and no livepatch stubs (klp_arch_set_pc, etc) implemented
yet for aarch64. Also, livepatch has switched from klp_relocs/dynrelas
to .klp.rela. sections since 4.7, so I'm curious how your patch module
has a .kpatch.dynrelas section working with livepatch.
Unrelated to this patch, if there is a working aarch64 livepatch port (and
kpatch build tool, it seems) floating out there, it would be
wonderful to push that upstream :-)
Jessica
Powered by blists - more mailing lists