Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

static key support for error injection functions #7231

Closed
wants to merge 7 commits into from
Closed

static key support for error injection functions #7231

wants to merge 7 commits into from

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: static key support for error injection functions
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f6afdaf
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: bf977ee
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 1ae7a19
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 717d631
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 6ddf3a9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: cc5083d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 2807db7
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 2bb138c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: cd387ce
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 93265a0
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 04efaeb
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 5a53245
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: c73a968
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@tehcaster
fault-inject: add support for static keys around fault injection sites
a590219 
Some fault injection sites are placed in hotpaths and incur overhead
even if not enabled, due to one or more function calls leading up to
should_fail_ex() that returns false due to attr->probability == 0.

This overhead can be eliminated if the outermost call into the checks is
guarded with a static key, so add support for that. The framework should
be told that such static key exist for a fault_attr, by initializing
fault_attr->active with the static key address. When it's not NULL,
enable the static key from setup_fault_attr() when the fault probability
is non-zero.

Also wire up writing into debugfs "probability" file to enable or
disable the static key when transitioning between zero and non-zero
probability.

For now, do not add configfs interface support as the immediate plan is
to leverage this for should_failslab() and should_fail_alloc_page()
after other necessary preparatory changes, and not for any of the
configfs based fault injection users.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
@tehcaster
error-injection: support static keys around injectable functions
81f7e16 
Error injectable functions cannot be inlined and since some are called
from hot paths, this incurs overhead even if no error injection is
enabled for them.

To avoid this overhead when disabled, allow the callsites of error
injectable functions to put the calls behind a static key, which the
framework can control when error injection is enabled or disabled for
the function.

Introduce a new ALLOW_ERROR_INJECTION_KEY() macro that adds a parameter
with the static key's address, and store it in struct
error_injection_entry. This new field has caused a mismatch when
populating the injection list from the _error_injection_whitelist
section using the current STRUCT_ALIGN(), so change the alignment to 8.

During the population, copy the key's address also to struct ei_entry,
and make it possible to retrieve it by get_injection_key().

Finally, make the processing of writes to the debugfs inject file enable
the static key when the function is added to the injection list, and
disable when removed.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
@tehcaster
bpf: support error injection static keys for perf_event attached progs
148bfe9 
Functions marked for error injection can have an associated static key
that guards the callsite(s) to avoid overhead of calling an empty
function when no error injection is in progress.

Outside of the error injection framework itself, bpf programs can be
atteched to perf events and override results of error-injectable
functions. To make sure these functions are actually called, attaching
such bpf programs should control the static key accordingly.

Therefore, add the static key's address to struct trace_kprobe and fill
it in trace_kprobe_error_injectable(), using get_injection_key() instead
of within_error_injection_list(). Introduce
trace_kprobe_error_injection_control() to control the static key and
call the control function when attaching or detaching programs with
kprobe_override to perf events.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: d65f376
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=863626
version: 2

@tehcaster
bpf: support error injection static keys for multi_link attached progs
2dad738 
Functions marked for error injection can have an associated static key
that guards the callsite(s) to avoid overhead of calling an empty
function when no error injection is in progress.

Outside of the error injection framework itself, bpf programs can be
atteched to kprobes and override results of error-injectable functions.
To make sure these functions are actually called, attaching such bpf
programs should control the static key accordingly.

Therefore, add an array of static keys to struct bpf_kprobe_multi_link
and fill it in addrs_check_error_injection_list() for programs with
kprobe_override enabled, using get_injection_key() instead of
within_error_injection_list(). Introduce bpf_kprobe_ei_keys_control() to
control the static keys and call the control function when doing
multi_link_attach and release.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
@tehcaster
bpf: do not create bpf_non_sleepable_error_inject list when unnecessary
40e044a 
When CONFIG_FUNCTION_ERROR_INJECTION is disabled,
within_error_injection_list() will return false for any address and the
result of check_non_sleepable_error_inject() denylist is thus redundant.
The bpf_non_sleepable_error_inject list thus does not need to be
constructed at all, so #ifdef it out.

This will allow to inline functions on the list when
CONFIG_FUNCTION_ERROR_INJECTION is disabled as there will be no BTF_ID()
reference for them.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
@tehcaster
mm, slab: add static key for should_failslab()
b357ef2 
Since commit 4f6923f ("mm: make should_failslab always available for
fault injection") should_failslab() is unconditionally a noinline
function. This adds visible overhead to the slab allocation hotpath,
even if the function is empty. With CONFIG_FAILSLAB=y there's additional
overhead, even when the functionality is not activated by a boot
parameter or via debugfs.

The overhead can be eliminated with a static key around the callsite.
Fault injection and error injection frameworks including bpf can now be
told that this function has a static key associated, and are able to
enable and disable it accordingly.

Additionally, compile out all relevant code if neither CONFIG_FAILSLAB
nor CONFIG_FUNCTION_ERROR_INJECTION is enabled. When only the latter is
not enabled, make should_failslab() static inline instead of noinline.

To demonstrate the reduced overhead of calling an empty
should_failslab() function, a kernel build with
CONFIG_FUNCTION_ERROR_INJECTION enabled but CONFIG_FAILSLAB disabled,
and CPU mitigations enabled, was used in a qemu-kvm (virtme-ng) on AMD
Ryzen 7 2700 machine, and execution of a program trying to open() a
non-existent file was measured 3 times:

    for (int i = 0; i < 10000000; i++) {
        open("non_existent", O_RDONLY);
    }

After this patch, the measured real time was 4.3% smaller. Using perf
profiling it was verified that should_failslab was gone from the
profile.

With CONFIG_FAILSLAB also enabled, the patched kernel performace was
unaffected, as expected, while unpatched kernel's performance was worse,
resulting in the relative speedup being 10.5%. This means it no longer
needs to be an option suitable only for debug kernel builds.

Acked-by: Alexei Starovoitov <ast@kernel.org>
Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
@tehcaster
mm, page_alloc: add static key for should_fail_alloc_page()
b6236b3 
Similarly to should_failslab(), remove the overhead of calling the
noinline function should_fail_alloc_page() with a static key that guards
the callsite in the page allocator hotpath, and is controlled by the
fault and error injection frameworks and bpf.

Additionally, compile out all relevant code if neither
CONFIG_FAIL_ALLOC_PAGE nor CONFIG_FUNCTION_ERROR_INJECTION is enabled.
When only the latter is not enabled, make should_fail_alloc_page()
static inline instead of noinline.

No measurement was done other than verifying the should_fail_alloc_page
is gone from the perf profile. A measurement with the analogical change
for should_failslab() suggests that for a page allocator intensive
workload there might be noticeable improvement. It also makes
CONFIG_FAIL_ALLOC_PAGE an option suitable not only for debug kernels.

Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=863626 irrelevant now. Closing PR.

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot deleted the series/863626=>bpf-next branch June 26, 2024 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bpf-next handled-elsewhere new V2-ci-pass V2
Projects
None yet
Milestone
No milestone
1 participant
@tehcaster