linux-kernel - Re: [PATCH v8 20/22] 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: <20250519070425.xDEPNyBe@linutronix.de>
Date: Mon, 19 May 2025 09:04:25 +0200
From: Nam Cao <namcao@...utronix.de>
To: Gabriele Monaco <gmonaco@...hat.com>
Cc: Steven Rostedt <rostedt@...dmis.org>,
	linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org,
	john.ogness@...utronix.de
Subject: Re: [PATCH v8 20/22] rv: Add rtapp_sleep monitor

On Fri, May 16, 2025 at 04:31:03PM +0000, Gabriele Monaco wrote:
> 2025-05-12T10:56:30Z Nam Cao <namcao@...utronix.de>:
> > diff --git a/kernel/trace/rv/monitors/sleep/Kconfig b/kernel/trace/rv/monitors/sleep/Kconfig
> > new file mode 100644
> > index 000000000000..d00aa1aae069
> > --- /dev/null
> > +++ b/kernel/trace/rv/monitors/sleep/Kconfig
> > @@ -0,0 +1,13 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +config RV_MON_SLEEP
> > +   depends on RV
> > +   select RV_LTL_MONITOR
> > +   depends on HAVE_SYSCALL_TRACEPOINTS
> > +   depends on RV_MON_RTAPP
> > +   select TRACE_IRQFLAGS
> 
> I had a different approach towards those (the preemptirq tracepoints)
> under the assumption adding them introduces latency. Besides me picking
> the wrong config (I used IRQSOFF, I'll fix that) I considered the monitor
> should /depend/ on the tracepoint instead of select it.
>
> This way it looks easier to me to avoid making a change that introduces
> latency slip in when distribution maintainers enable the monitor (e.g.
> TRACE_IRQFLAGS may be enabled on debug kernels and using depends would
> automatically prevent the monitor on non-debug kernels).
> 
> Now is this concern justified? Is it only a performance issue for the
> preempt tracepoint or not even there?  I'd like to keep consistency but I
> really can't decide on which approach is better.

Both approach is fine, I don't have a strong preference.

I doubt that the distribution people would carelessly enable anything new,
and these monitors are disabled by default. So I wouldn't worry too much.

I will do some measurements on the runtime impact of having these monitors
built, so that there will be a recommendation whether to enable them in
distribution kernel. But for now, just like any other debug configs, people
should expect some performance hit.

> Also curiosity on my side (I didn't try), you require TRACE_IRQFLAGS to
> use hardirq_context but how different is it from in_hardirq() in your
> case?

There is a wake_timersd() in __irq_exit_rcu(). This is a wakeup performed
within interrupt handling, but in_hardirq() doesn't say that.

Best regards,
Nam