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

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

--- Patch [5]: [PATCH 5/5] x86/paravirt: Trace contended_release on unlock =
---
commit 1a2ad2e72bd14437a21c720ae7476112bc7cb8d5
Author: Dmitry Ilvokhin <[email protected]>

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/parav=
irt-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 =3D static_call_query(queued_spin_unlock);
> +
> +	if (cur =3D=3D __raw_callee_save___native_queued_spin_unlock)
> +		static_call_update(queued_spin_unlock,
> +				   __raw_callee_save_native_queued_spin_unlock_traced);
> +	else if (cur =3D=3D __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=3Dn?

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

> +
> +void arch_contended_release_trace_unreg(void)
> +{
> +	void *cur =3D static_call_query(queued_spin_unlock);
> +
> +	if (cur =3D=3D __raw_callee_save_native_queued_spin_unlock_traced)
> +		static_call_update(queued_spin_unlock,
> +				   __raw_callee_save___native_queued_spin_unlock);
> +	else if (cur =3D=3D __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 tracepoi=
nt
is disabled globally, even if spinlock tracing was intentionally excluded
during the build.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/cover.1785778551.gi=
[email protected]?part=3D5