Re: [PATCH RFC 0/4] static key support for error injection functions

[Vlastimil Babka]

On 6/2/24 10:47 PM, David Rientjes wrote:
> On Fri, 31 May 2024, Vlastimil Babka wrote:
> 
>> Patches 3 and 4 implement the static keys for the two mm fault injection
>> sites in slab and page allocators. For a quick demonstration I've run a
>> VM and the simple test from [1] that stresses the slab allocator and got
>> this time before the series:
>> 
>> real    0m8.349s
>> user    0m0.694s
>> sys     0m7.648s
>> 
>> with perf showing
>> 
>>    0.61%  nonexistent  [kernel.kallsyms]  [k] should_failslab.constprop.0
>>    0.00%  nonexistent  [kernel.kallsyms]  [k] should_fail_alloc_page                                                                                                                                                                                        ▒
>> 
>> And after the series
>> 
>> real    0m7.924s
>> user    0m0.727s
>> sys     0m7.191s
>> 
>> and the functions gone from perf report.
>> 
> 
> Impressive results that will no doubt be a win for kernels that enable 
> these options.

Oh, I should have been more clear about it. This was done with both of these
options disabled. It's measuring just removing the overhead of calling the
empty noninline function, otherwise the difference would be larger. CPU
mitigations (defaults) were enabled though, which affects the cost of
function calls (this was in KVM guest on Ryzen 2700).

> Both CONFIG_FAILSLAB and CONFIG_FAIL_PAGE_ALLOC go out of their way to 
> have no overhead, both in performance and kernel text overhead, when the 
> .config options are disabled.

Except the unavoidable function call overhead since commits 4f6923fbb352 and
af3b854492f3.

> Do we have any insight into the distros or users that enable either of 
> these options and are expecting optimal performance?  I would have assumed 
> that while CONFIG_FAULT_INJECTION may be enabled that any users who would 
> care deeply about this would have disabled both of these debug options.

Eliminating the empty function call overhead, which is currently not
possible to configure out in any way, was my primary goal. For our distro we
disable the options and they are enabled only in a debug kernel option. So
the additional benefit of the static key is we could enable them with no
cost, and have them available for when needed, without the need to change
kernel. This is great for debugging functionality in general
(debug_pagealloc, page_owner), maybe this would be less likely to be useful,
but one never knows.

>> There might be other such fault injection callsites in hotpaths of other
>> subsystems but I didn't search for them at this point.
>> 
>> [1] https://lore.kernel.org/all/6d5bb852-8703-4abf-a52b-90816bccbd7f@xxxxxxx/
>> [2] https://lore.kernel.org/all/3j5d3p22ssv7xoaghzraa7crcfih3h2qqjlhmjppbp6f42pg2t@kg7qoicog5ye/
>> 
>> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
>> ---
>> Vlastimil Babka (4):
>>       fault-inject: add support for static keys around fault injection sites
>>       error-injection: support static keys around injectable functions
>>       mm, slab: add static key for should_failslab()
>>       mm, page_alloc: add static key for should_fail_alloc_page()
>> 
>>  include/asm-generic/error-injection.h | 13 ++++++++++-
>>  include/asm-generic/vmlinux.lds.h     |  2 +-
>>  include/linux/error-injection.h       |  9 +++++---
>>  include/linux/fault-inject.h          |  7 +++++-
>>  kernel/fail_function.c                | 22 +++++++++++++++---
>>  lib/error-inject.c                    |  6 ++++-
>>  lib/fault-inject.c                    | 43 ++++++++++++++++++++++++++++++++++-
>>  mm/fail_page_alloc.c                  |  3 ++-
>>  mm/failslab.c                         |  2 +-
>>  mm/internal.h                         |  2 ++
>>  mm/page_alloc.c                       | 11 ++++++---
>>  mm/slab.h                             |  3 +++
>>  mm/slub.c                             | 10 +++++---
>>  13 files changed, 114 insertions(+), 19 deletions(-)
>> ---
>> base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
>> change-id: 20240530-fault-injection-statickeys-66b7222e91b7
>> 
>> Best regards,
>> -- 
>> Vlastimil Babka <vbabka@xxxxxxx>
>> 
>>