Re: git: ddf0ed09bd8f - main - sdt: Implement SDT probes using hot-patching

From: Ryan Libby <rlibby_at_gmail.com>
Date: Mon, 24 Jun 2024 16:27:55 UTC
On Mon, Jun 24, 2024 at 9:06 AM Mark Johnston <markj@freebsd.org> wrote:
>
> On Mon, Jun 24, 2024 at 08:36:55AM -0700, Ryan Libby wrote:
> > On Wed, Jun 19, 2024 at 1:58 PM Mark Johnston <markj@freebsd.org> wrote:
> > >
> > > The branch main has been updated by markj:
> > >
> > > URL: https://cgit.FreeBSD.org/src/commit/?id=ddf0ed09bd8f83677407db36828aca2c10f419c9
> > >
> > > commit ddf0ed09bd8f83677407db36828aca2c10f419c9
> > > Author:     Mark Johnston <markj@FreeBSD.org>
> > > AuthorDate: 2024-06-19 20:57:09 +0000
> > > Commit:     Mark Johnston <markj@FreeBSD.org>
> > > CommitDate: 2024-06-19 20:57:41 +0000
> > >
> > >     sdt: Implement SDT probes using hot-patching
> > >
> > >     The idea here is to avoid a memory access and conditional branch per
> > >     probe site.  Instead, the probe is represented by an "unreachable"
> > >     unconditional function call.  asm goto is used to store the address of
> > >     the probe site (represented by a no-op sled) and the address of the
> > >     function call into a tracepoint record.  Each SDT probe carries a list
> > >     of tracepoints.
> >
> > Questions out of curiosity and maybe ignorance:
> >
> > How does this work with relocations?  Something must be adjusting these
> > addresses?
>
> The compiler handles this as part of the implementation of asm goto:
> the inline assembly can reference jump targets with "%l<index>" and
> they're specified as operands to the asm goto statement.  In the kernel
> these references are resolved statically, and kernel modules will
> contain relocations for the sdt_tracepoint_set section.
>
> > > +/*
> > > + * Work around an apparent clang bug or limitation which prevents the use of the
> > > + * "i" (immediate) constraint with the probe structure.
> > > + */
> > > +#define        _SDT_ASM_PROBE_CONSTRAINT       "Ws"
> > > +#define        _SDT_ASM_PROBE_OPERAND          "p"
> >
> > Is it because i386 kmods are built with -fPIC?
>
> I suspect that that's related, yeah.  The compiler might be assuming
> that some indirection is needed to compute the target address, but in
> this case it's an address in the same function and presumably can safely
> be assumed to be an immediate.
>

That makes sense for the "%l1", does it also apply to the "%c0"?  Or
does use of "%c" for the probe pointer require non-PIC?  As in, don't
the _probes_ get relocated, and don't we need to patch the pointers to
the probes?

> > By the way, it seems gcc13 (latest in ports) doesn't support the "Ws"
> > constraint.  It seems to have been added to gcc 14.  I know i386 is tier
> > 2 these days, and gcc is a second consideration anyway.  Trying to test
> > out a patch for i386 gcc, I found that it doesn't build currently and
> > this is one of a few reasons.
>
> What happens if "i" is used with gcc?  IIRC I had tried gcc+i386 when
> testing the patch but the kernel failed to build for other reasons at
> the time.

It also errors out with constraint "%i" and modifier "%c".
/usr/src/freebsd/sys/sys/sdt.h:216:9: error: 'asm' operand 0 probably
does not match constraints [-Werror]
...
/usr/src/freebsd/sys/sys/sdt.h:216:9: error: impossible constraint in 'asm'

It works if I remove -fPIC, but I assume we're doing that for a reason...

Ryan