Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1714813 > unrolled thread

[PATCH v2 0/6] add support for relative references in special sections

Started byArd Biesheuvel <ard.biesheuvel@linaro.org>
First post2017-08-18 13:30 +0200
Last post2017-08-18 16:00 +0200
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/6] add support for relative references in special sections Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-08-18 13:30 +0200
    [PATCH v2 6/6] kernel: tracepoints: add support for relative references Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-08-18 13:30 +0200
      Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative references Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-08-18 13:50 +0200
        Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative  references Steven Rostedt <rostedt@goodmis.org> - 2017-08-18 15:50 +0200
          Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative references Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-08-18 15:50 +0200
            Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative references Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-08-18 16:00 +0200
            Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative  references Steven Rostedt <rostedt@goodmis.org> - 2017-08-18 16:00 +0200

#1714813 — [PATCH v2 0/6] add support for relative references in special sections

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-08-18 13:30 +0200
Subject[PATCH v2 0/6] add support for relative references in special sections
Message-ID<ufNDr-30g-3@gated-at.bofh.it>
This adds support for emitting special sections such as initcall arrays,
PCI fixups and tracepoints as relative references rather than absolute
references. This reduces the size by 50% on 64-bit architectures, but
more importantly, it removes the need for carrying relocation metadata
for these sections in relocatables kernels (e.g., for KASLR) that need
to fix up these absolute references at boot time. On arm64, this reduces
the vmlinux footprint of such a reference by 8x (8 byte absolute reference
+ 24 byte RELA entry vs 4 byte relative reference)

Patch #2 was sent out before as a single patch. This series supersedes
the previous submission. This version makes relative ksymtab entries
dependent on the new Kconfig symbol HAVE_ARCH_PREL32_RELOCATIONS rather
than trying to infer from kbuild test robot replies for which architectures
it should be blacklisted.

Patch #1 introduces the new Kconfig symbol HAVE_ARCH_PREL32_RELOCATIONS,
and sets it for the main architectures that are expected to benefit the
most from this feature, i.e., 64-bit architectures, and ones that use
runtime relocation.

Patch #3 fixes several instances where the trailing semicolon is missing
from an initcall() definition. (Needed for patch #4)

Patches #4 - #6 implement relative references for initcallls, PCI fixups
and tracepoints, respectively, all of which produce sections with order
~1000 entries on an arm64 defconfig kernel with tracing enabled. This
means we save about 28 KB of vmlinux space for each of these patches.

For the arm64 kernel, all patches combined reduce the size of vmlinux
by about 300 KB (with KASLR enabled).

Changes since v1:
- Remove checkpatch errors to the extent feasible: in some cases, this
  involves moving extern declarations into C files, and switching to
  struct definitions rather than typedefs. Some errors are impossible
  to fix: please find the remaining ones after the diffstat.
- Used 'int' instead if 'signed int' for the various offset fields: there
  is no ambiguity between architectures regarding its signedness (unlike
  'char')
- Refactor the different patches to be more uniform in the way they define
  the section entry type and accessors in the .h file, and avoid the need to
  add #ifdefs to the C code.

Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Thomas Garnier <thgarnie@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Morris <james.l.morris@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Joe Perches <joe@perches.com>
Cc: Nicolas Pitre <nico@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Jessica Yu <jeyu@kernel.org>

Ard Biesheuvel (6):
  arch: enable relative relocations for arm64, power, x86, s390 and x86
  module: use relative references for __ksymtab entries
  treewide: add missing trailing semicolons to initcall() invocations
  init: allow initcall tables to be emitted using relative references
  drivers: pci: add support for relative addressing in quirk tables
  kernel: tracepoints: add support for relative references

 arch/Kconfig                             | 10 +++
 arch/arm64/Kconfig                       |  1 +
 arch/arm64/kernel/perf_event.c           |  2 +-
 arch/arm64/kernel/vmlinux.lds.S          |  2 +-
 arch/powerpc/Kconfig                     |  1 +
 arch/s390/Kconfig                        |  1 +
 arch/s390/hypfs/inode.c                  |  2 +-
 arch/x86/Kconfig                         |  1 +
 arch/x86/events/intel/core.c             |  2 +-
 arch/x86/include/asm/Kbuild              |  1 +
 arch/x86/include/asm/export.h            |  4 --
 drivers/net/cris/eth_v10.c               |  2 +-
 drivers/net/dsa/dsa_loop_bdinfo.c        |  2 +-
 drivers/parport/procfs.c                 |  4 +-
 drivers/pci/quirks.c                     |  6 +-
 drivers/perf/arm_pmu_acpi.c              |  2 +-
 drivers/platform/x86/intel_turbo_max_3.c |  2 +-
 fs/hugetlbfs/inode.c                     |  2 +-
 include/asm-generic/export.h             | 12 +++-
 include/linux/compiler.h                 | 11 ++++
 include/linux/export.h                   | 68 ++++++++++++++++----
 include/linux/init.h                     | 64 ++++++++++++++----
 include/linux/pci.h                      | 32 +++++++++
 include/linux/tracepoint.h               | 42 ++++++++++--
 init/main.c                              | 22 ++-----
 kernel/module.c                          | 14 ++--
 kernel/printk/printk.c                   |  4 +-
 kernel/tracepoint.c                      |  7 +-
 mm/compaction.c                          |  2 +-
 mm/oom_kill.c                            |  2 +-
 mm/page_alloc.c                          |  2 +-
 mm/page_owner.c                          |  2 +-
 net/core/lwt_bpf.c                       |  2 +-
 net/sched/sch_blackhole.c                |  2 +-
 security/security.c                      |  6 +-
 sound/core/seq_device.c                  |  4 +-
 36 files changed, 256 insertions(+), 89 deletions(-)
 delete mode 100644 arch/x86/include/asm/export.h

The following checkpatch errors cannot be fixed without breaking the
code, because inline asm at file scope cannot use parens or do-while
loops:

  ERROR: Macros with complex values should be enclosed in parentheses
  #201: FILE: include/linux/export.h:64:
  +#define __KSYMTAB_ENTRY(sym, sec)					\
  +	__ADDRESSABLE(sym)						\
  +	asm("	.section \"___ksymtab" sec "+" #sym "\", \"a\"\n"	\
  +	    "	.balign	8\n"						\
  +	    VMLINUX_SYMBOL_STR(__ksymtab_##sym) ":\n"			\
  +	    "	.long "	VMLINUX_SYMBOL_STR(sym) "- .\n"			\
  +	    "	.long "	VMLINUX_SYMBOL_STR(__kstrtab_##sym) "- .\n"	\
  +	    "	.previous\n")

  ERROR: Macros with multiple statements should be enclosed in a do - while loop
  #284: FILE: include/linux/init.h:162:
  +#define ___define_initcall(fn, id, __sec)		\
  +	__ADDRESSABLE(fn)				\
  +	asm(".section	\"" #__sec ".init\", \"a\"\n"	\
  +	"__initcall_" #fn #id ":\n"			\
  +	    ".long "	VMLINUX_SYMBOL_STR(fn) " - .\n"	\
  +	    ".previous\n");

  ERROR: Macros with complex values should be enclosed in parentheses
  #375: FILE: include/linux/pci.h:1757:
  +#define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class,	\
  +				    class_shift, hook)			\
  +	__ADDRESSABLE(hook)						\
  +	asm(".section "	#sec ", \"a\"\n"				\
  +	    ".balign	16\n"						\
  +	    ".short "	#vendor ", " #device "\n"			\
  +	    ".long "	#class ", " #class_shift "\n"			\
  +	    ".long "	VMLINUX_SYMBOL_STR(hook) " - .\n"		\
  +	    ".previous\n")

The following errors: 

  ERROR: space required after that ',' (ctx:VxO)
  #446: FILE: include/linux/init.h:250:
  +#define console_initcall(fn)	___define_initcall(fn,, .con_initcall)
                               	                     ^
  ERROR: space required after that ',' (ctx:VxO)
  #447: FILE: include/linux/init.h:251:
  +#define security_initcall(fn)	___define_initcall(fn,, .security_initcall)
                              	                     ^
cannot be fixed because attempting so results in:

  ERROR: space prohibited before that ',' (ctx:WxW)
  #349: FILE: include/linux/init.h:250:
  +#define console_initcall(fn)	___define_initcall(fn, , .con_initcall)
                             	                       ^
  ERROR: space prohibited before that ',' (ctx:WxW)
  #350: FILE: include/linux/init.h:251:
  +#define security_initcall(fn)	___define_initcall(fn, , .security_initcall)
                              	                       ^

[toc] | [next] | [standalone]

#1714814 — [PATCH v2 6/6] kernel: tracepoints: add support for relative references

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-08-18 13:30 +0200
Subject[PATCH v2 6/6] kernel: tracepoints: add support for relative references
Message-ID<ufNDt-30g-23@gated-at.bofh.it>
In reply to#1714813
To avoid the need for relocating absolute references to tracepoint
structures at boot time when running relocatable kernels (which may
take a disproportionate amount of space), add the option to emit
these tables as relative references instead.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 include/linux/tracepoint.h | 42 ++++++++++++++++++--
 kernel/tracepoint.c        |  7 +---
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index a26ffbe09e71..68701821933a 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -228,6 +228,42 @@ extern void syscall_unregfunc(void);
 		return static_key_false(&__tracepoint_##name.key);	\
 	}
 
+#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
+#define __TRACEPOINT_ENTRY(name)					\
+	asm("	.section \"__tracepoints_ptrs\", \"a\"\n"		\
+	    "	.balign 4\n"						\
+	    "	.long " VMLINUX_SYMBOL_STR(__tracepoint_##name) " - .\n"\
+	    "	.previous\n")
+
+struct tracepoint_entry_t {
+	int tp_offset;
+};
+
+static inline
+struct tracepoint *tracepoint_from_entry(const struct tracepoint_entry_t *ent)
+{
+	return (struct tracepoint *)((unsigned long)ent + ent->tp_offset);
+}
+#else
+#define __TRACEPOINT_ENTRY(name)					 \
+	static struct tracepoint * const __tracepoint_ptr_##name __used	 \
+	__attribute__((section("__tracepoints_ptrs"))) =		 \
+		&__tracepoint_##name
+
+struct tracepoint_entry_t {
+	struct tracepoint *tp;
+};
+
+static inline
+struct tracepoint *tracepoint_from_entry(const struct tracepoint_entry_t *ent)
+{
+	return ent->tp;
+}
+#endif
+
+extern struct tracepoint_entry_t const __start___tracepoints_ptrs[];
+extern struct tracepoint_entry_t const __stop___tracepoints_ptrs[];
+
 /*
  * We have no guarantee that gcc and the linker won't up-align the tracepoint
  * structures, so we create an array of pointers that will be used for iteration
@@ -237,11 +273,9 @@ extern void syscall_unregfunc(void);
 	static const char __tpstrtab_##name[]				 \
 	__attribute__((section("__tracepoints_strings"))) = #name;	 \
 	struct tracepoint __tracepoint_##name				 \
-	__attribute__((section("__tracepoints"))) =			 \
+	__attribute__((section("__tracepoints"))) __used =		 \
 		{ __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
-	static struct tracepoint * const __tracepoint_ptr_##name __used	 \
-	__attribute__((section("__tracepoints_ptrs"))) =		 \
-		&__tracepoint_##name;
+	__TRACEPOINT_ENTRY(name);
 
 #define DEFINE_TRACE(name)						\
 	DEFINE_TRACE_FN(name, NULL, NULL);
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 685c50ae6300..21bc41454fd6 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -28,9 +28,6 @@
 #include <linux/sched/task.h>
 #include <linux/static_key.h>
 
-extern struct tracepoint * const __start___tracepoints_ptrs[];
-extern struct tracepoint * const __stop___tracepoints_ptrs[];
-
 /* Set to 1 to enable tracepoint debug output */
 static const int tracepoint_debug;
 
@@ -508,12 +505,12 @@ static void for_each_tracepoint_range(struct tracepoint * const *begin,
 		void (*fct)(struct tracepoint *tp, void *priv),
 		void *priv)
 {
-	struct tracepoint * const *iter;
+	struct tracepoint_entry_t const *iter;
 
 	if (!begin)
 		return;
 	for (iter = begin; iter < end; iter++)
-		fct(*iter, priv);
+		fct(tracepoint_from_entry(iter), priv);
 }
 
 /**
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1714835 — Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative references

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-08-18 13:50 +0200
SubjectRe: [PATCH v2 6/6] kernel: tracepoints: add support for relative references
Message-ID<ufNWO-37L-21@gated-at.bofh.it>
In reply to#1714814
On 18 August 2017 at 12:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> To avoid the need for relocating absolute references to tracepoint
> structures at boot time when running relocatable kernels (which may
> take a disproportionate amount of space), add the option to emit
> these tables as relative references instead.
>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  include/linux/tracepoint.h | 42 ++++++++++++++++++--
>  kernel/tracepoint.c        |  7 +---
>  2 files changed, 40 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index a26ffbe09e71..68701821933a 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -228,6 +228,42 @@ extern void syscall_unregfunc(void);
>                 return static_key_false(&__tracepoint_##name.key);      \
>         }
>
> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +#define __TRACEPOINT_ENTRY(name)                                       \
> +       asm("   .section \"__tracepoints_ptrs\", \"a\"\n"               \
> +           "   .balign 4\n"                                            \
> +           "   .long " VMLINUX_SYMBOL_STR(__tracepoint_##name) " - .\n"\
> +           "   .previous\n")
> +
> +struct tracepoint_entry_t {
> +       int tp_offset;
> +};
> +
> +static inline
> +struct tracepoint *tracepoint_from_entry(const struct tracepoint_entry_t *ent)
> +{
> +       return (struct tracepoint *)((unsigned long)ent + ent->tp_offset);
> +}
> +#else
> +#define __TRACEPOINT_ENTRY(name)                                        \
> +       static struct tracepoint * const __tracepoint_ptr_##name __used  \
> +       __attribute__((section("__tracepoints_ptrs"))) =                 \
> +               &__tracepoint_##name
> +
> +struct tracepoint_entry_t {
> +       struct tracepoint *tp;
> +};
> +
> +static inline
> +struct tracepoint *tracepoint_from_entry(const struct tracepoint_entry_t *ent)
> +{
> +       return ent->tp;
> +}
> +#endif
> +
> +extern struct tracepoint_entry_t const __start___tracepoints_ptrs[];
> +extern struct tracepoint_entry_t const __stop___tracepoints_ptrs[];
> +

It appears the stuff above needs to be move inside the double-include
guard (which oddly enough does not cover the entire file)

>  /*
>   * We have no guarantee that gcc and the linker won't up-align the tracepoint
>   * structures, so we create an array of pointers that will be used for iteration
> @@ -237,11 +273,9 @@ extern void syscall_unregfunc(void);
>         static const char __tpstrtab_##name[]                            \
>         __attribute__((section("__tracepoints_strings"))) = #name;       \
>         struct tracepoint __tracepoint_##name                            \
> -       __attribute__((section("__tracepoints"))) =                      \
> +       __attribute__((section("__tracepoints"))) __used =               \
>                 { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
> -       static struct tracepoint * const __tracepoint_ptr_##name __used  \
> -       __attribute__((section("__tracepoints_ptrs"))) =                 \
> -               &__tracepoint_##name;
> +       __TRACEPOINT_ENTRY(name);
>
>  #define DEFINE_TRACE(name)                                             \
>         DEFINE_TRACE_FN(name, NULL, NULL);
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 685c50ae6300..21bc41454fd6 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -28,9 +28,6 @@
>  #include <linux/sched/task.h>
>  #include <linux/static_key.h>
>
> -extern struct tracepoint * const __start___tracepoints_ptrs[];
> -extern struct tracepoint * const __stop___tracepoints_ptrs[];
> -
>  /* Set to 1 to enable tracepoint debug output */
>  static const int tracepoint_debug;
>
> @@ -508,12 +505,12 @@ static void for_each_tracepoint_range(struct tracepoint * const *begin,
>                 void (*fct)(struct tracepoint *tp, void *priv),
>                 void *priv)
>  {
> -       struct tracepoint * const *iter;
> +       struct tracepoint_entry_t const *iter;
>
>         if (!begin)
>                 return;
>         for (iter = begin; iter < end; iter++)
> -               fct(*iter, priv);
> +               fct(tracepoint_from_entry(iter), priv);
>  }
>
>  /**
> --
> 2.11.0
>

[toc] | [prev] | [next] | [standalone]

#1715028 — Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative references

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-08-18 15:50 +0200
SubjectRe: [PATCH v2 6/6] kernel: tracepoints: add support for relative references
Message-ID<ufPOW-4q2-19@gated-at.bofh.it>
In reply to#1714835
On Fri, 18 Aug 2017 12:44:17 +0100
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 18 August 2017 at 12:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > To avoid the need for relocating absolute references to tracepoint
> > structures at boot time when running relocatable kernels (which may
> > take a disproportionate amount of space), add the option to emit
> > these tables as relative references instead.
> >
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  include/linux/tracepoint.h | 42 ++++++++++++++++++--
> >  kernel/tracepoint.c        |  7 +---
> >  2 files changed, 40 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> > index a26ffbe09e71..68701821933a 100644
> > --- a/include/linux/tracepoint.h
> > +++ b/include/linux/tracepoint.h
> > @@ -228,6 +228,42 @@ extern void syscall_unregfunc(void);
> >                 return static_key_false(&__tracepoint_##name.key);      \
> >         }
> >
> > +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> > +#define __TRACEPOINT_ENTRY(name)                                       \
> > +       asm("   .section \"__tracepoints_ptrs\", \"a\"\n"               \
> > +           "   .balign 4\n"                                            \
> > +           "   .long " VMLINUX_SYMBOL_STR(__tracepoint_##name) " - .\n"\
> > +           "   .previous\n")
> > +
> > +struct tracepoint_entry_t {
> > +       int tp_offset;
> > +};
> > +
> > +static inline
> > +struct tracepoint *tracepoint_from_entry(const struct tracepoint_entry_t *ent)
> > +{
> > +       return (struct tracepoint *)((unsigned long)ent + ent->tp_offset);
> > +}
> > +#else
> > +#define __TRACEPOINT_ENTRY(name)                                        \
> > +       static struct tracepoint * const __tracepoint_ptr_##name __used  \
> > +       __attribute__((section("__tracepoints_ptrs"))) =                 \
> > +               &__tracepoint_##name
> > +
> > +struct tracepoint_entry_t {
> > +       struct tracepoint *tp;
> > +};
> > +
> > +static inline
> > +struct tracepoint *tracepoint_from_entry(const struct tracepoint_entry_t *ent)
> > +{
> > +       return ent->tp;
> > +}
> > +#endif
> > +
> > +extern struct tracepoint_entry_t const __start___tracepoints_ptrs[];
> > +extern struct tracepoint_entry_t const __stop___tracepoints_ptrs[];
> > +  
> 
> It appears the stuff above needs to be move inside the double-include
> guard (which oddly enough does not cover the entire file)

Why was this moved to the header file? To fulfill some checkpatch
warning?

-- Steve

> 
> >  /*
> >   * We have no guarantee that gcc and the linker won't up-align the tracepoint
> >   * structures, so we create an array of pointers that will be used for iteration
> > @@ -237,11 +273,9 @@ extern void syscall_unregfunc(void);
> >         static const char __tpstrtab_##name[]                            \
> >         __attribute__((section("__tracepoints_strings"))) = #name;       \
> >         struct tracepoint __tracepoint_##name                            \
> > -       __attribute__((section("__tracepoints"))) =                      \
> > +       __attribute__((section("__tracepoints"))) __used =               \
> >                 { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
> > -       static struct tracepoint * const __tracepoint_ptr_##name __used  \
> > -       __attribute__((section("__tracepoints_ptrs"))) =                 \
> > -               &__tracepoint_##name;
> > +       __TRACEPOINT_ENTRY(name);
> >
> >  #define DEFINE_TRACE(name)                                             \
> >         DEFINE_TRACE_FN(name, NULL, NULL);
> > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> > index 685c50ae6300..21bc41454fd6 100644
> > --- a/kernel/tracepoint.c
> > +++ b/kernel/tracepoint.c
> > @@ -28,9 +28,6 @@
> >  #include <linux/sched/task.h>
> >  #include <linux/static_key.h>
> >
> > -extern struct tracepoint * const __start___tracepoints_ptrs[];
> > -extern struct tracepoint * const __stop___tracepoints_ptrs[];
> > -
> >  /* Set to 1 to enable tracepoint debug output */
> >  static const int tracepoint_debug;
> >
> > @@ -508,12 +505,12 @@ static void for_each_tracepoint_range(struct tracepoint * const *begin,
> >                 void (*fct)(struct tracepoint *tp, void *priv),
> >                 void *priv)
> >  {
> > -       struct tracepoint * const *iter;
> > +       struct tracepoint_entry_t const *iter;
> >
> >         if (!begin)
> >                 return;
> >         for (iter = begin; iter < end; iter++)
> > -               fct(*iter, priv);
> > +               fct(tracepoint_from_entry(iter), priv);
> >  }
> >
> >  /**
> > --
> > 2.11.0
> >

[toc] | [prev] | [next] | [standalone]

#1715045 — Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative references

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-08-18 15:50 +0200
SubjectRe: [PATCH v2 6/6] kernel: tracepoints: add support for relative references
Message-ID<ufPOY-4q2-55@gated-at.bofh.it>
In reply to#1715028
On 18 August 2017 at 14:43, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 18 Aug 2017 12:44:17 +0100
> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> On 18 August 2017 at 12:26, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > To avoid the need for relocating absolute references to tracepoint
>> > structures at boot time when running relocatable kernels (which may
>> > take a disproportionate amount of space), add the option to emit
>> > these tables as relative references instead.
>> >
>> > Cc: Steven Rostedt <rostedt@goodmis.org>
>> > Cc: Ingo Molnar <mingo@redhat.com>
>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > ---
>> >  include/linux/tracepoint.h | 42 ++++++++++++++++++--
>> >  kernel/tracepoint.c        |  7 +---
>> >  2 files changed, 40 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
>> > index a26ffbe09e71..68701821933a 100644
>> > --- a/include/linux/tracepoint.h
>> > +++ b/include/linux/tracepoint.h
>> > @@ -228,6 +228,42 @@ extern void syscall_unregfunc(void);
>> >                 return static_key_false(&__tracepoint_##name.key);      \
>> >         }
>> >
>> > +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>> > +#define __TRACEPOINT_ENTRY(name)                                       \
>> > +       asm("   .section \"__tracepoints_ptrs\", \"a\"\n"               \
>> > +           "   .balign 4\n"                                            \
>> > +           "   .long " VMLINUX_SYMBOL_STR(__tracepoint_##name) " - .\n"\
>> > +           "   .previous\n")
>> > +
>> > +struct tracepoint_entry_t {
>> > +       int tp_offset;
>> > +};
>> > +
>> > +static inline
>> > +struct tracepoint *tracepoint_from_entry(const struct tracepoint_entry_t *ent)
>> > +{
>> > +       return (struct tracepoint *)((unsigned long)ent + ent->tp_offset);
>> > +}
>> > +#else
>> > +#define __TRACEPOINT_ENTRY(name)                                        \
>> > +       static struct tracepoint * const __tracepoint_ptr_##name __used  \
>> > +       __attribute__((section("__tracepoints_ptrs"))) =                 \
>> > +               &__tracepoint_##name
>> > +
>> > +struct tracepoint_entry_t {
>> > +       struct tracepoint *tp;
>> > +};
>> > +
>> > +static inline
>> > +struct tracepoint *tracepoint_from_entry(const struct tracepoint_entry_t *ent)
>> > +{
>> > +       return ent->tp;
>> > +}
>> > +#endif
>> > +
>> > +extern struct tracepoint_entry_t const __start___tracepoints_ptrs[];
>> > +extern struct tracepoint_entry_t const __stop___tracepoints_ptrs[];
>> > +
>>
>> It appears the stuff above needs to be move inside the double-include
>> guard (which oddly enough does not cover the entire file)
>
> Why was this moved to the header file? To fulfill some checkpatch
> warning?
>

Yes.

[toc] | [prev] | [next] | [standalone]

#1715086 — Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative references

FromArd Biesheuvel <ard.biesheuvel@linaro.org>
Date2017-08-18 16:00 +0200
SubjectRe: [PATCH v2 6/6] kernel: tracepoints: add support for relative references
Message-ID<ufPYD-4vA-59@gated-at.bofh.it>
In reply to#1715045
On 18 August 2017 at 14:52, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 18 Aug 2017 14:44:15 +0100
> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> >> It appears the stuff above needs to be move inside the double-include
>> >> guard (which oddly enough does not cover the entire file)
>> >
>> > Why was this moved to the header file? To fulfill some checkpatch
>> > warning?
>> >
>>
>> Yes.
>
> My preference is to ignore that checkpatch warning. The section
> variables are created by linker magic, and not normal "extern"
> variables. They are only used in one location, and I like to keep them
> where they are used, and not be something other places might think they
> can be used. In other words, keep them by the C code, and out of
> headers.
>
> Tracepoints and linker/asm work always triggers a lot of bogus
> checkpatch warnings. Which is unfortunate. :-/
>

Actually, I couldn't agree more. I will backpedal on the checkpatch
appeasement in v3 in general.

Thanks,
Ard.

[toc] | [prev] | [next] | [standalone]

#1715094 — Re: [PATCH v2 6/6] kernel: tracepoints: add support for relative references

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-08-18 16:00 +0200
SubjectRe: [PATCH v2 6/6] kernel: tracepoints: add support for relative references
Message-ID<ufPYD-4vA-61@gated-at.bofh.it>
In reply to#1715045
On Fri, 18 Aug 2017 14:44:15 +0100
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> >> It appears the stuff above needs to be move inside the double-include
> >> guard (which oddly enough does not cover the entire file)  
> >
> > Why was this moved to the header file? To fulfill some checkpatch
> > warning?
> >  
> 
> Yes.

My preference is to ignore that checkpatch warning. The section
variables are created by linker magic, and not normal "extern"
variables. They are only used in one location, and I like to keep them
where they are used, and not be something other places might think they
can be used. In other words, keep them by the C code, and out of
headers.

Tracepoints and linker/asm work always triggers a lot of bogus
checkpatch warnings. Which is unfortunate. :-/

Thanks!

-- Steve

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web