Message-ID: <20250722122538.6ce25ca2@batman.local.home>
Date: Tue, 22 Jul 2025 12:25:38 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 bpf@...r.kernel.org, x86@...nel.org, Masami Hiramatsu
 <mhiramat@...nel.org>, Josh Poimboeuf <jpoimboe@...nel.org>, Peter Zijlstra
 <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, Jiri Olsa
 <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>, Thomas Gleixner
 <tglx@...utronix.de>, Andrii Nakryiko <andrii@...nel.org>, Indu Bhagat
 <indu.bhagat@...cle.com>, "Jose E. Marchesi" <jemarch@....org>, Beau
 Belgrave <beaub@...ux.microsoft.com>, Jens Remus <jremus@...ux.ibm.com>,
 Linus Torvalds <torvalds@...ux-foundation.org>, Andrew Morton
 <akpm@...ux-foundation.org>, Jens Axboe <axboe@...nel.dk>, Florian Weimer
 <fweimer@...hat.com>, Sam James <sam@...too.org>, Brian Robbins
 <brianrob@...rosoft.com>, Elena Zannoni <elena.zannoni@...cle.com>
Subject: Re: [RFC] New codectl(2) system call for sframe registration

On Tue, 22 Jul 2025 09:51:22 -0400
Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:

> > Here's a hypothetical, what if for some reason (say having the sframe
> > sections outside of the elf file) that the linker shares that?  
> 
> So your hypothetical scenario is having sframe provided as a separate
> file. This sframe file (or part of it) would still describe how to
> unwind a given elf file text range. So I would argue that this would

No. It should describe how to get access to an sframe section for some
text that has already been loaded in memory.

I'm looking for a mapping between already loaded text memory to how to
unwind it that will be in an sframe format somewhere on disk.

> still fit into the model of CODE_REGISTER_ELF, it's just that the
> address range from sframe_start to sframe_end would be mapped from
> a different file. This is entirely up to the dynamic loader and should
> not impact the kernel ABI.
> 
> AFAIK the a.out binary support was deprecated in Linux kernel v5.1. So
> being elf specific is not an issue.

Yes, but we are not registering ELF. We are registering how to unwind
something with sframes. If it's not sframes we are registering, what is
it?

> 
> And if for some reason we end up inventing a new model to hand over the
> sframe information in the future, for instance if we choose not to map
> the sframe information in userspace and hand over a sframe-file pathname
> and offset instead, we'll just extend the code_opt enum with a new
> label.

This is not a new model. We could likely do it today without much
effort. We are handing over sframe data regardless if it's in an ELF
file or not.

The systemcall is to let the dynamic linker know where the kernel can
find the sframes for newly loaded text.

> 
> > 
> > For instance, if the sframe sections are downloaded separately as a
> > separate package for a given executable (to make it not mandatory for an
> > install), the linker could be smart enough to see that they exist in some
> > special location and then pass that to the kernel. In other words, this is
> > option is specific for sframe and not ELF. I rather call it by that.  
> 
> As I explained above, if the dynamic loader populates the sframe section
> in userspace memory, this fits within the CODE_REGISTER_ELF ABI. If we

But this isn't about ELF! It's about sframes! Why not name it that?

> eventually choose not to map the sframe section into userspace memory
> (even though this is not an envisioned use-case at the moment), we can
> just extend enum code_opt with a new label.

Why call this at all if you don't plan on mapping sframes?

> 
> >   
> >>
> >> If there are other file types in the future that happen to contain an
> >> sframe section (but are not ELF), then we can simply add a new label to
> >> enum code_opt.
> >>  
> >>>      
> >>>>
> >>>> sys_codectl(2)
> >>>> =================
> >>>>
> >>>> * arg0: unsigned int @option:
> >>>>
> >>>> /* Additional labels can be added to enum code_opt, for extensibility. */
> >>>>
> >>>> enum code_opt {
> >>>>        CODE_REGISTER_ELF,  
> >>>
> >>> Perhaps the above should be: CODE_REGISTER_SFRAME,
> >>>
> >>> as currently SFrame is read only via files.  
> >>
> >> As I pointed out above, on GNU/Linux, sframe is always an allocated,loaded
> >> ELF section. AFAIU, your comment implies that we'd want to support other scenarios
> >> where the sframe is in files outside of elf binary sframe sections. Can you
> >> expand on the use-case you have for this, or is it just for future-proofing ?  
> > 
> > Heh, I just did above (before reading this). But yeah, it could be. As I
> > mentioned above, this is not about ELF files. Sframes just happen to be in
> > an ELF file. CODE_REGISTER_ELF sounds like this is for doing special
> > actions to an ELF file, when in reality it is doing special actions to tell
> > the kernel this is an sframe table. It just happens that sframes are in
> > ELF. Let's call it for what it is used for.  
> 
> I see sframe as one "aspect" of an ELF file. Sure, we could do one
> system call for every aspect of an ELF file that we want to register,
> but that would require many round trips from userspace to the kernel
> every time a library is loaded. In my opinion it makes sense to combine
> all aspects of an elf file that we want the kernel to know about into
> one registration system call. In that sense, we're not registering just
> sframe, but the various aspects of an ELF file, which include sframe.

So you are making this a generic ELF function?  What other functions do
you plan to do with this system call?

> 
> By the way, the sframe section is optional as well. If we allow
> sframe_start and sframe_end to be NULL, this would let libc register
> an sframe-less ELF file with its pathname, build-id, and debug info
> to the kernel. This would be immediately useful on its own for
> distributions that have frame pointers enabled even without sframe
> section.

The above is called mission creep. Looks to me that you are using this
as a way to have LTTng get easier access to build ids and such. We can
add *that* later if needed, as a separate option. This has nothing to
do with the current requirements.


> >>>
> >>> And call it "struct code_sframe_info"
> >>>      
> >>>>        __u64 text_start;
> >>>>        __u64 text_end;  
> >>>      
> >>>>        __u64 sframe_start;
> >>>>        __u64 sframe_end;  
> >>>
> >>> What is the above "sframe" for?  
> > 
> > Still wondering what the above is for.  
> 
> Well we have an sframe section which is mapped into userspace memory
> from sframe_start to sframe_end, which contains the unwind information
> that covers the code from text_start to text_end.

Actually, the sframe section shouldn't be mapped into user space
memory. The kernel will be doing that, not the linker. I would say that
the system call can give a hint of where it would like it mapped, but
it should allow the kernel to decide where to map it as the user space
code doesn't care where it gets mapped.

In the future, if we wants to compress the sframe section, it will not
even be a loadable ELF section. But the system call can tell the
kernel: "there's a sframe compressed section at this offset/size in
this file" for this text address range and then the kernel will do the
rest.

> 
> Am I unknowingly adding some kind of redundancy here ?
> 

Maybe. This systemcall was to add unwinding information for the kernel.
It looks like you are having it be much more than that. I'm not against
that, but that should only be for extensions, and currently, this is
supposed to only make sframes work.

-- Steve

Powered by blists - more mailing lists