Re: [GIT PULL] KVM/rseq changes for Linux 5.15-rc3

From: Paolo Bonzini
Date: Thu Sep 23 2021 - 14:44:26 EST


On 23/09/21 20:35, Linus Torvalds wrote:
But I'm adding a few people to the cc for a completely different
reason: the cleanup to move all the notify_resume stuff to
tracehook_notify_resume() is good, but it does make me go - once again
- "Hmm, that naming is really really bad".

Absolutely, it's even in the commit message:

---
Note, tracehook_notify_resume() is horribly named and arguably does not
belong in tracehook.h as literally every line of code in it has nothing
to do with tracing. But, that's been true since commit a42c6ded827d
("move key_repace_session_keyring() into tracehook_notify_resume()")
first usurped tracehook_notify_resume() back in 2012. Punt cleaning that
mess up to future patches.
---

As you point out, it's really all of the header and not just that one
function.

Paolo

The <linux/tracehook.h> code was literally meant for tracing. It's
where the name comes from, and it's the original intent: having a
place that you can hook into for tracing that doesn't depend on how
the core kernel code ends up changing.

But that's not how it actually acts right now. That header file is now
some very core functionality, and little of it is actually related to
tracing any more. It's more core process state handling for the user
space return path.

So I don't object to the patches, and they are merged, but I'm cc'ing people to

(a) let them know about this (see commit a68de80f61f6: "entry: rseq:
Call rseq_handle_notify_resume() in tracehook_notify_resume()" in the
current -git tree)

(b) possibly prod some people into perhaps moving/renaming some of
that code to actual core kernel C files, instead of a misnamed header
file..

Hmm?

Linus