linux-kernel - Re: [GIT PULL] tracing: More updates for 6.13

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: <20241128155120.6e6cd300@rorschach.local.home>
Date: Thu, 28 Nov 2024 15:51:20 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Thomas Gleixner <tglx@...utronix.de>,
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Miguel Ojeda
 <ojeda@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
 rust-for-linux@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] tracing: More updates for 6.13

On Thu, 28 Nov 2024 11:55:34 -0800
Linus Torvalds <torvalds@...ux-foundation.org> wrote:


> I checked my resolution against yours, and I don't think your
> resolution is right.
> 
> You didn't check 'cond' on regular rust tracepoints, and you didn't do
> any locking on either kind.

Bah, you're right. The cond is only needed if they utilize it, which
I'm not sure they do, but the locking is definitely needed. My brain
was off when doing the merge conflict. I was thinking more of "what
makes this compile" than how it actually works.

> 
> I've pushed out my resolution, and hopefully rust people can actually
> test it. I might just be full of it.

Looks better than what I had. I'll kick my tests on it just as a sanity
check.

> 
> That said, I also think that the "__rust_do_trace_##name" inline
> helper should just be renamed to "__trace_##name", and then the
> regular trace_##name() helper could use that inside the
> static_branch_unlikely() check. Because that seems to be the only real
> thing the "rust" version wants - avoiding the static branch
> infrastructure in favor of whatever rust infrastructure.

Hmm, I think that could be done.

> 
> But hey, I do basic sanity build testing of the rust code, I don't
> actually _know_ it. Again, I might be missing something.

I need to spend some time understanding the rust code, so I can at least
review it better.

-- Steve