[PATCH v5] mm,kfence: decouple kfence from page granularity mapping judgement

[Zhenhua Huang]

Thanks Marco!

On 2023/3/13 17:49, Marco Elver wrote:
> On Mon, 13 Mar 2023 at 10:05, Zhenhua Huang <quic_zhenhuah at quicinc.com> wrote:
>>
>> Thanks Marco!
>>
>> On 2023/3/13 15:50, Marco Elver wrote:
>>> On Mon, 13 Mar 2023 at 06:04, Zhenhua Huang <quic_zhenhuah at quicinc.com> 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 at liebao ]# cat /proc/meminfo
>>>> MemTotal:         999484 kB
>>>> After:
>>>> [root at 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