Skip to content

Commit 2edcadb

Browse files
committed
Emit nop1,nop5 instructions for x86_64 usdt probe
We can currently optimize uprobes on top of nop5 instructions, so application can define USDT_NOP to nop5 and use USDT macro to define optimized usdt probes. This works fine on new kernels, but could have performance penalty on older kernels. Until Linux vX.Y kernel's uprobe implementation treated nop5 as an instruction that needs to be single-stepped. execution of the usdt probe on top of nop1 on old kernels: - nop1 -> trigger usdt -> emulate nop1 -> continue execution of the usdt probe on top of nop5 on old kernels: - nop5 -> trigger usdt -> single step nop5 -> continue Note the 'single step nop5' as the source of performance regression. This creates a dilemma where we want nop5 on new kernels and nop1 on old ones, but we can't know upfront which kernel we'll run on. To workaround that we change the USDT macro to emit nop1,nop5 for the probe (instead of default nop1) and make record of that in USDT record (more on that below). This can be detected by application (libbpf) and it can place the uprobe either on nop1 or nop5 based on the optimization support in the kernel. We make record of using the nop1,nop5 instructions in the USDT ELF note data. Current elf note format is as follows: namesz (4B) | descsz (4B) | type (4B) | name | desc And current usdt record (with "stapsdt" name) placed in the note's desc data look like: loc_addr | 8 bytes base_addr | 8 bytes sema_addr | 8 bytes provider | zero terminated string name | zero terminated string args | zero terminated string None of the tested parsers (bpftrace-bcc, libbpf) checked that the args zero terminated byte is the actual end of the 'desc' data. As Andrii suggested we could use this and place extra zero byte right there as an indication for the parser we use the nop1,nop5 instructions. It's bit tricky, but the other way would be to introduce new elf note type or note name and change all existing parsers to recognize it. With the change above the existing parsers would still recognize such usdt probes and use safer (performance-wise) nop1 as uprobe target We can treat this extra zero as a backwards-compatible extension of provider+name+args section. If we ever need to have some extra flags or extra information (e.g., argument names or whatnot), we can treat this as either a fourth string or think about this as a single-byte length prefix for a fixed binary extra information that should follow (right now it's zero, so forward-compatible). For now just extra zero is the least amount of work but good enough to solve the problem, while being extendable for the future. Note we do not emit this extra byte if app defined its own nop through USDT_NOP macro. Suggested-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
1 parent f4ea2f5 commit 2edcadb

1 file changed

Lines changed: 19 additions & 0 deletions

File tree

‎usdt.h‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,19 @@ struct usdt_sema { volatile unsigned short active; };
312312
#ifndef USDT_NOP
313313
#if defined(__ia64__) || defined(__s390__) || defined(__s390x__)
314314
#define USDT_NOP nop 0
315+
#elif defined(__x86_64__)
316+
#define USDT_NOP .byte 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x0 /* nop, nop5 */
315317
#else
316318
#define USDT_NOP nop
317319
#endif
320+
#else
321+
/*
322+
* User defines its own nop instruction, do not emit extra note data
323+
* unless it's also defined by user.
324+
*/
325+
#ifndef __usdt_asm_extra
326+
#define __usdt_asm_extra
327+
#endif
318328
#endif /* USDT_NOP */
319329

320330
/*
@@ -403,6 +413,14 @@ struct usdt_sema { volatile unsigned short active; };
403413
__asm__ __volatile__ ("" :: "m" (sema));
404414
#endif
405415

416+
#ifndef __usdt_asm_extra
417+
#ifdef __x86_64__
418+
#define __usdt_asm_extra __usdt_asm1(.ascii "\0")
419+
#else
420+
#define __usdt_asm_extra
421+
#endif
422+
#endif
423+
406424
/* main USDT definition (nop and .note.stapsdt metadata) */
407425
#define __usdt_probe(group, name, sema_def, sema, ...) do { \
408426
sema_def(sema) \
@@ -420,6 +438,7 @@ struct usdt_sema { volatile unsigned short active; };
420438
__usdt_asm_strz(name) \
421439
__usdt_asm_args(__VA_ARGS__) \
422440
__usdt_asm1( .ascii "\0") \
441+
__usdt_asm_extra \
423442
__usdt_asm1(994: .balign 4) \
424443
__usdt_asm1( .popsection) \
425444
__usdt_asm1(.ifndef _.stapsdt.base) \

0 commit comments

Comments
 (0)