Thanks Marco!
On 2023/3/13 17:49, Marco Elver wrote:
On Mon, 13 Mar 2023 at 10:05, Zhenhua Huang <quic_zhenhuah@xxxxxxxxxxx> wrote:
Thanks Marco!
On 2023/3/13 15:50, Marco Elver wrote:
On Mon, 13 Mar 2023 at 06:04, Zhenhua Huang <quic_zhenhuah@xxxxxxxxxxx> wrote:
Kfence only needs its pool to be mapped as page granularity, previous
judgement was a bit over protected. From [1], Mark suggested to "just
map the KFENCE region a page granularity". So I decouple it from judgement
and do page granularity mapping for kfence pool only.
Page granularity mapping in theory cost more(2M per 1GB) memory on arm64
platform. Like what I've tested on QEMU(emulated 1GB RAM) with
gki_defconfig, also turning off rodata protection:
Before:
[root@liebao ]# cat /proc/meminfo
MemTotal: 999484 kB
After:
[root@liebao ]# cat /proc/meminfo
MemTotal: 1001480 kB
To implement this, also relocate the kfence pool allocation before the
linear mapping setting up, arm64_kfence_alloc_pool is to allocate phys
addr, __kfence_pool is to be set after linear mapping set up.
This patch still breaks the late-init capabilities that Kefeng pointed out.
I think the only viable option is:
1. If KFENCE early init is requested on arm64, do what you're doing here.
2. If KFENCE is compiled in, but not enabled, do what was done
before, so it can be enabled late.
I'm fine with above solution as well. The Disadvantage is if we want to
dynamically disable kfence through kfence_sample_interval, it must be
mapped into page granularity still.
Am I missing an option?
Another option is what Kefeng firstly thought and I had proposed on
comments of patchsetV3, actually I wanted to do in an separate patch:
Please do it in the same patch (or patch series), otherwise we end up
with a regression.
OK.
"
So how about we raise another change, like you mentioned bootargs
indicating to use late init of b33f778bba5e ("kfence: alloc kfence_pool
Please avoid introducing another bootarg just for this. It will
confuse users and will cause serious annoyance (bad UX).
OK, got it.
after system startup").
1. in arm64_kfence_alloc_pool():
if (!kfence_sample_interval && !using_late_init)
return 0;
else
allocate pool
The whole point of late allocation was that the entire pool is _not_
allocated until it's needed (during late init). So for space-conscious
users, this option is actually worse.
2. also do the check in late allocation,like
if (do_allocation_late && !using_late_init)
BUG();
BUG() needs to be avoided. Just because a user used the system wrong,
should not cause it to crash (WARN instead)... but I'd really prefer
you avoid introducing another boot arg, because it'll lead to bad UX.
"
The thought is to allocate pool early as well if we need to
using_late_init.
Kefeng, Marco,
How's your idea?
I recommend that you just make can_set_direct_map() conditional on
KFENCE being initialized early or not. With rodata protection most
arm64 kernels likely pay the page granular direct map cost anyway. And
for your special usecase where you want to optimize memory use, but
know that KFENCE is enabled, it'll result in the savings you desire.
Thanks Marco, got your idea. Yeah.. rodata is another over-protection
case. I will do the change following your suggestion for your review.
Thanks,
-- Marco
