On Wed, 16 Oct 2024 14:07:31 +0200 Sven Schnelle <[email protected]> wrote:
> "Masami Hiramatsu (Google)" <[email protected]> writes: > > > From: Masami Hiramatsu (Google) <[email protected]> > > > > Rewrite fprobe implementation on function-graph tracer. > > Major API changes are: > > - 'nr_maxactive' field is deprecated. > > - This depends on CONFIG_DYNAMIC_FTRACE_WITH_ARGS or > > !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS, and > > CONFIG_HAVE_FUNCTION_GRAPH_FREGS. So currently works only > > on x86_64. > > - Currently the entry size is limited in 15 * sizeof(long). > > - If there is too many fprobe exit handler set on the same > > function, it will fail to probe. > > > > Signed-off-by: Masami Hiramatsu (Google) <[email protected]> > > Cc: Steven Rostedt <[email protected]> > > Cc: Mark Rutland <[email protected]> > > Cc: Catalin Marinas <[email protected]> > > Cc: Will Deacon <[email protected]> > > Cc: Huacai Chen <[email protected]> > > Cc: WANG Xuerui <[email protected]> > > Cc: Michael Ellerman <[email protected]> > > Cc: Nicholas Piggin <[email protected]> > > Cc: Christophe Leroy <[email protected]> > > Cc: Naveen N Rao <[email protected]> > > Cc: Madhavan Srinivasan <[email protected]> > > Cc: Paul Walmsley <[email protected]> > > Cc: Palmer Dabbelt <[email protected]> > > Cc: Albert Ou <[email protected]> > > Cc: Heiko Carstens <[email protected]> > > Cc: Vasily Gorbik <[email protected]> > > Cc: Alexander Gordeev <[email protected]> > > Cc: Christian Borntraeger <[email protected]> > > Cc: Sven Schnelle <[email protected]> > > Cc: Thomas Gleixner <[email protected]> > > Cc: Ingo Molnar <[email protected]> > > Cc: Borislav Petkov <[email protected]> > > Cc: Dave Hansen <[email protected]> > > Cc: [email protected] > > Cc: "H. Peter Anvin" <[email protected]> > > Cc: Mathieu Desnoyers <[email protected]> > > Cc: Andrew Morton <[email protected]> > > > [..] > > > diff --git a/include/linux/fprobe.h b/include/linux/fprobe.h > > index ef609bcca0f9..2d06bbd99601 100644 > > --- a/include/linux/fprobe.h > > +++ b/include/linux/fprobe.h > > @@ -5,10 +5,11 @@ > [..] > > +static inline unsigned long encode_fprobe_header(struct fprobe *fp, int > > size_words) > > +{ > > + if (WARN_ON_ONCE(size_words > MAX_FPROBE_DATA_SIZE_WORD || > > + ((unsigned long)fp & ~FPROBE_HEADER_PTR_MASK) != > > + ~FPROBE_HEADER_PTR_MASK)) { > > + return 0; > > } > > + return ((unsigned long)size_words << FPROBE_HEADER_PTR_BITS) | > > + ((unsigned long)fp & FPROBE_HEADER_PTR_MASK); > > +} > > + > > +/* Return reserved data size in words */ > > +static inline int decode_fprobe_header(unsigned long val, struct fprobe > > **fp) > > +{ > > + unsigned long ptr; > > + > > + ptr = (val & FPROBE_HEADER_PTR_MASK) | ~FPROBE_HEADER_PTR_MASK; > > + if (fp) > > + *fp = (struct fprobe *)ptr; > > + return val >> FPROBE_HEADER_PTR_BITS; > > +} > > I think that still has the issue that the size is encoded in the > leftmost fields of the pointer, which doesn't work on all > architectures. I reported this already in v15 > (https://lore.kernel.org/all/[email protected]/) Oops, thanks for reporting. I should missed that. > I haven't yet fully understood why this logic is needed, but the > WARN_ON_ONCE triggers on s390. I'm assuming this fails because fp always > has the upper bits of the address set on x86 (and likely others). As an > example, in my test setup, fp is 0x8feec218 on s390, while it is > 0xffff888100add118 in x86-kvm. Ah, so s390 kernel/user memory layout is something like 4G/4G? Hmm, this encode expects the leftmost 4bit is filled. For the architecture which has 32bit address space, we may be possible to use "unsigned long long" for 'val' on shadow stack (and use the first 32bit for fp and another 32bit for size). Anyway, I need to redesign it depending on architecture. Thank you! -- Masami Hiramatsu (Google) <[email protected]>
