jump-label/x86: Compress jmps to 2 bytes where possible

From: Steven Rostedt <[email protected]>

Add the infrastructure to allow architectures to modify the jump label locations at compile time. This is mainly for x86, where the jmps may

be either 2 bytes or 5 bytes. Instead of wasting 5 bytes for all jump labels, this code will let x86 put in a jmp instead of a 5 byte nop. Then the assembler will make either a 2 byte or 5 byte jmp depending on where the target is.

At compile time, this code will replace the jmps with either a 2 byte or 5 byte nop depending on the size of the jmp that was added.

Signed-off-by Steven Rostedt <[email protected]>

---
 Makefile                    |    7 +
 arch/Kconfig                |    6 +
 scripts/Makefile            |    1 +
 scripts/Makefile.build      |   15 ++-
 scripts/update_jump_label.c |  335 +++++++++++++++++++++++++++++++++++++++++++
 scripts/update_jump_label.h |  283 ++++++++++++++++++++++++++++++++++++
 6 files changed, 645 insertions(+), 2 deletions(-)
 create mode 100644 scripts/update_jump_label.c
 create mode 100644 scripts/update_jump_label.h

scripts/update_jump_label.c new file +43 -0

new file mode 100644
index 0000000..399d898
@@ -0,0 +1,335 @@
+/*
+ * update_jump_label.c: replace jmps with nops at compile time.
+ * Copyright 2010 Steven Rostedt <[email protected]>, Red Hat Inc.
+ *  Parsing of the elf file was influenced by recordmcount.c
+ *  originally written by and copyright to John F. Reiser <[email protected]>.
+ */
+
+/*
+ * Note, this code is originally designed for x86, but may be used by
+ * other archs to do the nop updates at compile time instead of at boot time.
+ * X86 uses this as an optimization, as jmps can be either 2 bytes or 5 bytes.
+ * Inserting a 2 byte where possible helps with both CPU performance and
+ * icache strain.
+ */
+#include <sys/types.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <getopt.h>
+#include <elf.h>
+#include <fcntl.h>
+#include <setjmp.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <unistd.h>
+
+static int fd_map;	/* File descriptor for file being modified. */
+static struct stat sb;	/* Remember .st_size, etc. */
+static int mmap_failed; /* Boolean flag. */
+
+static void die(const char *err, const char *fmt, ...)
+{
+	va_list ap;
+
+	if (err)
+		perror(err);
+
+	if (fmt) {
+		va_start(ap, fmt);
+		fprintf(stderr, "Fatal error:  ");
+		vfprintf(stderr, fmt, ap);
+		fprintf(stderr, "\n");

scripts/update_jump_label.c new file +9 -0

+
+	while (p >= arg && *p != '/')
+		p--;
+	p++;
+
+	printf("usage: %s file\n"
+	       "\n", p);
+	exit(-1);
+}

+/*
+ * Read the relocation table again, but this time its called on sections
+ * that are not going to be traced. The mcount calls here will be converted

+ * into nops.
+ */

scripts/update_jump_label.c new file +18 -0

+static void FUNC(nop_jump_label)(Elf_Shdr const *const relhdr,
+		       Elf_Ehdr const *const ehdr,
+		       const char *const txtname)
+{
+	Elf_Shdr *const shdr0 = get_shdr(ehdr);
+	Elf_Sym const *sym0;
+	char const *str0;
+	Elf_Rel const *relp;
+	Elf_Rela const *relap;
+	Elf_Shdr const *const shdr = &shdr0[w(relhdr->sh_info)];
+	unsigned rel_entsize = w(relhdr->sh_entsize);
+	unsigned const nrel = _w(relhdr->sh_size) / rel_entsize;
+	int t;
+
+	FUNC(get_sym_str_and_relp)(relhdr, ehdr, &sym0, &str0, &relp);
+
+	for (t = nrel; t > 0; t -= 3) {
+		int ret = -1;

scripts/update_jump_label.c new file +73 -0

+
+		/* jump label sections are paired in threes */
+		relp = (Elf_Rel const *)(rel_entsize * 3 + (void *)relp);
+	}
+}
+
+/* Find relocation section hdr for a given section */
+static const Elf_Shdr *
+FUNC(find_relhdr)(const Elf_Ehdr *ehdr, const Elf_Shdr *shdr)
+{
+	const Elf_Shdr *shdr0 = get_shdr(ehdr);
+	int nhdr = w2(ehdr->e_shnum);
+	const Elf_Shdr *hdr;
+	int i;
+
+	for (hdr = shdr0, i = 0; i < nhdr; hdr = &shdr0[++i]) {
+		if (w(hdr->sh_type) != SHT_REL &&
+		    w(hdr->sh_type) != SHT_RELA)
+			continue;
+
+		/*
+		 * The relocation section's info field holds
+		 * the section index that it represents.
+		 */
+		if (shdr == &shdr0[w(hdr->sh_info)])
+			return hdr;
+	}
+	return NULL;
+}
+
+/* Find a section headr based on name and type */
+static const Elf_Shdr *
+FUNC(find_shdr)(const Elf_Ehdr *ehdr, const char *name, uint_t type)
+{
+	const Elf_Shdr *shdr0 = get_shdr(ehdr);
+	const Elf_Shdr *shstr = &shdr0[w2(ehdr->e_shstrndx)];
+	const char *shstrtab = (char *)get_section_loc(ehdr, shstr);
+	int nhdr = w2(ehdr->e_shnum);
+	const Elf_Shdr *hdr;
+	const char *hdrname;
+	int i;
+
+	for (hdr = shdr0, i = 0; i < nhdr; hdr = &shdr0[++i]) {
+		if (w(hdr->sh_type) != type)
+			continue;
+
+		/* If we are just looking for a section by type (ie. SYMTAB) */
+		if (!name)
+			return hdr;
+
+		hdrname = &shstrtab[w(hdr->sh_name)];
+		if (strcmp(hdrname, name) == 0)
+			return hdr;
+	}
+	return NULL;
+}
+
+static void
+FUNC(section_update)(const Elf_Ehdr *ehdr, const Elf_Shdr *symhdr,
+		     unsigned shtype, const Elf_Rel *rel, void *data)
+{
+	const Elf_Shdr *shdr0 = get_shdr(ehdr);
+	const Elf_Shdr *targethdr;
+	const Elf_Rela *rela;
+	const Elf_Sym *syment;
+	uint_t offset = _w(rel->r_offset);
+	uint_t info = _w(rel->r_info);
+	uint_t sym = ELF_R_SYM(info);
+	uint_t type = ELF_R_TYPE(info);
+	uint_t addend;
+	uint_t targetloc;
+
+	if (shtype == SHT_RELA) {

scripts/update_jump_label.c new file +63 -0

+	if (type != R_386_32)
+		die(NULL, "Arch relocation type %d not supported", type);
+
+	targetloc += addend;
+
+#if 0
+	printf("offset=%x target=%x shoffset=%x add=%x indx=%x\n",
+	       offset, targetloc, _w(targethdr->sh_offset), addend,
+	       targetloc - _w(targethdr->sh_offset));
+#endif
+	*(uint_t *)(data + offset) = targetloc;
+}
+
+/* Overall supervision for Elf32 ET_REL file. */
+static void
+FUNC(do_func)(Elf_Ehdr *ehdr, char const *const fname, unsigned const reltype)
+{
+	const Elf_Shdr *jlshdr;
+	const Elf_Shdr *jlrhdr;
+	const Elf_Shdr *symhdr;
+	const Elf_Rel *rel;
+	unsigned size;
+	unsigned cnt;
+	unsigned i;
+	uint_t type;
+	void *jdata;
+	void *data;
+
+	jlshdr = FUNC(find_shdr)(ehdr, "__jump_table", SHT_PROGBITS);
+	if (!jlshdr)
+		return;
+
+	jlrhdr = FUNC(find_relhdr)(ehdr, jlshdr);
+	if (!jlrhdr)
+		return;
+
+	/*
+	 * Create and fill in the __jump_table section and use it to
+	 * find the offsets into the text that we want to update.
+	 * We create it so that we do not depend on the order of the
+	 * relocations, and use the table directly, as it is broken
+	 * up into sections.
+	 */
+	size = _w(jlshdr->sh_size);
+	data = umalloc(size);
+
+	jdata = (void *)get_section_loc(ehdr, jlshdr);
+	memcpy(data, jdata, size);
+
+	cnt = _w(jlrhdr->sh_size) / w(jlrhdr->sh_entsize);
+
+	rel = (const Elf_Rel *)get_section_loc(ehdr, jlrhdr);
+
+	/* Is this as Rel or Rela? */
+	type = w(jlrhdr->sh_type);
+
+	symhdr = FUNC(find_shdr)(ehdr, NULL, SHT_SYMTAB);
+
+	for (i = 0; i < cnt; i++) {
+		FUNC(section_update)(ehdr, symhdr, type, rel, data);
+		rel = (void *)rel + w(jlrhdr->sh_entsize);
+	}
+

scripts/update_jump_label.c new file +2 -0

+	free(data);
+}

Signature · 1 line

1.7.8.3

Hi Steven,

I really like this patchset! A few comments below,

Quoted 2 lines · * Steven Rostedt ([email protected]) wrote:

From: Steven Rostedt <[email protected]>

Quoted 2 lines

Add the infrastructure to allow architectures to modify the jump label locations at compile time. This is mainly for x86, where the jmps may

is it "at compile time" or "after compilation" ? AFAIU, this update_jump_label script gets executed on the .o, which makes it technically more part of the linking stage than the compilation stage.

Quoted 6 lines

be either 2 bytes or 5 bytes. Instead of wasting 5 bytes for all jump labels, this code will let x86 put in a jmp instead of a 5 byte nop. Then the assembler will make either a 2 byte or 5 byte jmp depending on where the target is.

Quoted 3 lines

At compile time, this code will replace the jmps with either a 2 byte or 5 byte nop depending on the size of the jmp that was added.

A small note somewhere saying that this 2 vs 5 bytes choice done by the compiler is a pessimistic approximation would be good, so people don't end up complaining if they see that the compiler chose a 5-byte nop for a 120 bytes offset.

Quoted 13 lines

Signed-off-by Steven Rostedt <[email protected]>

---
 Makefile                    |    7 +
 arch/Kconfig                |    6 +
 scripts/Makefile            |    1 +
 scripts/Makefile.build      |   15 ++-
 scripts/update_jump_label.c |  335 +++++++++++++++++++++++++++++++++++++++++++
 scripts/update_jump_label.h |  283 ++++++++++++++++++++++++++++++++++++
 6 files changed, 645 insertions(+), 2 deletions(-)
 create mode 100644 scripts/update_jump_label.c
 create mode 100644 scripts/update_jump_label.h

[...]

Quoted 55 lines

scripts/update_jump_label.c new file +49 -0

new file mode 100644
index 0000000..399d898
@@ -0,0 +1,335 @@
+/*
+ * update_jump_label.c: replace jmps with nops at compile time.
+ * Copyright 2010 Steven Rostedt <[email protected]>, Red Hat Inc.
+ *  Parsing of the elf file was influenced by recordmcount.c
+ *  originally written by and copyright to John F. Reiser <[email protected]>.
+ */
+
+/*
+ * Note, this code is originally designed for x86, but may be used by
+ * other archs to do the nop updates at compile time instead of at boot time.
+ * X86 uses this as an optimization, as jmps can be either 2 bytes or 5 bytes.
+ * Inserting a 2 byte where possible helps with both CPU performance and
+ * icache strain.
+ */
+#include <sys/types.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <getopt.h>
+#include <elf.h>
+#include <fcntl.h>
+#include <setjmp.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <unistd.h>
+
+static int fd_map;	/* File descriptor for file being modified. */
+static struct stat sb;	/* Remember .st_size, etc. */
+static int mmap_failed; /* Boolean flag. */
+
+static void die(const char *err, const char *fmt, ...)
+{
+	va_list ap;
+
+	if (err)
+		perror(err);
+
+	if (fmt) {
+		va_start(ap, fmt);
+		fprintf(stderr, "Fatal error:  ");
+		vfprintf(stderr, fmt, ap);
+		fprintf(stderr, "\n");
+		va_end(ap);
+	}
+
+	exit(1);
+}
+

scripts/update_jump_label.c line 53

Quoted 4 lines

scripts/update_jump_label.c new file +4 -0

+static void usage(char **argv)
+{
+	char *arg = argv[0];
+	char *p = arg+strlen(arg);

I'm pretty sure checkpatch will complain about the coding style of this file. I won't point out the various nits, but there is a need for cleanup before pulling it.

Quoted 10 lines

scripts/update_jump_label.c new file +9 -0

+
+	while (p >= arg && *p != '/')
+		p--;
+	p++;
+
+	printf("usage: %s file\n"
+	       "\n", p);
+	exit(-1);
+}

[...]

Quoted 3 lines

+/*
+ * Read the relocation table again, but this time its called on sections
+ * that are not going to be traced. The mcount calls here will be converted

mcount calls -> jump labels

Quoted 21 lines

+ * into nops.
+ */

scripts/update_jump_label.c new file +18 -0

+static void FUNC(nop_jump_label)(Elf_Shdr const *const relhdr,
+		       Elf_Ehdr const *const ehdr,
+		       const char *const txtname)
+{
+	Elf_Shdr *const shdr0 = get_shdr(ehdr);
+	Elf_Sym const *sym0;
+	char const *str0;
+	Elf_Rel const *relp;
+	Elf_Rela const *relap;
+	Elf_Shdr const *const shdr = &shdr0[w(relhdr->sh_info)];
+	unsigned rel_entsize = w(relhdr->sh_entsize);
+	unsigned const nrel = _w(relhdr->sh_size) / rel_entsize;
+	int t;
+
+	FUNC(get_sym_str_and_relp)(relhdr, ehdr, &sym0, &str0, &relp);
+
+	for (t = nrel; t > 0; t -= 3) {
+		int ret = -1;

scripts/update_jump_label.c

Quoted 10 lines

scripts/update_jump_label.c new file +10 -0

+
+		relap = (Elf_Rela const *)relp;
+		printf("rel offset=%lx info=%lx sym=%lx type=%lx addend=%lx\n",
+		       (long)relap->r_offset, (long)relap->r_info,
+		       (long)ELF_R_SYM(relap->r_info),
+		       (long)ELF_R_TYPE(relap->r_info),
+		       (long)relap->r_addend);
+
+		if (0 && make_nop)
+			ret = make_nop((void *)ehdr, shdr->sh_offset + relp->r_offset);

if (0 && -> leftover code ?

This whole FUNC(nop_jump_label) function seems to be unused.

Quoted 78 lines

scripts/update_jump_label.c new file +77 -0

+
+		/* jump label sections are paired in threes */
+		relp = (Elf_Rel const *)(rel_entsize * 3 + (void *)relp);
+	}
+}
+
+/* Find relocation section hdr for a given section */
+static const Elf_Shdr *
+FUNC(find_relhdr)(const Elf_Ehdr *ehdr, const Elf_Shdr *shdr)
+{
+	const Elf_Shdr *shdr0 = get_shdr(ehdr);
+	int nhdr = w2(ehdr->e_shnum);
+	const Elf_Shdr *hdr;
+	int i;
+
+	for (hdr = shdr0, i = 0; i < nhdr; hdr = &shdr0[++i]) {
+		if (w(hdr->sh_type) != SHT_REL &&
+		    w(hdr->sh_type) != SHT_RELA)
+			continue;
+
+		/*
+		 * The relocation section's info field holds
+		 * the section index that it represents.
+		 */
+		if (shdr == &shdr0[w(hdr->sh_info)])
+			return hdr;
+	}
+	return NULL;
+}
+
+/* Find a section headr based on name and type */
+static const Elf_Shdr *
+FUNC(find_shdr)(const Elf_Ehdr *ehdr, const char *name, uint_t type)
+{
+	const Elf_Shdr *shdr0 = get_shdr(ehdr);
+	const Elf_Shdr *shstr = &shdr0[w2(ehdr->e_shstrndx)];
+	const char *shstrtab = (char *)get_section_loc(ehdr, shstr);
+	int nhdr = w2(ehdr->e_shnum);
+	const Elf_Shdr *hdr;
+	const char *hdrname;
+	int i;
+
+	for (hdr = shdr0, i = 0; i < nhdr; hdr = &shdr0[++i]) {
+		if (w(hdr->sh_type) != type)
+			continue;
+
+		/* If we are just looking for a section by type (ie. SYMTAB) */
+		if (!name)
+			return hdr;
+
+		hdrname = &shstrtab[w(hdr->sh_name)];
+		if (strcmp(hdrname, name) == 0)
+			return hdr;
+	}
+	return NULL;
+}
+
+static void
+FUNC(section_update)(const Elf_Ehdr *ehdr, const Elf_Shdr *symhdr,
+		     unsigned shtype, const Elf_Rel *rel, void *data)
+{
+	const Elf_Shdr *shdr0 = get_shdr(ehdr);
+	const Elf_Shdr *targethdr;
+	const Elf_Rela *rela;
+	const Elf_Sym *syment;
+	uint_t offset = _w(rel->r_offset);
+	uint_t info = _w(rel->r_info);
+	uint_t sym = ELF_R_SYM(info);
+	uint_t type = ELF_R_TYPE(info);
+	uint_t addend;
+	uint_t targetloc;
+
+	if (shtype == SHT_RELA) {
+		rela = (const Elf_Rela *)rel;
+		addend = _w(rela->r_addend);
+	} else
+		addend = _w(*(int *)(data + offset));

scripts/update_jump_label.c

Quoted 6 lines

scripts/update_jump_label.c new file +6 -0

+
+	syment = (const Elf_Sym *)get_section_loc(ehdr, symhdr);
+	targethdr = &shdr0[w2(syment[sym].st_shndx)];
+	targetloc = _w(targethdr->sh_offset);
+
+	/* TODO, need a separate function for all archs */

maybe this is where you want to call FUNC(nop_jump_label) too ?

scripts/update_jump_label.c

Quoted 11 lines

scripts/update_jump_label.c new file +10 -0

+	if (type != R_386_32)
+		die(NULL, "Arch relocation type %d not supported", type);
+
+	targetloc += addend;
+
+#if 0
+	printf("offset=%x target=%x shoffset=%x add=%x indx=%x\n",
+	       offset, targetloc, _w(targethdr->sh_offset), addend,
+	       targetloc - _w(targethdr->sh_offset));
+#endif

Quoted 59 lines

scripts/update_jump_label.c new file +59 -0

+	*(uint_t *)(data + offset) = targetloc;
+}
+
+/* Overall supervision for Elf32 ET_REL file. */
+static void
+FUNC(do_func)(Elf_Ehdr *ehdr, char const *const fname, unsigned const reltype)
+{
+	const Elf_Shdr *jlshdr;
+	const Elf_Shdr *jlrhdr;
+	const Elf_Shdr *symhdr;
+	const Elf_Rel *rel;
+	unsigned size;
+	unsigned cnt;
+	unsigned i;
+	uint_t type;
+	void *jdata;
+	void *data;
+
+	jlshdr = FUNC(find_shdr)(ehdr, "__jump_table", SHT_PROGBITS);
+	if (!jlshdr)
+		return;
+
+	jlrhdr = FUNC(find_relhdr)(ehdr, jlshdr);
+	if (!jlrhdr)
+		return;
+
+	/*
+	 * Create and fill in the __jump_table section and use it to
+	 * find the offsets into the text that we want to update.
+	 * We create it so that we do not depend on the order of the
+	 * relocations, and use the table directly, as it is broken
+	 * up into sections.
+	 */
+	size = _w(jlshdr->sh_size);
+	data = umalloc(size);
+
+	jdata = (void *)get_section_loc(ehdr, jlshdr);
+	memcpy(data, jdata, size);
+
+	cnt = _w(jlrhdr->sh_size) / w(jlrhdr->sh_entsize);
+
+	rel = (const Elf_Rel *)get_section_loc(ehdr, jlrhdr);
+
+	/* Is this as Rel or Rela? */
+	type = w(jlrhdr->sh_type);
+
+	symhdr = FUNC(find_shdr)(ehdr, NULL, SHT_SYMTAB);
+
+	for (i = 0; i < cnt; i++) {
+		FUNC(section_update)(ehdr, symhdr, type, rel, data);
+		rel = (void *)rel + w(jlrhdr->sh_entsize);
+	}
+
+	/*
+	 * This is specific to x86. The jump_table is stored in three
+	 * long words. The first is the location of the jmp target we
+	 * must update.
+	 */
+	cnt = size / sizeof(uint_t);

scripts/update_jump_label.c

Quoted 4 lines

scripts/update_jump_label.c new file +4 -0

+
+	for (i = 0; i < cnt; i += 3)
+		make_nop((void *)ehdr, *(uint_t *)(data + i * sizeof(uint_t)));
+

Why is this function iterating twice on the __jump_table section rather than simply writing the nops in the first pass, within the section_update function ?

From: Steven Rostedt <[email protected]>

Have the jump labels add a "jmp" in the assembly instead of a default nop. This will cause the assembler to put in either a 2 byte or 5 byte jmp depending on where the target lable is.

Then at compile time, the update_jump_label code will replace the jmps with either 2 or 5 byte nops.

On boot up, the code can be examined to see if the jump label uses either a 2 or 5 byte nop and replace it.

By allowing the jump labels to be 2 bytes, it speeds up the nops, not only 2 byte nops are faster than 5 byte nops, but also because it saves on cache foot print.

text data bss dec hex filename
13403667 3666856 2998272 20068795 13239bb ../nobackup/mxtest/vmlinux-old 13398536 3666856 2998272 20063664 13225b0 ../nobackup/mxtest/vmlinux-new

Converting the current v3.2 trace points saved 5,131 bytes. As more places use jump labels, this will have a bigger savings.

From: Steven Rostedt <[email protected]>

Have the jump labels add a "jmp" in the assembly instead of a default nop. This will cause the assembler to put in either a 2 byte or 5 byte jmp depending on where the target lable is.

Then at compile time, the update_jump_label code will replace the jmps with either 2 or 5 byte nops.

On boot up, the code can be examined to see if the jump label uses either a 2 or 5 byte nop and replace it.

By allowing the jump labels to be 2 bytes, it speeds up the nops, not only 2 byte nops are faster than 5 byte nops, but also because it saves on cache foot print.

text data bss dec hex filename
13403667 3666856 2998272 20068795 13239bb ../nobackup/mxtest/vmlinux-old 13398536 3666856 2998272 20063664 13225b0 ../nobackup/mxtest/vmlinux-new

Converting the current v3.2 trace points saved 5,131 bytes. As more places use jump labels, this will have a bigger savings.

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/x86/Kconfig                  |    1 +
 arch/x86/include/asm/jump_label.h |    2 +-
 arch/x86/kernel/jump_label.c      |   86 ++++++++++++++++++++++++++++++++++---
 3 files changed, 81 insertions(+), 8 deletions(-)

arch/x86/Kconfig +1 -0

index efb4294..b5004c1 100644
@@ -61,6 +61,7 @@ config X86
 	select HAVE_ARCH_KMEMCHECK
 	select HAVE_USER_RETURN_NOTIFIER
 	select HAVE_ARCH_JUMP_LABEL
+	select HAVE_BUILD_TIME_JUMP_LABEL
 	select HAVE_TEXT_POKE_SMP
 	select HAVE_GENERIC_HARDIRQS
 	select HAVE_SPARSE_IRQ

arch/x86/include/asm/jump_label.h +0 -0

index a32b18c..872b3e1 100644

arch/x86/include/asm/jump_label.h +0 -0

 		".pushsection __jump_table,  \"aw\" \n\t"
 		_ASM_ALIGN "\n\t"
 		_ASM_PTR "1b, %l[l_yes], %c0 \n\t"

arch/x86/kernel/jump_label.c +16 -1

index ea9d5f2f..d5b84de 100644
@@ -16,12 +16,27 @@
 
 #ifdef HAVE_JUMP_LABEL
 
+static unsigned char nop_short[] = { P6_NOP2 };
+
+/* These are the nops added at compile time */
+#ifdef CONFIG_X86_32
+static unsigned char default_nop[5] = { 0x3e, 0x8d, 0x74, 0x26, 0x00 };
+#else
+static unsigned char default_nop[5] = { 0x0f, 0x1f, 0x44, 0x00, 0x00 };
+#endif
+
+static int update_nops;
+
 union jump_code_union {
 	char code[JUMP_LABEL_NOP_SIZE];
 	struct {
 		char jump;
 		int offset;
-	} __attribute__((packed));
+	} __packed;
+	struct {
+		char jump_short;
+		char offset_short;
+	} __packed;
 };
 
 static void __jump_label_transform(struct jump_entry *entry,

arch/x86/kernel/jump_label.c +18 -1

+			return;
+
+		if (memcmp(ip, nop_short, 2) == 0) {
+			size = 2;
+			code.jump_short = 0xeb;
+			code.offset = entry->target -
+				(entry->code + 2);
+			/* Check for overflow ? */
+		} else if (memcmp(ip, ideal, 5) == 0 ||
+			   memcmp(ip, default_nop, 5)) {
+			size = JUMP_LABEL_NOP_SIZE;
+			code.jump = 0xe9;
+			code.offset = entry->target - (entry->code + size);
+		} else
+			BUG();
 
-	(*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
+	} else {
+		/* Check if already disabled */
+		if (memcmp(ip, nop_short, 2) == 0)

arch/x86/kernel/jump_label.c +12 -0

+
+	(*poker)((void *)entry->code, &code, size);
 }
 
 void arch_jump_label_transform(struct jump_entry *entry,
 			       enum jump_label_type type)
 {
+	/* All nops should be updated to the ideal nop by now */
+	update_nops = 0;
+
 	get_online_cpus();
 	mutex_lock(&text_mutex);
 	__jump_label_transform(entry, type, text_poke_smp);

@@ -53,6 +118,13 @@ void arch_jump_label_transform(struct jump_entry *entry,
 void arch_jump_label_transform_static(struct jump_entry *entry,
 				      enum jump_label_type type)
 {
+	/*
+	 * If the default nop does not equal the ideal nop, then
+	 * update them.
+	 */
+	if (memcmp(default_nop, ideal_nops[NOP_ATOMIC5], 5) != 0)
+		update_nops = 1;
+
 	__jump_label_transform(entry, type, text_poke_early);
 }
 

Signature · 1 line

1.7.8.3

Quoted 7 lines

arch/x86/include/asm/jump_label.h +0 -0

index a32b18c..872b3e1 100644
@@ -14,7 +14,7 @@
 static __always_inline bool arch_static_branch(struct jump_label_key *key)
 {

arch/x86/include/asm/jump_label.h line 17

Quoted 3 lines

arch/x86/include/asm/jump_label.h +1 -1

 	asm goto("1:"
-		JUMP_LABEL_INITIAL_NOP
+		"jmp %l[l_yes]\n"

Is it possible that the compiler choose a jump that is not 2 or 5-byte ? e.g. a jmp rel16 (e9 opcode) on 32-bit x86, or any of the other instruction listed under the JMP-Jump instruction in the Intel insn manual ?

arch/x86/include/asm/jump_label.h

Quoted 8 lines

arch/x86/include/asm/jump_label.h +5 -0

 				   void *(*poker)(void *, const void *, size_t))
 {
 	union jump_code_union code;
+	unsigned char nop;
+	unsigned char op;
+	unsigned size;
+	void *ip = (void *)entry->code;
+	void *ideal = (void *)ideal_nops[NOP_ATOMIC5];

"void *" should possibly be "unsigned char *" here to respect the nop place-holder typing.

Quoted 5 lines

arch/x86/include/asm/jump_label.h +4 -0

+
+	/* Use probe_kernel_read()? */
+	op = *(unsigned char *)ip;
+	nop = ideal_nops[NOP_ATOMIC5][0];

arch/x86/include/asm/jump_label.h

Quoted 8 lines

arch/x86/include/asm/jump_label.h +2 -5

 	if (type == JUMP_LABEL_ENABLE) {
-		code.jump = 0xe9;
-		code.offset = entry->target -
-				(entry->code + JUMP_LABEL_NOP_SIZE);
-	} else
-		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
+		if (op == 0xe9 || op == 0xeb)
+			/* Already enabled. Warn? */

This could be caused by failure to run the link-time script, or running the transform twice. A warning would indeed be welcome, as this should never happen.

Quoted 12 lines

arch/x86/kernel/jump_label.c static void __jump_label_transform(struct jump_entry *entry, · line 51

Quoted 8 lines

arch/x86/kernel/jump_label.c +5 -0

 				   void *(*poker)(void *, const void *, size_t))
 {
 	union jump_code_union code;
+	unsigned char nop;
+	unsigned char op;
+	unsigned size;
+	void *ip = (void *)entry->code;
+	void *ideal = (void *)ideal_nops[NOP_ATOMIC5];

"void *" should possibly be "unsigned char *" here to respect the nop place-holder typing.

const unsigned char * please.

Quoted 6 lines

Quoted 6 lines

arch/x86/kernel/jump_label.c +4 -0

+
+	/* Use probe_kernel_read()? */
+	op = *(unsigned char *)ip;
+	nop = ideal_nops[NOP_ATOMIC5][0];

Quoted 13 lines

arch/x86/kernel/jump_label.c static void __jump_label_transform(struct jump_entry *entry, · line 59

Quoted 8 lines

arch/x86/kernel/jump_label.c +2 -5

 	if (type == JUMP_LABEL_ENABLE) {
-		code.jump = 0xe9;
-		code.offset = entry->target -
-				(entry->code + JUMP_LABEL_NOP_SIZE);
-	} else
-		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
+		if (op == 0xe9 || op == 0xeb)
+			/* Already enabled. Warn? */

This could be caused by failure to run the link-time script, or running the transform twice. A warning would indeed be welcome, as this should never happen.

Warning? No. ERROR. Something very bad could be happening here. We have covered this before.

Quoted 20 lines · H. Peter Anvin

Quoted 19 lines

Quoted 4 lines

arch/x86/kernel/jump_label.c +3 -0

+
+	/* Use probe_kernel_read()? */
+	op = *(unsigned char *)ip;

arch/x86/kernel/jump_label.c static void __jump_label_transform(struct jump_entry *entry, · line 59

Quoted 10 lines

arch/x86/kernel/jump_label.c +3 -5

+	nop = ideal_nops[NOP_ATOMIC5][0];
 
 	if (type == JUMP_LABEL_ENABLE) {
-		code.jump = 0xe9;
-		code.offset = entry->target -
-				(entry->code + JUMP_LABEL_NOP_SIZE);
-	} else
-		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
+		if (op == 0xe9 || op == 0xeb)
+			/* Already enabled. Warn? */

This could be caused by failure to run the link-time script, or running the transform twice. A warning would indeed be welcome, as this should never happen.

Quoted 2 lines

Warning? No. ERROR. Something very bad could be happening here. We have covered this before.

Heh, not this exactly. We covered run time errors, this is build time errors.

But I agree, it should error. The mcount code errors on problems, this should too.

Quoted 48 lines · * Steven Rostedt ([email protected]) wrote:

From: Steven Rostedt <[email protected]>

Have the jump labels add a "jmp" in the assembly instead of a default nop. This will cause the assembler to put in either a 2 byte or 5 byte jmp depending on where the target lable is.

Then at compile time, the update_jump_label code will replace the jmps with either 2 or 5 byte nops.

On boot up, the code can be examined to see if the jump label uses either a 2 or 5 byte nop and replace it.

By allowing the jump labels to be 2 bytes, it speeds up the nops, not only 2 byte nops are faster than 5 byte nops, but also because it saves on cache foot print.

text data bss dec hex filename
13403667 3666856 2998272 20068795 13239bb ../nobackup/mxtest/vmlinux-old 13398536 3666856 2998272 20063664 13225b0 ../nobackup/mxtest/vmlinux-new

Converting the current v3.2 trace points saved 5,131 bytes. As more places use jump labels, this will have a bigger savings.

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/x86/Kconfig                  |    1 +
 arch/x86/include/asm/jump_label.h |    2 +-
 arch/x86/kernel/jump_label.c      |   86 ++++++++++++++++++++++++++++++++++---
 3 files changed, 81 insertions(+), 8 deletions(-)

arch/x86/Kconfig +1 -0

index efb4294..b5004c1 100644
@@ -61,6 +61,7 @@ config X86
 	select HAVE_ARCH_KMEMCHECK
 	select HAVE_USER_RETURN_NOTIFIER
 	select HAVE_ARCH_JUMP_LABEL
+	select HAVE_BUILD_TIME_JUMP_LABEL
 	select HAVE_TEXT_POKE_SMP
 	select HAVE_GENERIC_HARDIRQS
 	select HAVE_SPARSE_IRQ

arch/x86/include/asm/jump_label.h +0 -0

index a32b18c..872b3e1 100644
@@ -14,7 +14,7 @@

arch/x86/include/asm/jump_label.h line 17

Quoted 5 lines

arch/x86/include/asm/jump_label.h +1 -1

 static __always_inline bool arch_static_branch(struct jump_label_key *key)
 {
 	asm goto("1:"
-		JUMP_LABEL_INITIAL_NOP
+		"jmp %l[l_yes]\n"

Is it possible that the compiler choose a jump that is not 2 or 5-byte ? e.g. a jmp rel16 (e9 opcode) on 32-bit x86, or any of the other

instruction listed under the JMP-Jump instruction in the Intel insn manual ?

Quoted 38 lines

arch/x86/include/asm/jump_label.h +0 -0

 		".pushsection __jump_table,  \"aw\" \n\t"
 		_ASM_ALIGN "\n\t"
 		_ASM_PTR "1b, %l[l_yes], %c0 \n\t"

arch/x86/kernel/jump_label.c +16 -1

index ea9d5f2f..d5b84de 100644
@@ -16,12 +16,27 @@
 
 #ifdef HAVE_JUMP_LABEL
 
+static unsigned char nop_short[] = { P6_NOP2 };
+
+/* These are the nops added at compile time */
+#ifdef CONFIG_X86_32
+static unsigned char default_nop[5] = { 0x3e, 0x8d, 0x74, 0x26, 0x00 };
+#else
+static unsigned char default_nop[5] = { 0x0f, 0x1f, 0x44, 0x00, 0x00 };
+#endif
+
+static int update_nops;
+
 union jump_code_union {
 	char code[JUMP_LABEL_NOP_SIZE];
 	struct {
 		char jump;
 		int offset;
-	} __attribute__((packed));
+	} __packed;
+	struct {
+		char jump_short;
+		char offset_short;
+	} __packed;
 };
 
 static void __jump_label_transform(struct jump_entry *entry,

arch/x86/kernel/jump_label.c static void __jump_label_transform(struct jump_entry *entry, · line 51

Quoted 8 lines

arch/x86/kernel/jump_label.c +5 -0

@@ -29,20 +44,70 @@ static void __jump_label_transform(struct jump_entry *entry,
 				   void *(*poker)(void *, const void *, size_t))
 {
 	union jump_code_union code;
+	unsigned char nop;
+	unsigned char op;
+	unsigned size;
+	void *ip = (void *)entry->code;
+	void *ideal = (void *)ideal_nops[NOP_ATOMIC5];

"void *" should possibly be "unsigned char *" here to respect the nop place-holder typing.

Quoted 5 lines

arch/x86/kernel/jump_label.c +4 -0

+
+	/* Use probe_kernel_read()? */
+	op = *(unsigned char *)ip;
+	nop = ideal_nops[NOP_ATOMIC5][0];
 

arch/x86/kernel/jump_label.c static void __jump_label_transform(struct jump_entry *entry, · line 59

Quoted 8 lines

arch/x86/kernel/jump_label.c +2 -5

 	if (type == JUMP_LABEL_ENABLE) {
-		code.jump = 0xe9;
-		code.offset = entry->target -
-				(entry->code + JUMP_LABEL_NOP_SIZE);
-	} else
-		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
+		if (op == 0xe9 || op == 0xeb)
+			/* Already enabled. Warn? */

This could be caused by failure to run the link-time script, or running the transform twice. A warning would indeed be welcome, as this should never happen.

Quoted 3 lines

Is it possible that the compiler choose a jump that is not 2 or 5-byte ? e.g. a jmp rel16 (e9 opcode) on 32-bit x86, or any of the other

Isn't e9 the 5 byte opcode?