linux-kernel - Re: [PATCH v5 21/23] rv: Add rtapp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bfe8322cf5817037af57f10ffbffcd9b30f43b42.camel@redhat.com>
Date: Tue, 29 Apr 2025 18:01:01 +0200
From: Gabriele Monaco <gmonaco@...hat.com>
To: Nam Cao <namcao@...utronix.de>, Steven Rostedt <rostedt@...dmis.org>, 
	linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: john.ogness@...utronix.de, Peter Zijlstra <peterz@...radead.org>, Ingo
 Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>, Boqun Feng
 <boqun.feng@...il.com>,  Waiman Long <longman@...hat.com>
Subject: Re: [PATCH v5 21/23] rv: Add rtapp_sleep monitor

On Tue, 2025-04-29 at 14:01 +0200, Nam Cao wrote:
> Add a monitor for checking that real-time tasks do not go to sleep in
> a
> manner that may cause undesirable latency.
> 
> Also change
> 	RV depends on TRACING
> to
> 	RV select TRACING
> to avoid the following recursive dependency:
> 
>  error: recursive dependency detected!
> 	symbol TRACING is selected by PREEMPTIRQ_TRACEPOINTS
> 	symbol PREEMPTIRQ_TRACEPOINTS depends on TRACE_IRQFLAGS
> 	symbol TRACE_IRQFLAGS is selected by RV_MON_SLEEP
> 	symbol RV_MON_SLEEP depends on RV
> 	symbol RV depends on TRACING
> 
> Reviewed-by: Gabriele Monaco <gmonaco@...hat.com>
> Signed-off-by: Nam Cao <namcao@...utronix.de>
> ---
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Will Deacon <will@...nel.org>
> Cc: Boqun Feng <boqun.feng@...il.com>
> Cc: Waiman Long <longman@...hat.com>
> ---
> 
> [...]
> 
> +RULE = always ((RT and SLEEP) imply (RT_FRIENDLY_SLEEP or
> ALLOWLIST))
> +
> +RT_FRIENDLY_SLEEP = (RT_VALID_SLEEP_REASON or KERNEL_THREAD)
> +                and ((not WAKE) until RT_FRIENDLY_WAKE)
> +
> +RT_VALID_SLEEP_REASON = PI_FUTEX
> +                     or RT_FRIENDLY_NANOSLEEP
> +
> +RT_FRIENDLY_NANOSLEEP = CLOCK_NANOSLEEP
> +                    and NANOSLEEP_TIMER_ABSTIME
> +                    and NANOSLEEP_CLOCK_MONOTONIC
> +
> +RT_FRIENDLY_WAKE = WOKEN_BY_EQUAL_OR_HIGHER_PRIO
> +                or WOKEN_BY_HARDIRQ
> +                or WOKEN_BY_NMI
> +                or KTHREAD_SHOULD_STOP
> +
> +ALLOWLIST = BLOCK_ON_RT_MUTEX
> +         or TASK_IS_RCU
> +         or TASK_IS_MIGRATION

So, just thinking out loud, PI_FUTEX is a valid sleep reason,
technically also BLOCK_ON_RT_MUTEX is something you are allowing.

In my understanding, the contention tracepoints already in the kernel
can track all contention by kernel code and are leaving aside the PI
futexes, which use the untracked rt_mutex_wait_proxy_lock.

In your case, you are tracking PI_FUTEX via the system call, which
should cover the above scenario.
Do you really need extra tracepoints to track this too? Or is there any
other use of start_proxy_lock/wait_proxy_lock I'm missing here?

I see the only case in which rt_mutex_start_proxy_lock is called with a
task different than current is via FUTEX_CMP_REQUEUE_PI, wouldn't
considering this one too make the new tracepoints superfluous (assuming
this one is even needed to be tracked before FUTEX_WAIT_REQUEUE_PI).

Thanks,
Gabriele