| Message ID | 20220329222514.51af6c07@gandalf.local.home (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tracing: Set user_events to BROKEN | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
----- On Mar 29, 2022, at 10:25 PM, rostedt rostedt@goodmis.org wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > After being merged, user_events become more visible to a wider audience > that have concerns with the current API. It is too late to fix this for > this release, but instead of a full revert, just mark it as BROKEN (which > prevents it from being selected in make config). Then we can work finding > a better API. If that fails, then it will need to be completely reverted. Hi Steven, What are the constraints for changing a uapi header after it has been present in a kernel release ? If we are not ready to commit to an ABI, perhaps it would be safer to ensure that include/uapi/linux/user_events.h is not installed with the uapi headers until it's ready. Thanks, Mathieu > > Link: > https://lore.kernel.org/all/2059213643.196683.1648499088753.JavaMail.zimbra@efficios.com/ > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > kernel/trace/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index 16a52a71732d..edc8159f63ef 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -741,6 +741,7 @@ config USER_EVENTS > bool "User trace events" > select TRACING > select DYNAMIC_EVENTS > + depends on BROKEN # API needs to be straighten out > help > User trace events are user-defined trace events that > can be used like an existing kernel trace event. User trace > -- > 2.35.1
On Wed, 30 Mar 2022 12:54:13 -0400 (EDT) Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: > ----- On Mar 29, 2022, at 10:25 PM, rostedt rostedt@goodmis.org wrote: > > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > > > After being merged, user_events become more visible to a wider audience > > that have concerns with the current API. It is too late to fix this for > > this release, but instead of a full revert, just mark it as BROKEN (which > > prevents it from being selected in make config). Then we can work finding > > a better API. If that fails, then it will need to be completely reverted. > > Hi Steven, > > What are the constraints for changing a uapi header after it has been present > in a kernel release ? > > If we are not ready to commit to an ABI, perhaps it would be safer to ensure > that include/uapi/linux/user_events.h is not installed with the uapi headers > until it's ready. > Linus may say otherwise, but from what I understand is that we can not break a user space application from one release to the next. That means, the only way to break something is if it is actually using something in binary form. I can not think of a situation where a header file is useful if the API it's used for is not available. Thus do we really need to hide it? What applications will use a header file that has no interface for it? I do not see the need to remove the uapi if the API for that structure is not available yet. -- Steve
On Wed, Mar 30, 2022 at 9:54 AM Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: > > If we are not ready to commit to an ABI, perhaps it would be safer to ensure > that include/uapi/linux/user_events.h is not installed with the uapi headers > until it's ready. I don't th8ink the uapi matters if the code then cannot be used. There's no regression in that. That said, if we leave the code in the kernel source tree, I feel like we should probably at least compile-test it. So maybe it should be marked as depends on BROKEN || COMPILE_TEST instead? Linus
On Wed, 30 Mar 2022 12:28:11 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > So maybe it should be marked as > > depends on BROKEN || COMPILE_TEST > > instead? Agreed. I'll send a v2 of the patch. Thanks, -- Steve
On Wed, 30 Mar 2022 15:31:45 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > So maybe it should be marked as > > > > depends on BROKEN || COMPILE_TEST > > > > instead? > > Agreed. I'll send a v2 of the patch. Hopefully no distros are idiotic enough to enable COMPILE_TEST to get this feature. I could add "panic" to each of the API calls ;-) -- Steve
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index 16a52a71732d..edc8159f63ef 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -741,6 +741,7 @@ config USER_EVENTS bool "User trace events" select TRACING select DYNAMIC_EVENTS + depends on BROKEN # API needs to be straighten out help User trace events are user-defined trace events that can be used like an existing kernel trace event. User trace