Re: [PATCH 5/5] x86/paravirt: Trace contended_release on unlock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Paravirt queued spinlocks unconditionally trace 'contended_release' when the tracepoint is enabled globally, ignoring the CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE opt-out.
--

--- Patch [5]: [PATCH 5/5] x86/paravirt: Trace contended_release on unlock ---
commit 1a2ad2e72bd14437a21c720ae7476112bc7cb8d5
Author: Dmitry Ilvokhin <d@ilvokhin.com>

x86/paravirt: Trace contended_release on unlock

This commit introduces traced variants of paravirt spinlock unlock functions
to trace the contended_release event. It dynamically swaps the static call
target to point to these traced versions when the tracepoint is enabled,
keeping the disabled path zero-cost.

> diff --git a/arch/x86/kernel/paravirt-spinlocks.c b/arch/x86/kernel/paravirt-spinlocks.c
> index ddc19dc28ba16..ca12b36553074 100644
> --- a/arch/x86/kernel/paravirt-spinlocks.c
> +++ b/arch/x86/kernel/paravirt-spinlocks.c
[ ... ]
> +int arch_contended_release_trace_reg(void)
> +{
> +	void *cur = static_call_query(queued_spin_unlock);
> +
> +	if (cur == __raw_callee_save___native_queued_spin_unlock)
> +		static_call_update(queued_spin_unlock,
> +				   __raw_callee_save_native_queued_spin_unlock_traced);
> +	else if (cur == __raw_callee_save___pv_queued_spin_unlock)
> +		static_call_update(queued_spin_unlock,
> +				   __raw_callee_save_pv_queued_spin_unlock_traced);
> +	return 0;
> +}

[Severity: Medium]
Will arch_contended_release_trace_reg() unconditionally trace the paravirt
queued spinlocks when the tracepoint is enabled globally, even if the kernel
was built with CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE=n?

It looks like this registration callback swaps the static calls to the traced
variants without checking if spinlock tracing was explicitly opted out in the
kernel configuration.

> +
> +void arch_contended_release_trace_unreg(void)
> +{
> +	void *cur = static_call_query(queued_spin_unlock);
> +
> +	if (cur == __raw_callee_save_native_queued_spin_unlock_traced)
> +		static_call_update(queued_spin_unlock,
> +				   __raw_callee_save___native_queued_spin_unlock);
> +	else if (cur == __raw_callee_save_pv_queued_spin_unlock_traced)
> +		static_call_update(queued_spin_unlock,
> +				   __raw_callee_save___pv_queued_spin_unlock);
>  }

[Severity: Medium]
Similarly, does arch_contended_release_trace_unreg() also need to check the
CONFIG_QUEUED_SPINLOCKS_TRACE_CONTENDED_RELEASE configuration?

Without a config guard, this manipulates the static calls when the tracepoint
is disabled globally, even if spinlock tracing was intentionally excluded
during the build.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785778551.git.d@ilvokhin.com?part=5