On Wed, 03 Jun 2026 10:41:18 +0200 "Arnd Bergmann" <[email protected]> wrote:
> On Wed, Jun 3, 2026, at 09:15, Rasmus Villemoes wrote: > > On Tue, Jun 02 2026, "Arnd Bergmann" <[email protected]> wrote: > >> On Tue, Jun 2, 2026, at 20:59, Andy Shevchenko wrote: > >>> On Tue, Jun 02, 2026 at 05:07:05PM +0200, Arnd Bergmann wrote: > > > > May I suggest a different approach, that avoids having that extra > > function emitted (which presumably compiles to a single jump > > instruction, but still, with retpoline and CFI and all that it all adds > > up): Keep the declaration of __vsnprintf() in the header without the > > __print() attribute, but then do > > > > int __vsnprintf(char *buf, size_t size, const char *fmt_str, va_list args) > > __alias(vsnprintf); > > > > in vsprintf.c. Aside from reusing the same entry point, I could well > > imagine a compiler some day complaining about seeing the printf > > attribute applied in a local extra declaration but not having it in the > > header file. > > > > Presumably it will need its own EXPORT_SYMBOL if any of the intended > > users are modular, and it certainly still needs a comment. > > I had tried that earlier but given up because the attributes have to > match exactly. > > This definition works with all currently supported versions of gcc, > but may have to change when the there is a new version that adds > even more attributes: > > int > __printf(3, 0) > __attribute__((nothrow)) > __attribute__((nonnull(1))) > __vsnprintf(char *__restrict buf, size_t size, > const char * __restrict fmt_str, va_list args) > __alias(vsnprintf); > > We'd probably want to also add __nothrow and __nonnull macros > in linux/compiler-attributes.h if we do this. > > For reference, see below for the alternative idea I had > that avoids adding the __vsnprintf() alias altogether by > passing down the va_format using "%pV". > > I don't think I actually got this one right in the end > since I only build-tested it, but I expect it could be done > if someone is able to test and fix all the corner cases > properly. > > Arnd > > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > index 4715330c7b6b..8e44fc3e60b0 100644 > --- a/include/linux/trace_events.h > +++ b/include/linux/trace_events.h > @@ -956,14 +956,11 @@ perf_trace_buf_submit(void *raw_data, int size, int > rctx, u16 type, > * gcc warns that you can not use a va_list in an inlined > * function. But lets me make it into a macro :-/ > */ > -#define __trace_event_vstr_len(fmt, va) \ > +#define __trace_event_vstr_len(vf) \ > ({ \ > - va_list __ap; \ > int __ret; \ > \ > - va_copy(__ap, *(va)); \ > - __ret = __vsnprintf(NULL, 0, fmt, __ap) + 1; \ > - va_end(__ap); \ > + __ret = snprintf(NULL, 0, "%pV", vf) + 1; \ This adds an extra snprintf call - non-trivial and more stack. Can't you just use the old code with vf->fmt and vf->ap ? And does the %pV" include the va_copy()? It isn't normally needed. Any scheme for avoiding doing the printf processing twice is likely to be a gain. -- David
