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

Groups > linux.kernel > #1338193 > unrolled thread

[RFC v2 0/7] linux: add linker tables

Started by"Luis R. Rodriguez" <mcgrof@kernel.org>
First post2016-02-19 14:50 +0100
Last post2016-02-19 22:20 +0100
Articles 14 on this page of 34 — 9 participants

Back to article view | Back to linux.kernel

Contents

  [RFC v2 0/7] linux: add linker tables "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 14:50 +0100
    [RFC v2 3/7] firmware: port built-in section to linker table "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 14:50 +0100
      Re: [RFC v2 3/7] firmware: port built-in section to linker table David Woodhouse <dwmw2@infradead.org> - 2016-02-29 11:20 +0100
        Re: [RFC v2 3/7] firmware: port built-in section to linker table "Luis R. Rodriguez" <mcgrof@suse.com> - 2016-02-29 20:00 +0100
        Re: [RFC v2 3/7] firmware: port built-in section to linker table James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-03-01 17:20 +0100
          Re: [RFC v2 3/7] firmware: port built-in section to linker table "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-03-01 19:00 +0100
    [RFC v2 6/7] dynamic_debug: port to use linker tables "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 14:50 +0100
    [RFC v2 5/7] jump_label: port __jump_table to linker tables "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 14:50 +0100
    [RFC v2 1/7] sections.h: add sections header to collect all section info "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 14:50 +0100
      Re: [RFC v2 1/7] sections.h: add sections header to collect all  section info Greg KH <gregkh@linuxfoundation.org> - 2016-02-19 17:30 +0100
        Re: [RFC v2 1/7] sections.h: add sections header to collect all  section info "Luis R. Rodriguez" <mcgrof@suse.com> - 2016-02-19 21:10 +0100
          Re: [RFC v2 1/7] sections.h: add sections header to collect all  section info Greg KH <gregkh@linuxfoundation.org> - 2016-02-19 22:30 +0100
            Re: [RFC v2 1/7] sections.h: add sections header to collect all  section info "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 23:10 +0100
    [RFC v2 4/7] asm/sections: add a generic push_section_tbl() "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 14:50 +0100
      Re: [RFC v2 4/7] asm/sections: add a generic push_section_tbl() "H. Peter Anvin" <hpa@zytor.com> - 2016-02-19 21:40 +0100
        Re: [RFC v2 4/7] asm/sections: add a generic push_section_tbl() "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 22:10 +0100
          Re: [RFC v2 4/7] asm/sections: add a generic push_section_tbl() "H. Peter Anvin" <hpa@zytor.com> - 2016-02-22 04:00 +0100
            Re: [RFC v2 4/7] asm/sections: add a generic push_section_tbl() Heiko Carstens <heiko.carstens@de.ibm.com> - 2016-02-26 16:00 +0100
    [RFC v2 7/7] kprobes: port to linker table "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 14:50 +0100
      Re: [RFC v2 7/7] kprobes: port to linker table Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-02-19 15:20 +0100
        Re: [RFC v2 7/7] kprobes: port to linker table "Luis R. Rodriguez" <mcgrof@suse.com> - 2016-02-19 16:00 +0100
      RE: [RFC v2 7/7] kprobes: port to linker table 平松雅巳 / HIRAMATU，MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2016-02-22 02:40 +0100
        Re: [Xen-devel] [RFC v2 7/7] kprobes: port to linker table "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-23 02:00 +0100
    Re: [RFC v2 0/7] linux: add linker tables "H. Peter Anvin" <hpa@zytor.com> - 2016-02-19 21:30 +0100
      Re: [RFC v2 0/7] linux: add linker tables "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 22:30 +0100
    Re: [RFC v2 2/7] tables.h: add linker table support "H. Peter Anvin" <hpa@zytor.com> - 2016-02-19 21:40 +0100
      Re: [RFC v2 2/7] tables.h: add linker table support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-19 22:50 +0100
        Re: [RFC v2 2/7] tables.h: add linker table support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-24 00:10 +0100
          Re: [RFC v2 2/7] tables.h: add linker table support "H. Peter Anvin" <hpa@zytor.com> - 2016-02-24 00:30 +0100
            Re: [RFC v2 2/7] tables.h: add linker table support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-24 00:40 +0100
              Re: [RFC v2 2/7] tables.h: add linker table support "H. Peter Anvin" <hpa@zytor.com> - 2016-02-24 01:20 +0100
                Re: [RFC v2 2/7] tables.h: add linker table support "Luis R. Rodriguez" <mcgrof@kernel.org> - 2016-02-24 02:00 +0100
    Re: [RFC v2 2/7] tables.h: add linker table support "H. Peter Anvin" <hpa@zytor.com> - 2016-02-19 21:40 +0100
      Re: [RFC v2 2/7] tables.h: add linker table support "Luis R. Rodriguez" <mcgrof@suse.com> - 2016-02-19 22:20 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1338265 — Re: [RFC v2 7/7] kprobes: port to linker table

From"Luis R. Rodriguez" <mcgrof@suse.com>
Date2016-02-19 16:00 +0100
SubjectRe: [RFC v2 7/7] kprobes: port to linker table
Message-ID<r3UHh-6xJ-27@gated-at.bofh.it>
In reply to#1338220
On Fri, Feb 19, 2016 at 02:15:11PM +0000, Russell King - ARM Linux wrote:
> On Fri, Feb 19, 2016 at 05:45:59AM -0800, Luis R. Rodriguez wrote:
> > kprobe makes use of two custom sections:
> > 
> > type       name                  begin                    end
> > init.data _kprobe_blacklist __start_kprobe_blacklist	__stop_kprobe_blacklist
> > text      .kprobes.text     __kprobes_text_start        __kprobes_text_end
> > 
> > Port these to the linker table generic solution. This lets
> > us remove all the custom kprobe section declarations on the
> > linker script.
> 
> I don't like this.  You create this "table" thing, which you use for
> tables, and then you go and use it for something else - for kprobes
> text which is not a table.  The kprobes text section is a section for
> text which is not kprobe-able.

No the thing about linker tables is its generalizing custom section solutions
we have scattered all over the place.

> So, it seems to be a complete mis-nomer to me.  Maybe you can explain
> the rationale here?

Sure, so each custom section solution requires modifying the linker script.
With linker tables we end up only with a few basic types of tables, a
read-only, text, data, init init_data, etc. A linker table belongs to one of
these. The linker table provides a means by which to describe explicitly
what type of section you are using for your solution but also allows us
to enable such custom solutions without furthering the custom linker
script.

  Luis

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

#1339048 — RE: [RFC v2 7/7] kprobes: port to linker table

From平松雅巳 / HIRAMATU，MASAMI <masami.hiramatsu.pt@hitachi.com>
Date2016-02-22 02:40 +0100
SubjectRE: [RFC v2 7/7] kprobes: port to linker table
Message-ID<r4NDH-6z9-1@gated-at.bofh.it>
In reply to#1338203
>From: Luis R. Rodriguez [mailto:mcgrof@kernel.org]
>
>kprobe makes use of two custom sections:
>
>type       name                  begin                    end
>init.data _kprobe_blacklist __start_kprobe_blacklist	__stop_kprobe_blacklist
>text      .kprobes.text     __kprobes_text_start        __kprobes_text_end
>
>Port these to the linker table generic solution. This lets
>us remove all the custom kprobe section declarations on the
>linker script.
>
>Tested with CONFIG_KPROBES_SANITY_TEST, it passes with:
>
>Kprobe smoke test: started
>Kprobe smoke test: passed successfully
>
>Then tested CONFIG_SAMPLE_KPROBES on do_fork, and the
>kprobe bites and kicks as expected. Lastly tried registering
>a kprobe on a kprobe blacklisted symbol (NOKPROBE_SYMBOL()),
>and confirms that fails to work.

Could you also check to run the testcases by using ftracetest as below?

$ cd tools/testing/selftests/ftrace/
$ sudo ./ftracetest

And I'm not sure about linker table. Is that possible to support
__kprobes prefix, which moves the functions into kprobes.text?
Actually, I'm on the way to replacing __kprobes to NOKPROBE_SYMBOL
macro, since NOKPROBE_SYMBOL() doesn't effect the kernel text itself.
On x86, it is already replaced (see commit 820aede0209a), and same
work should be done on other archs. So, could you hold this after
that? I think we should remove .kprobes.text first and move to 
linker table.

Thank you,


>
>Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
>---
> arch/arc/kernel/vmlinux.lds.S        |  1 -
> arch/arm/kernel/entry-armv.S         |  2 +-
> arch/arm/kernel/vmlinux.lds.S        |  1 -
> arch/avr32/kernel/entry-avr32b.S     |  4 ++--
> arch/avr32/kernel/vmlinux.lds.S      |  1 -
> arch/blackfin/kernel/vmlinux.lds.S   |  1 -
> arch/c6x/kernel/vmlinux.lds.S        |  1 -
> arch/hexagon/kernel/vmlinux.lds.S    |  1 -
> arch/ia64/kernel/jprobes.S           |  2 +-
> arch/ia64/kernel/vmlinux.lds.S       |  1 -
> arch/ia64/lib/flush.S                |  4 ++--
> arch/metag/kernel/vmlinux.lds.S      |  1 -
> arch/microblaze/kernel/vmlinux.lds.S |  1 -
> arch/mips/kernel/vmlinux.lds.S       |  1 -
> arch/mn10300/kernel/vmlinux.lds.S    |  1 -
> arch/nios2/kernel/vmlinux.lds.S      |  1 -
> arch/openrisc/kernel/vmlinux.lds.S   |  1 -
> arch/parisc/kernel/vmlinux.lds.S     |  1 -
> arch/powerpc/include/asm/ppc_asm.h   |  6 +++---
> arch/powerpc/kernel/vmlinux.lds.S    |  1 -
> arch/s390/kernel/entry.S             |  4 ++--
> arch/s390/kernel/kprobes.c           |  2 +-
> arch/s390/kernel/mcount.S            |  2 +-
> arch/s390/kernel/vmlinux.lds.S       |  1 -
> arch/score/kernel/vmlinux.lds.S      |  1 -
> arch/sh/kernel/vmlinux.lds.S         |  1 -
> arch/sparc/kernel/vmlinux.lds.S      |  1 -
> arch/sparc/mm/ultra.S                |  2 +-
> arch/tile/kernel/vmlinux.lds.S       |  1 -
> arch/x86/kernel/kprobes/core.c       | 10 ++++++----
> arch/x86/kernel/vmlinux.lds.S        |  1 -
> include/asm-generic/sections.h       |  4 ++--
> include/asm-generic/vmlinux.lds.h    | 16 ----------------
> include/linux/compiler.h             |  3 ++-
> include/linux/kprobes.h              |  7 +++++--
> kernel/kprobes.c                     | 16 +++++++---------
> scripts/mod/modpost.c                |  3 ++-
> scripts/recordmcount.c               |  3 ++-
> scripts/recordmcount.pl              |  2 +-
> 39 files changed, 41 insertions(+), 72 deletions(-)
>
>diff --git a/arch/arc/kernel/vmlinux.lds.S b/arch/arc/kernel/vmlinux.lds.S
>index 894e696bddaa..52f23df2b9b6 100644
>--- a/arch/arc/kernel/vmlinux.lds.S
>+++ b/arch/arc/kernel/vmlinux.lds.S
>@@ -98,7 +98,6 @@ SECTIONS
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		*(.fixup)
> 		*(.gnu.warning)
> 	}
>diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
>index 3ce377f7251f..4959a83833fd 100644
>--- a/arch/arm/kernel/entry-armv.S
>+++ b/arch/arm/kernel/entry-armv.S
>@@ -83,7 +83,7 @@
> 	.endm
>
> #ifdef CONFIG_KPROBES
>-	.section	.kprobes.text,"ax",%progbits
>+	.section	.text.tbl.kprobes.all,"ax",%progbits
> #else
> 	.text
> #endif
>diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>index 8b60fde5ce48..8de7921ac331 100644
>--- a/arch/arm/kernel/vmlinux.lds.S
>+++ b/arch/arm/kernel/vmlinux.lds.S
>@@ -108,7 +108,6 @@ SECTIONS
> 			TEXT_TEXT
> 			SCHED_TEXT
> 			LOCK_TEXT
>-			KPROBES_TEXT
> 			*(.gnu.warning)
> 			*(.glue_7)
> 			*(.glue_7t)
>diff --git a/arch/avr32/kernel/entry-avr32b.S b/arch/avr32/kernel/entry-avr32b.S
>index 7301f4806bbe..c0fe699d0a21 100644
>--- a/arch/avr32/kernel/entry-avr32b.S
>+++ b/arch/avr32/kernel/entry-avr32b.S
>@@ -605,7 +605,7 @@ fault_exit_work:
> 	brcc	fault_resume_user
> 	rjmp	enter_monitor_mode
>
>-	.section .kprobes.text, "ax", @progbits
>+	.section .text.tbl.kprobes.all, "ax", @progbits
> 	.type	handle_debug, @function
> handle_debug:
> 	sub	sp, 4		/* r12_orig */
>@@ -826,7 +826,7 @@ irq_level\level:
> 	IRQ_LEVEL 2
> 	IRQ_LEVEL 3
>
>-	.section .kprobes.text, "ax", @progbits
>+	.section .text.tbl.kprobes.all, "ax", @progbits
> 	.type	enter_monitor_mode, @function
> enter_monitor_mode:
> 	/*
>diff --git a/arch/avr32/kernel/vmlinux.lds.S b/arch/avr32/kernel/vmlinux.lds.S
>index a4589176bed5..bf4f3f1f9dbb 100644
>--- a/arch/avr32/kernel/vmlinux.lds.S
>+++ b/arch/avr32/kernel/vmlinux.lds.S
>@@ -49,7 +49,6 @@ SECTIONS
> 		_stext = .;
> 		*(.ex.text)
> 		*(.irq.text)
>-		KPROBES_TEXT
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>diff --git a/arch/blackfin/kernel/vmlinux.lds.S b/arch/blackfin/kernel/vmlinux.lds.S
>index c9eec84aa258..8a193fad4c5d 100644
>--- a/arch/blackfin/kernel/vmlinux.lds.S
>+++ b/arch/blackfin/kernel/vmlinux.lds.S
>@@ -35,7 +35,6 @@ SECTIONS
> #endif
> 		LOCK_TEXT
> 		IRQENTRY_TEXT
>-		KPROBES_TEXT
> #ifdef CONFIG_ROMKERNEL
> 		__sinittext = .;
> 		INIT_TEXT
>diff --git a/arch/c6x/kernel/vmlinux.lds.S b/arch/c6x/kernel/vmlinux.lds.S
>index 5a6e141d1641..a23bed62cec3 100644
>--- a/arch/c6x/kernel/vmlinux.lds.S
>+++ b/arch/c6x/kernel/vmlinux.lds.S
>@@ -72,7 +72,6 @@ SECTIONS
> 		SCHED_TEXT
> 		LOCK_TEXT
> 		IRQENTRY_TEXT
>-		KPROBES_TEXT
> 		*(.fixup)
> 		*(.gnu.warning)
> 	}
>diff --git a/arch/hexagon/kernel/vmlinux.lds.S b/arch/hexagon/kernel/vmlinux.lds.S
>index 5f268c1071b3..a358b699ec64 100644
>--- a/arch/hexagon/kernel/vmlinux.lds.S
>+++ b/arch/hexagon/kernel/vmlinux.lds.S
>@@ -51,7 +51,6 @@ SECTIONS
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		*(.fixup)
> 	}
> 	_etext = .;
>diff --git a/arch/ia64/kernel/jprobes.S b/arch/ia64/kernel/jprobes.S
>index f69389c7be1d..c6c23a3e92fb 100644
>--- a/arch/ia64/kernel/jprobes.S
>+++ b/arch/ia64/kernel/jprobes.S
>@@ -50,7 +50,7 @@
> 	/*
> 	 * void jprobe_break(void)
> 	 */
>-	.section .kprobes.text, "ax"
>+	.section .text.tbl.kprobes.all, "ax"
> ENTRY(jprobe_break)
> 	break.m __IA64_BREAK_JPROBE
> END(jprobe_break)
>diff --git a/arch/ia64/kernel/vmlinux.lds.S b/arch/ia64/kernel/vmlinux.lds.S
>index dc506b05ffbd..884f36d38989 100644
>--- a/arch/ia64/kernel/vmlinux.lds.S
>+++ b/arch/ia64/kernel/vmlinux.lds.S
>@@ -47,7 +47,6 @@ SECTIONS {
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		*(.gnu.linkonce.t*)
> 	}
>
>diff --git a/arch/ia64/lib/flush.S b/arch/ia64/lib/flush.S
>index 1d8c88860063..7f257810e858 100644
>--- a/arch/ia64/lib/flush.S
>+++ b/arch/ia64/lib/flush.S
>@@ -20,7 +20,7 @@
> 	 *
> 	 *	Note: "in0" and "in1" are preserved for debugging purposes.
> 	 */
>-	.section .kprobes.text,"ax"
>+	.section .text.tbl.kprobes.all,"ax"
> GLOBAL_ENTRY(flush_icache_range)
>
> 	.prologue
>@@ -72,7 +72,7 @@ END(flush_icache_range)
> 	 *
> 	 *	Note: "in0" and "in1" are preserved for debugging purposes.
> 	 */
>-	.section .kprobes.text,"ax"
>+	.section .text.tbl.kprobes.all,"ax"
> GLOBAL_ENTRY(clflush_cache_range)
>
> 	.prologue
>diff --git a/arch/metag/kernel/vmlinux.lds.S b/arch/metag/kernel/vmlinux.lds.S
>index e12055e88bfe..da1689c85bc8 100644
>--- a/arch/metag/kernel/vmlinux.lds.S
>+++ b/arch/metag/kernel/vmlinux.lds.S
>@@ -22,7 +22,6 @@ SECTIONS
> 	TEXT_TEXT
> 	SCHED_TEXT
> 	LOCK_TEXT
>-	KPROBES_TEXT
> 	IRQENTRY_TEXT
> 	*(.text.*)
> 	*(.gnu.warning)
>diff --git a/arch/microblaze/kernel/vmlinux.lds.S b/arch/microblaze/kernel/vmlinux.lds.S
>index be9488d69734..94c1c5574a77 100644
>--- a/arch/microblaze/kernel/vmlinux.lds.S
>+++ b/arch/microblaze/kernel/vmlinux.lds.S
>@@ -34,7 +34,6 @@ SECTIONS {
> 		EXIT_CALL
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		IRQENTRY_TEXT
> 		. = ALIGN (4) ;
> 		_etext = . ;
>diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
>index 0a93e83cd014..9183e9235651 100644
>--- a/arch/mips/kernel/vmlinux.lds.S
>+++ b/arch/mips/kernel/vmlinux.lds.S
>@@ -56,7 +56,6 @@ SECTIONS
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		IRQENTRY_TEXT
> 		*(.text.*)
> 		*(.fixup)
>diff --git a/arch/mn10300/kernel/vmlinux.lds.S b/arch/mn10300/kernel/vmlinux.lds.S
>index 13c4814c29f8..ead5efc01802 100644
>--- a/arch/mn10300/kernel/vmlinux.lds.S
>+++ b/arch/mn10300/kernel/vmlinux.lds.S
>@@ -31,7 +31,6 @@ SECTIONS
> 	TEXT_TEXT
> 	SCHED_TEXT
> 	LOCK_TEXT
>-	KPROBES_TEXT
> 	*(.fixup)
> 	*(.gnu.warning)
> 	} = 0xcb
>diff --git a/arch/nios2/kernel/vmlinux.lds.S b/arch/nios2/kernel/vmlinux.lds.S
>index 326fab40a9de..dc8a84183700 100644
>--- a/arch/nios2/kernel/vmlinux.lds.S
>+++ b/arch/nios2/kernel/vmlinux.lds.S
>@@ -39,7 +39,6 @@ SECTIONS
> 		SCHED_TEXT
> 		LOCK_TEXT
> 		IRQENTRY_TEXT
>-		KPROBES_TEXT
> 	} =0
> 	_etext = .;
>
>diff --git a/arch/openrisc/kernel/vmlinux.lds.S b/arch/openrisc/kernel/vmlinux.lds.S
>index 2d69a853b742..b010cb75edc4 100644
>--- a/arch/openrisc/kernel/vmlinux.lds.S
>+++ b/arch/openrisc/kernel/vmlinux.lds.S
>@@ -48,7 +48,6 @@ SECTIONS
> 	  TEXT_TEXT
> 	  SCHED_TEXT
> 	  LOCK_TEXT
>-	  KPROBES_TEXT
> 	  IRQENTRY_TEXT
> 	  *(.fixup)
> 	  *(.text.__*)
>diff --git a/arch/parisc/kernel/vmlinux.lds.S b/arch/parisc/kernel/vmlinux.lds.S
>index 308f29081d46..9d9dc4984465 100644
>--- a/arch/parisc/kernel/vmlinux.lds.S
>+++ b/arch/parisc/kernel/vmlinux.lds.S
>@@ -70,7 +70,6 @@ SECTIONS
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		IRQENTRY_TEXT
> 		*(.text.do_softirq)
> 		*(.text.sys_exit)
>diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
>index 499d9f89435a..4e36e86338f3 100644
>--- a/arch/powerpc/include/asm/ppc_asm.h
>+++ b/arch/powerpc/include/asm/ppc_asm.h
>@@ -218,7 +218,7 @@ name: \
> 	.localentry name,.-name
>
> #define _KPROBE(name) \
>-	.section ".kprobes.text","a"; \
>+	.section ".text.tbl.kprobes.all","a"; \
> 	.align 2 ; \
> 	.type name,@function; \
> 	.globl name; \
>@@ -248,7 +248,7 @@ GLUE(.,name):
> #define _GLOBAL_TOC(name) _GLOBAL(name)
>
> #define _KPROBE(name) \
>-	.section ".kprobes.text","a"; \
>+	.section ".text.tbl.kprobes.all","a"; \
> 	.align 2 ; \
> 	.globl name; \
> 	.globl GLUE(.,name); \
>@@ -280,7 +280,7 @@ n:
> #define _GLOBAL_TOC(name) _GLOBAL(name)
>
> #define _KPROBE(n)	\
>-	.section ".kprobes.text","a";	\
>+	.section ".text.tbl.kprobes.all","a";	\
> 	.globl	n;	\
> n:
>
>diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
>index d41fd0af8980..8779ae180783 100644
>--- a/arch/powerpc/kernel/vmlinux.lds.S
>+++ b/arch/powerpc/kernel/vmlinux.lds.S
>@@ -53,7 +53,6 @@ SECTIONS
> 		*(.text .fixup __ftr_alt_* .ref.text)
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		IRQENTRY_TEXT
>
> #ifdef CONFIG_PPC32
>diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
>index cd5a191381b9..b09aa9ab5a1f 100644
>--- a/arch/s390/kernel/entry.S
>+++ b/arch/s390/kernel/entry.S
>@@ -162,7 +162,7 @@ _PIF_WORK	= (_PIF_PER_TRAP)
> 		tm	off+\addr, \mask
> 	.endm
>
>-	.section .kprobes.text, "ax"
>+	.section .text.tbl.kprobes.all, "ax"
>
> /*
>  * Scheduler resume function, called by switch_to
>@@ -965,7 +965,7 @@ ENTRY(restart_int_handler)
> 	brc	2,2b
> 3:	j	3b
>
>-	.section .kprobes.text, "ax"
>+	.section .text.tbl.kprobes.all, "ax"
>
> #ifdef CONFIG_CHECK_STACK
> /*
>diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
>index 250f5972536a..376d4bd487c5 100644
>--- a/arch/s390/kernel/kprobes.c
>+++ b/arch/s390/kernel/kprobes.c
>@@ -288,7 +288,7 @@ static void kprobe_reenter_check(struct kprobe_ctlblk *kcb, struct kprobe *p)
> 	default:
> 		/*
> 		 * A kprobe on the code path to single step an instruction
>-		 * is a BUG. The code path resides in the .kprobes.text
>+		 * is a BUG. The code path resides in the .text.tbl.kprobes.all
> 		 * section and is executed with interrupts disabled.
> 		 */
> 		printk(KERN_EMERG "Invalid kprobe detected at %p.\n", p->addr);
>diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S
>index e499370fbccb..4572045b968e 100644
>--- a/arch/s390/kernel/mcount.S
>+++ b/arch/s390/kernel/mcount.S
>@@ -10,7 +10,7 @@
> #include <asm/ftrace.h>
> #include <asm/ptrace.h>
>
>-	.section .kprobes.text, "ax"
>+	.section .text.tbl.kprobes.all, "ax"
>
> ENTRY(ftrace_stub)
> 	br	%r14
>diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S
>index 445657fe658c..4614b5bcab5e 100644
>--- a/arch/s390/kernel/vmlinux.lds.S
>+++ b/arch/s390/kernel/vmlinux.lds.S
>@@ -26,7 +26,6 @@ SECTIONS
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		IRQENTRY_TEXT
> 		*(.fixup)
> 		*(.gnu.warning)
>diff --git a/arch/score/kernel/vmlinux.lds.S b/arch/score/kernel/vmlinux.lds.S
>index 7274b5c4287e..36ebcb3b1215 100644
>--- a/arch/score/kernel/vmlinux.lds.S
>+++ b/arch/score/kernel/vmlinux.lds.S
>@@ -41,7 +41,6 @@ SECTIONS
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		*(.text.*)
> 		*(.fixup)
> 		. = ALIGN (4) ;
>diff --git a/arch/sh/kernel/vmlinux.lds.S b/arch/sh/kernel/vmlinux.lds.S
>index db88cbf9eafd..af4f37375345 100644
>--- a/arch/sh/kernel/vmlinux.lds.S
>+++ b/arch/sh/kernel/vmlinux.lds.S
>@@ -37,7 +37,6 @@ SECTIONS
> 		EXTRA_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		IRQENTRY_TEXT
> 		*(.fixup)
> 		*(.gnu.warning)
>diff --git a/arch/sparc/kernel/vmlinux.lds.S b/arch/sparc/kernel/vmlinux.lds.S
>index f1a2f688b28a..a1c8e3a3ebb6 100644
>--- a/arch/sparc/kernel/vmlinux.lds.S
>+++ b/arch/sparc/kernel/vmlinux.lds.S
>@@ -46,7 +46,6 @@ SECTIONS
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		IRQENTRY_TEXT
> 		*(.gnu.warning)
> 	} = 0
>diff --git a/arch/sparc/mm/ultra.S b/arch/sparc/mm/ultra.S
>index b4f4733abc6e..a08fb40656ac 100644
>--- a/arch/sparc/mm/ultra.S
>+++ b/arch/sparc/mm/ultra.S
>@@ -148,7 +148,7 @@ __spitfire_flush_tlb_mm_slow:
> /*
>  * The following code flushes one page_size worth.
>  */
>-	.section .kprobes.text, "ax"
>+	.section .text.tbl.kprobes.all, "ax"
> 	.align		32
> 	.globl		__flush_icache_page
> __flush_icache_page:	/* %o0 = phys_page */
>diff --git a/arch/tile/kernel/vmlinux.lds.S b/arch/tile/kernel/vmlinux.lds.S
>index 0e059a0101ea..0f04268fc394 100644
>--- a/arch/tile/kernel/vmlinux.lds.S
>+++ b/arch/tile/kernel/vmlinux.lds.S
>@@ -43,7 +43,6 @@ SECTIONS
>     HEAD_TEXT
>     SCHED_TEXT
>     LOCK_TEXT
>-    KPROBES_TEXT
>     IRQENTRY_TEXT
>     __fix_text_end = .;   /* tile-cpack won't rearrange before this */
>     ALIGN_FUNCTION();
>diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
>index 1deffe6cc873..dde4ec0e0185 100644
>--- a/arch/x86/kernel/kprobes/core.c
>+++ b/arch/x86/kernel/kprobes/core.c
>@@ -577,7 +577,10 @@ static int reenter_kprobe(struct kprobe *p, struct pt_regs *regs,
> 	case KPROBE_REENTER:
> 		/* A probe has been hit in the codepath leading up to, or just
> 		 * after, single-stepping of a probed instruction. This entire
>-		 * codepath should strictly reside in .kprobes.text section.
>+		 * codepath should strictly reside in the kprobes section:
>+		 *
>+		 * SECTION_TBL(SECTION_TEXT, kprobes, all)
>+		 *
> 		 * Raise a BUG or we'll continue in an endless reentering loop
> 		 * and eventually a stack overflow.
> 		 */
>@@ -1114,10 +1117,9 @@ NOKPROBE_SYMBOL(longjmp_break_handler);
>
> bool arch_within_kprobe_blacklist(unsigned long addr)
> {
>-	return  (addr >= (unsigned long)__kprobes_text_start &&
>-		 addr < (unsigned long)__kprobes_text_end) ||
>+	return  (LINKTABLE_ADDR_WITHIN(kprobes, addr) ||
> 		(addr >= (unsigned long)__entry_text_start &&
>-		 addr < (unsigned long)__entry_text_end);
>+		 addr < (unsigned long)__entry_text_end));
> }
>
> int __init arch_init_kprobes(void)
>diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
>index 74e4bf11f562..80a9f9106d2a 100644
>--- a/arch/x86/kernel/vmlinux.lds.S
>+++ b/arch/x86/kernel/vmlinux.lds.S
>@@ -99,7 +99,6 @@ SECTIONS
> 		TEXT_TEXT
> 		SCHED_TEXT
> 		LOCK_TEXT
>-		KPROBES_TEXT
> 		ENTRY_TEXT
> 		IRQENTRY_TEXT
> 		*(.fixup)
>diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
>index f5ea98bd85d2..0438aab5cb58 100644
>--- a/include/asm-generic/sections.h
>+++ b/include/asm-generic/sections.h
>@@ -24,7 +24,7 @@
>  * Following global variables are optional and may be unavailable on some
>  * architectures and/or kernel configurations.
>  *	_text, _data
>- *	__kprobes_text_start, __kprobes_text_end
>+ *	LINKTABLE_START(kprobes), LINKTABLE_END(kprobes)
>  *	__entry_text_start, __entry_text_end
>  *	__ctors_start, __ctors_end
>  */
>@@ -35,7 +35,7 @@ extern char __init_begin[], __init_end[];
> extern char _sinittext[], _einittext[];
> extern char _end[];
> extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[];
>-extern char __kprobes_text_start[], __kprobes_text_end[];
>+DECLARE_LINKTABLE_TEXT(char, kprobes);
> extern char __entry_text_start[], __entry_text_end[];
> extern char __start_rodata[], __end_rodata[];
>
>diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>index c5fcac902cbe..f1420714a549 100644
>--- a/include/asm-generic/vmlinux.lds.h
>+++ b/include/asm-generic/vmlinux.lds.h
>@@ -112,15 +112,6 @@
> #define BRANCH_PROFILE()
> #endif
>
>-#ifdef CONFIG_KPROBES
>-#define KPROBE_BLACKLIST()	. = ALIGN(8);				      \
>-				VMLINUX_SYMBOL(__start_kprobe_blacklist) = .; \
>-				*(_kprobe_blacklist)			      \
>-				VMLINUX_SYMBOL(__stop_kprobe_blacklist) = .;
>-#else
>-#define KPROBE_BLACKLIST()
>-#endif
>-
> #ifdef CONFIG_EVENT_TRACING
> #define FTRACE_EVENTS()	. = ALIGN(8);					\
> 			VMLINUX_SYMBOL(__start_ftrace_events) = .;	\
>@@ -433,12 +424,6 @@
> 		*(.spinlock.text)					\
> 		VMLINUX_SYMBOL(__lock_text_end) = .;
>
>-#define KPROBES_TEXT							\
>-		ALIGN_FUNCTION();					\
>-		VMLINUX_SYMBOL(__kprobes_text_start) = .;		\
>-		*(.kprobes.text)					\
>-		VMLINUX_SYMBOL(__kprobes_text_end) = .;
>-
> #define ENTRY_TEXT							\
> 		ALIGN_FUNCTION();					\
> 		VMLINUX_SYMBOL(__entry_text_start) = .;			\
>@@ -504,7 +489,6 @@
> 	*(.init.rodata)							\
> 	FTRACE_EVENTS()							\
> 	TRACE_SYSCALLS()						\
>-	KPROBE_BLACKLIST()						\
> 	MEM_DISCARD(init.rodata)					\
> 	CLK_OF_TABLES()							\
> 	RESERVEDMEM_OF_TABLES()						\
>diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>index 00b042c49ccd..8e33fc1ffd3c 100644
>--- a/include/linux/compiler.h
>+++ b/include/linux/compiler.h
>@@ -546,7 +546,8 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
>
> /* Ignore/forbid kprobes attach on very low level functions marked by this attribute: */
> #ifdef CONFIG_KPROBES
>-# define __kprobes	__attribute__((__section__(".kprobes.text")))
>+#include <linux/sections.h>
>+# define __kprobes	__attribute__((__section__(SECTION_TBL(SECTION_TEXT, kprobes, all))))
> # define nokprobe_inline	__always_inline
> #else
> # define __kprobes
>diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
>index 8f6849084248..544eb59cdbb9 100644
>--- a/include/linux/kprobes.h
>+++ b/include/linux/kprobes.h
>@@ -42,8 +42,12 @@
> #include <linux/ftrace.h>
>
> #ifdef CONFIG_KPROBES
>+#include <linux/tables.h>
> #include <asm/kprobes.h>
>
>+DECLARE_LINKTABLE_TEXT(char, kprobes);
>+DECLARE_LINKTABLE_INIT_DATA(unsigned long, _kprobe_blacklist);
>+
> /* kprobe_status settings */
> #define KPROBE_HIT_ACTIVE	0x00000001
> #define KPROBE_HIT_SS		0x00000002
>@@ -487,8 +491,7 @@ static inline int enable_jprobe(struct jprobe *jp)
>  * by using this macro.
>  */
> #define __NOKPROBE_SYMBOL(fname)			\
>-static unsigned long __used				\
>-	__attribute__((section("_kprobe_blacklist")))	\
>+static LINKTABLE_INIT_DATA(_kprobe_blacklist, all)		\
> 	_kbl_addr_##fname = (unsigned long)fname;
> #define NOKPROBE_SYMBOL(fname)	__NOKPROBE_SYMBOL(fname)
> #else
>diff --git a/kernel/kprobes.c b/kernel/kprobes.c
>index d10ab6b9b5e0..d816c659f358 100644
>--- a/kernel/kprobes.c
>+++ b/kernel/kprobes.c
>@@ -1328,8 +1328,7 @@ out:
> bool __weak arch_within_kprobe_blacklist(unsigned long addr)
> {
> 	/* The __kprobes marked functions and entry code must not be probed */
>-	return addr >= (unsigned long)__kprobes_text_start &&
>-	       addr < (unsigned long)__kprobes_text_end;
>+	return LINKTABLE_ADDR_WITHIN(kprobes, addr);
> }
>
> bool within_kprobe_blacklist(unsigned long addr)
>@@ -2054,14 +2053,13 @@ NOKPROBE_SYMBOL(dump_kprobe);
>  * since a kprobe need not necessarily be at the beginning
>  * of a function.
>  */
>-static int __init populate_kprobe_blacklist(unsigned long *start,
>-					     unsigned long *end)
>+static int __init populate_kprobe_blacklist(void)
> {
> 	unsigned long *iter;
> 	struct kprobe_blacklist_entry *ent;
> 	unsigned long entry, offset = 0, size = 0;
>
>-	for (iter = start; iter < end; iter++) {
>+	LINKTABLE_FOR_EACH(iter, _kprobe_blacklist) {
> 		entry = arch_deref_entry_point((void *)*iter);
>
> 		if (!kernel_text_address(entry) ||
>@@ -2126,8 +2124,9 @@ static struct notifier_block kprobe_module_nb = {
> };
>
> /* Markers of _kprobe_blacklist section */
>-extern unsigned long __start_kprobe_blacklist[];
>-extern unsigned long __stop_kprobe_blacklist[];
>+DEFINE_LINKTABLE_INIT_DATA(unsigned long, _kprobe_blacklist);
>+/* Actual kprobes linker table */
>+DEFINE_LINKTABLE_TEXT(char, kprobes);
>
> static int __init init_kprobes(void)
> {
>@@ -2141,8 +2140,7 @@ static int __init init_kprobes(void)
> 		raw_spin_lock_init(&(kretprobe_table_locks[i].lock));
> 	}
>
>-	err = populate_kprobe_blacklist(__start_kprobe_blacklist,
>-					__stop_kprobe_blacklist);
>+	err = populate_kprobe_blacklist();
> 	if (err) {
> 		pr_err("kprobes: failed to populate blacklist: %d\n", err);
> 		pr_err("Please take care of using kprobes.\n");
>diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>index 48958d3cec9e..b0a8f519efb8 100644
>--- a/scripts/mod/modpost.c
>+++ b/scripts/mod/modpost.c
>@@ -22,6 +22,7 @@
> #include "../../include/generated/autoconf.h"
> #include "../../include/linux/license.h"
> #include "../../include/linux/export.h"
>+#include "../../include/linux/sections.h"
>
> /* Are we using CONFIG_MODVERSIONS? */
> static int modversions = 0;
>@@ -888,7 +889,7 @@ static void check_section(const char *modname, struct elf_info *elf,
>
> #define DATA_SECTIONS ".data", ".data.rel"
> #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
>-		".kprobes.text"
>+		SECTION_TBL(SECTION_TEXT, kprobes, all)
> #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
> 		".fixup", ".entry.text", ".exception.text", ".text.*", \
> 		".coldtext"
>diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
>index e167592793a7..f37dac17f033 100644
>--- a/scripts/recordmcount.c
>+++ b/scripts/recordmcount.c
>@@ -32,6 +32,7 @@
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
>+#include "../../include/linux/sections.h"
>
> #ifndef EM_METAG
> /* Remove this when these make it to the standard system elf.h. */
>@@ -356,7 +357,7 @@ is_mcounted_section_name(char const *const txtname)
> 		strcmp(".sched.text",    txtname) == 0 ||
> 		strcmp(".spinlock.text", txtname) == 0 ||
> 		strcmp(".irqentry.text", txtname) == 0 ||
>-		strcmp(".kprobes.text", txtname) == 0 ||
>+		strcmp(SECTION_TBL(SECTION_TEXT, kprobe, all), txtname) == 0 ||
> 		strcmp(".text.unlikely", txtname) == 0;
> }
>
>diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
>index 96e2486a6fc4..63e13009dda9 100755
>--- a/scripts/recordmcount.pl
>+++ b/scripts/recordmcount.pl
>@@ -134,7 +134,7 @@ my %text_sections = (
>      ".sched.text" => 1,
>      ".spinlock.text" => 1,
>      ".irqentry.text" => 1,
>-     ".kprobes.text" => 1,
>+     ".text.tbl.kprobes.all" => 1,
>      ".text.unlikely" => 1,
> );
>
>--
>2.7.0

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

#1340110 — Re: [Xen-devel] [RFC v2 7/7] kprobes: port to linker table

From"Luis R. Rodriguez" <mcgrof@kernel.org>
Date2016-02-23 02:00 +0100
SubjectRe: [Xen-devel] [RFC v2 7/7] kprobes: port to linker table
Message-ID<r59ux-5YU-1@gated-at.bofh.it>
In reply to#1339048
On Mon, Feb 22, 2016 at 01:34:05AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
> >From: Luis R. Rodriguez [mailto:mcgrof@kernel.org]
> >
> >kprobe makes use of two custom sections:
> >
> >type       name                  begin                    end
> >init.data _kprobe_blacklist __start_kprobe_blacklist	__stop_kprobe_blacklist
> >text      .kprobes.text     __kprobes_text_start        __kprobes_text_end
> >
> >Port these to the linker table generic solution. This lets
> >us remove all the custom kprobe section declarations on the
> >linker script.
> >
> >Tested with CONFIG_KPROBES_SANITY_TEST, it passes with:
> >
> >Kprobe smoke test: started
> >Kprobe smoke test: passed successfully
> >
> >Then tested CONFIG_SAMPLE_KPROBES on do_fork, and the
> >kprobe bites and kicks as expected. Lastly tried registering
> >a kprobe on a kprobe blacklisted symbol (NOKPROBE_SYMBOL()),
> >and confirms that fails to work.
> 
> Could you also check to run the testcases by using ftracetest as below?
> 
> $ cd tools/testing/selftests/ftrace/
> $ sudo ./ftracetest

Sure, it all passed:

$ sudo ./ftracetest
=== Ftrace unit tests ===
[1] Basic trace file check	[PASS]
[2] Basic test for tracers	[PASS]
[3] Basic trace clock test	[PASS]
[4] Basic event tracing check	[PASS]
[5] event tracing - enable/disable with event level files	[PASS]
[6] event tracing - enable/disable with subsystem level files	[PASS]
[7] event tracing - enable/disable with top level files	[PASS]
[8] ftrace - function graph filters with stack tracer	[PASS]
[9] ftrace - function graph filters	[PASS]
[10] ftrace - function profiler with function tracing	[PASS]
[11] Test creation and deletion of trace instances	[PASS]
[12] Kprobe dynamic event - adding and removing	[PASS]
[13] Kprobe dynamic event - busy event check	[PASS]
[14] Kprobe dynamic event with arguments	[PASS]
[15] Kprobe dynamic event with function tracer	[PASS]
[16] Kretprobe dynamic event with arguments	[PASS]

# of passed:  16
# of failed:  0
# of unresolved:  0
# of untested:  0
# of unsupported:  0
# of xfailed:  0
# of undefined(test bug):  0

> And I'm not sure about linker table.

So there's really a few parts to the linker table work, out of
the ones that relate to kprobes:

  * linker tables try to generalize our section use, and provide some
    helpers for common section use
  * provides helpers to make it easier to make custom section,
    but by re-using standard sections
  * when a custom section uses standard sections and helpers
    we also get a few gains:
    - developers reviewing code can more easily get a quicker
      understanding and have expectations set of how the code
      feature using the custom section could be used
    - people grep'ing on the kernel can more easily find
      specific types of custom section use by looking for
      the type of interest
    - developers adding features do not need to modify
      any linker scripts (vmlinux.lds.S) to make use of
      custom sections

In kprobe's case, since it uses two custom sections, we have
only a small use for the first case: .kprobe.text is just used
as a place holder for future developer annotated special cased
executable code. It also makes use of the generic helpers:
LINKTABLE_ADDR_WITHIN(), LINKTABLE_START(), LINKTABLE_END().

The second use case, for the _kprobe_blacklist, makes much more
use of the more advanced linker table helpers, for instance the
iterator LINKTABLE_FOR_EACH().

For both though we now have each custom section's actual section
clearly highlighted:

  * kprobes: .text (SECTION_TEXT)
  * kprobe blacklist: init.data (SECTION_INIT_DATA)

A reader / developer can more easily gain an understanding
of how the above custom sections could be used just by its
type.

Another feature of linker tables, but outside of the scope of how kprobe
would use linker tables, is the ability to enable folks to avoid code
bit rot by using table-$(CONFIG_FOO) instead of oby-$(CONFIG_FOO) on
init paths of code but since this is outside of the scope of how kprobe would
use I leave that just as a reference as another part of linker table.
Unless of course you want to make people force compile all kprobe
support code but only have it linked in when actually enabled. That
would be outside of the scope and purpose of this patch though.

> Is that possible to support
> __kprobes prefix, which moves the functions into kprobes.text?

Absolutely, the respective change was just to annotate and make
it clear the section kprobes were using:

-# define __kprobes     __attribute__((__section__(".kprobes.text")))
+#include <linux/sections.h>
+# define __kprobes     __attribute__((__section__(SECTION_TBL(SECTION_TEXT, kprobes, all))))

> Actually, I'm on the way to replacing __kprobes to NOKPROBE_SYMBOL
> macro, since NOKPROBE_SYMBOL() doesn't effect the kernel text itself.
> On x86, it is already replaced (see commit 820aede0209a), and same
> work should be done on other archs. So, could you hold this after
> that?

Sure.

> I think we should remove .kprobes.text first 

You mean just remove the incorrect users of .kprobes.text because as I read
what you described above we have abuse of __kprobes use to protect against
kprobes being introduced on those routines, and we should be using
NOKPROBE_SYMBOL() instead. So from what I gather its not that you will
remove .kprobes.text but rather clean our current abuse of __kprobes
for protection to use NOKPROBE_SYMBOL() instead. Is that right?

> and move to  linker table.

Sure, can you Cc me on your patches? I can follow up later.

  Luis

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

#1338439

From"H. Peter Anvin" <hpa@zytor.com>
Date2016-02-19 21:30 +0100
Message-ID<r3ZQC-2mP-15@gated-at.bofh.it>
In reply to#1338193
On 02/19/2016 05:45 AM, Luis R. Rodriguez wrote:
> This is my v2 of the original linker table work [0], now with
> six proof of concepts ports of existing code using custom section
> with custom linker script modifications:
> 
>   * DEFINE_LINKTABLE_TEXT(char, kprobes);
>   * DEFINE_LINKTABLE_DATA(struct jump_entry, __jump_table);
>   * DEFINE_LINKTABLE_DATA(struct _ddebug, __verbose);
>   * DEFINE_LINKTABLE_RO(struct builtin_fw, builtin_fw);
>   * DEFINE_LINKTABLE_INIT(struct x86_init_fn, x86_init_fns);
>   * DEFINE_LINKTABLE_INIT_DATA(unsigned long, _kprobe_blacklist);
> 
> I've tested all except jump tables, I'd appreaciate some help with that.
> 

We should add support for read-mostly, probably.  In fact, some of these
probably *are* read-mostly.

	-hpa

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

#1338502

From"Luis R. Rodriguez" <mcgrof@kernel.org>
Date2016-02-19 22:30 +0100
Message-ID<r40MG-332-35@gated-at.bofh.it>
In reply to#1338439
On Fri, Feb 19, 2016 at 12:16:29PM -0800, H. Peter Anvin wrote:
> On 02/19/2016 05:45 AM, Luis R. Rodriguez wrote:
> > This is my v2 of the original linker table work [0], now with
> > six proof of concepts ports of existing code using custom section
> > with custom linker script modifications:
> > 
> >   * DEFINE_LINKTABLE_TEXT(char, kprobes);
> >   * DEFINE_LINKTABLE_DATA(struct jump_entry, __jump_table);
> >   * DEFINE_LINKTABLE_DATA(struct _ddebug, __verbose);
> >   * DEFINE_LINKTABLE_RO(struct builtin_fw, builtin_fw);
> >   * DEFINE_LINKTABLE_INIT(struct x86_init_fn, x86_init_fns);
> >   * DEFINE_LINKTABLE_INIT_DATA(unsigned long, _kprobe_blacklist);
> > 
> > I've tested all except jump tables, I'd appreaciate some help with that.
> > 
> 
> We should add support for read-mostly, probably. 

Sure, any candidate in mind? I actually don't see any hacks built
on top of READ_MOSTLY_DATA include/asm-generic/vmlinux.lds.h so
pointers appreciated for a case I could try.

> In fact, some of these probably *are* read-mostly.

To avoid regressions I made sure to use the same exact section that
these original solutions came from, do we want to move some? If so
I think perhaps that's best done as a separate atomic patch and
it may be easier to read and review once ported on linker tables?

 Luis

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

#1338442 — Re: [RFC v2 2/7] tables.h: add linker table support

From"H. Peter Anvin" <hpa@zytor.com>
Date2016-02-19 21:40 +0100
SubjectRe: [RFC v2 2/7] tables.h: add linker table support
Message-ID<r400h-2qQ-7@gated-at.bofh.it>
In reply to#1338193
On 02/19/2016 05:45 AM, Luis R. Rodriguez wrote:
> +
> +/**
> + * DOC: Regular linker linker table constructors
> + *
> + * Regular constructors are expected to be used for valid linker table entries.
> + * Valid uses of weak entries other than the beginning and is currently
> + * untested but should in theory work.
> + */
> +
> +/**
> + * LINKTABLE_TEXT - Declares a linker table entry for execution
> + *
> + * @name: linker table name
> + * @level: order level
> + *
> + * Declares a linker table to be used for execution.
> + */
> +#define LINKTABLE_TEXT(name, level)					\
> +	      __typeof__(name[0])					\
> +	      __attribute__((used,					\
> +			     __aligned__(LINKTABLE_ALIGNMENT(name)),	\
> +			     section(SECTION_TBL(SECTION_TEXT, name, level))))

I'm really confused by this.  Text should obviously be readonly, but I'm
not at all clear how this works here.

The issue with linktables for text is kind of confusing if nothing else;
Russel is right about that.  It doesn't prevent us from doing something
similar, but perhaps it ought to have a different name.

For one thing, priority level is meaningless for text, since it is not a
table that can be indexed into.

	-hpa

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

#1338507 — Re: [RFC v2 2/7] tables.h: add linker table support

From"Luis R. Rodriguez" <mcgrof@kernel.org>
Date2016-02-19 22:50 +0100
SubjectRe: [RFC v2 2/7] tables.h: add linker table support
Message-ID<r4162-3e8-7@gated-at.bofh.it>
In reply to#1338442
On Fri, Feb 19, 2016 at 12:25:55PM -0800, H. Peter Anvin wrote:
> On 02/19/2016 05:45 AM, Luis R. Rodriguez wrote:
> > +
> > +/**
> > + * DOC: Regular linker linker table constructors
> > + *
> > + * Regular constructors are expected to be used for valid linker table entries.
> > + * Valid uses of weak entries other than the beginning and is currently
> > + * untested but should in theory work.
> > + */
> > +
> > +/**
> > + * LINKTABLE_TEXT - Declares a linker table entry for execution
> > + *
> > + * @name: linker table name
> > + * @level: order level
> > + *
> > + * Declares a linker table to be used for execution.
> > + */
> > +#define LINKTABLE_TEXT(name, level)					\
> > +	      __typeof__(name[0])					\
> > +	      __attribute__((used,					\
> > +			     __aligned__(LINKTABLE_ALIGNMENT(name)),	\
> > +			     section(SECTION_TBL(SECTION_TEXT, name, level))))
> 
> I'm really confused by this.  Text should obviously be readonly, 

So this uses SECTION_TEXT, so we just pegged the linker table entry right below
the standard SECTION_TEXT:

@@ -422,7 +426,8 @@
  * during second ld run in second ld pass when generating System.map */
 #define TEXT_TEXT                                                      \
                ALIGN_FUNCTION();                                       \
-               *(.text.hot .text .text.fixup .text.unlikely)           \
+               *(.text.hot SECTION_TEXT .text.fixup .text.unlikely)            \
+               *(SORT(SECTION_TBL_ALL(SECTION_TEXT)))                  \
                *(.ref.text)                                            \
        MEM_KEEP(init.text)                                             \
        MEM_KEEP(exit.text) 

So this gets folded under TEXT_TEXT and we don't see to force all
architectures for this to be read-only specifically. I can't be sure
if it is always read-only for all architectures based on a cursory
review.

> but I'm not at all clear how this works here.

Its simply trying to piggy back on top of where the standard section
it is using so in this case SECTION_TEXT (.text). What attributes are
part of that follow.

> The issue with linktables for text is kind of confusing if nothing else;
> Russel is right about that.  It doesn't prevent us from doing something
> similar, but perhaps it ought to have a different name.

Any recommendations?

> For one thing, priority level is meaningless for text, since it is not a
> table that can be indexed into.

Ah right, there is no structure so we can't foo->in, let alone, know what
arguments to pass... the order level certainly would still kick in but
I agree that it would seem kind of pointless as something else would
have to know how to use it. I could simply remove the iterator and
order-level for SECTION_TEXT linker tables if we are sure we can't use it.
The only gain then here would be that of saving custom linker script
changes and having a unified way to represent the attributes.

If order is desired a structured data would would make more sense and
I could document / recommend that, however I don't yet have a ported
use case in mind yet, any ideas? The use case I have that follows
similar logic is for the x86 init stuff which I am sending separately
and that's a new use case though. It uses SECTION_INIT (.init.text)
and that allows for both use of structuralized data + callbacks we
can execute. It was not clear to me what section to use for this sort
of stuff for non init stuff, and I can envision people highly wanting
to use this to help clean up init paths on code, what sectoin
should be used in those cases?

 Luis

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

#1341156 — Re: [RFC v2 2/7] tables.h: add linker table support

From"Luis R. Rodriguez" <mcgrof@kernel.org>
Date2016-02-24 00:10 +0100
SubjectRe: [RFC v2 2/7] tables.h: add linker table support
Message-ID<r5ufE-44a-11@gated-at.bofh.it>
In reply to#1338507
On Fri, Feb 19, 2016 at 1:48 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Fri, Feb 19, 2016 at 12:25:55PM -0800, H. Peter Anvin wrote:
>> On 02/19/2016 05:45 AM, Luis R. Rodriguez wrote:
>> > +
>> > +/**
>> > + * DOC: Regular linker linker table constructors
>> > + *
>> > + * Regular constructors are expected to be used for valid linker table entries.
>> > + * Valid uses of weak entries other than the beginning and is currently
>> > + * untested but should in theory work.
>> > + */
>> > +
>> > +/**
>> > + * LINKTABLE_TEXT - Declares a linker table entry for execution
>> > + *
>> > + * @name: linker table name
>> > + * @level: order level
>> > + *
>> > + * Declares a linker table to be used for execution.
>> > + */
>> > +#define LINKTABLE_TEXT(name, level)                                        \
>> > +         __typeof__(name[0])                                       \
>> > +         __attribute__((used,                                      \
>> > +                        __aligned__(LINKTABLE_ALIGNMENT(name)),    \
>> > +                        section(SECTION_TBL(SECTION_TEXT, name, level))))
>>
>> I'm really confused by this.  Text should obviously be readonly,
>
> So this uses SECTION_TEXT, so we just pegged the linker table entry right below
> the standard SECTION_TEXT:

OK yes I see the issue now. I've modified this to use const, and
retested the kprobe patch and it works well still. kprobe would not
use LINKTABLE_TEXT, instead it uses its own macro, however users of
LINKTABLE_TEXT would then have const declared. The implications are
that you *can* declare structs so long as everything is const.

Folks may at times need to modify the structural definitions -- for
that LINKTABLE_DATA() is more appropriate, and as per my testing it
still allows execution of callbacks. I can document this.

Do we want .init.text to match this? I'd port my "struct x86_init_fn"
to use LINKTABLE_INIT_DATA() then. FWIW below is a paste of a simple
test program that demos what I mean.

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/workqueue.h>
#include <linux/tables.h>

static void stuff_todo(struct work_struct *work);
static DECLARE_WORK(stuff_work, stuff_todo);

struct stuff {
int a;
int b;
void (*print_a)(struct stuff *);
void (*print_b)(struct stuff *);

void (*print_a_ro)(const struct stuff *);
void (*print_b_ro)(const struct stuff *);
};

void print_a(struct stuff *s)
{
pr_info("print_a a: %d\n", s->a);
}

void print_a_ro(const struct stuff *s)
{
pr_info("print_a a: %d\n", s->a);
}

void print_b(struct stuff *s)
{
pr_info("print_b b: %d\n", s->b);
}

void print_b_ro(const struct stuff *s)
{
pr_info("print_b b: %d\n", s->b);
}

DEFINE_LINKTABLE_TEXT(struct stuff, my_stuff_fns_ro);
DEFINE_LINKTABLE_DATA(struct stuff, my_stuff_fns);

static LINKTABLE_TEXT(my_stuff_fns_ro, 0000) stuff_a_ro = {
.a = 1,
.b = 1,
.print_a_ro = print_a_ro,
.print_b_ro = print_b_ro,
};

static LINKTABLE_TEXT(my_stuff_fns_ro, 0000) stuff_b_ro = {
.a = 2,
.b = 2,
.print_a_ro = print_a_ro,
.print_b_ro = print_b_ro,
};

static LINKTABLE_TEXT(my_stuff_fns_ro, 0000) stuff_c_ro = {
.a = 3,
.b = 3,
.print_a_ro = print_a_ro,
.print_b_ro = print_b_ro,
};

static LINKTABLE_DATA(my_stuff_fns, 0000) stuff_a = {
.a = 1,
.b = 1,
.print_a = print_a,
.print_b = print_b,
};

static LINKTABLE_DATA(my_stuff_fns, 0000) stuff_b = {
.a = 2,
.b = 2,
.print_a = print_a,
.print_b = print_b,
};

static void stuff_todo(struct work_struct *work)
{
struct stuff *s;
const struct stuff *s_ro;
unsigned int i = 0;

pr_info("Looping over my_stuff_fns_ro\n");

LINKTABLE_FOR_EACH(s_ro, my_stuff_fns_ro) {
pr_info("Looping on s ro %d\n", i++);
s_ro->print_a_ro(s_ro);
s_ro->print_b_ro(s_ro);
}

i=0;
pr_info("Looping over my_stuff_fns\n");

LINKTABLE_FOR_EACH(s, my_stuff_fns) {
pr_info("Looping on s %d\n", i++);
s->print_a(s);
s->print_b(s);
}

i=0;
pr_info("Looping over my_stuff_fns and creating modifications\n");

LINKTABLE_FOR_EACH(s, my_stuff_fns) {
s->a = 10;
s->b = 10;
s->print_a(s);
s->print_b(s);
}
}

static int __init stuff_init(void)
{
/* get out of __init context */
schedule_work(&stuff_work);
return 0;
}

static void __exit stuff_exit(void)
{
cancel_work_sync(&stuff_work);
}

module_init(stuff_init)
module_exit(stuff_exit)
MODULE_LICENSE("GPL");


  Luis

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

#1341166 — Re: [RFC v2 2/7] tables.h: add linker table support

From"H. Peter Anvin" <hpa@zytor.com>
Date2016-02-24 00:30 +0100
SubjectRe: [RFC v2 2/7] tables.h: add linker table support
Message-ID<r5uyZ-4cb-15@gated-at.bofh.it>
In reply to#1341156
On 02/23/2016 03:08 PM, Luis R. Rodriguez wrote:
> 
> OK yes I see the issue now. I've modified this to use const, and
> retested the kprobe patch and it works well still. kprobe would not
> use LINKTABLE_TEXT, instead it uses its own macro, however users of
> LINKTABLE_TEXT would then have const declared. The implications are
> that you *can* declare structs so long as everything is const.
> 

No, such structures belong in .rodata not in .text.

I have been thinking about it somewhat, and for text we really have text
"ranges" rather than tables.  The big difference between the two are:

1. priority ordering doesn't make any sense for ranges.
2. ranges can be hierarchial, that is, range "bar" can be entirely
   inside range "foo".
3. ranges aren't typed (although in C, that pretty much means they are
   "char" or "unsigned char" as there really isn't any way to define an
   "array of void".)
4. the only useful operator on a range is "is address X inside this
   range"; this operator is likely *not* useful for a table, since
   if you have to ever invoke it you are probably doing something very
   wrong.

For this to work, we need strings such that they will always sort in the
appropriate order with the bracket symbols around subranges.  I need to
think about this a few minutes before I actually say anything about it...

	-hpa

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

#1341177 — Re: [RFC v2 2/7] tables.h: add linker table support

From"Luis R. Rodriguez" <mcgrof@kernel.org>
Date2016-02-24 00:40 +0100
SubjectRe: [RFC v2 2/7] tables.h: add linker table support
Message-ID<r5uIG-4hX-19@gated-at.bofh.it>
In reply to#1341166
On Tue, Feb 23, 2016 at 3:22 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 02/23/2016 03:08 PM, Luis R. Rodriguez wrote:
>>
>> OK yes I see the issue now. I've modified this to use const, and
>> retested the kprobe patch and it works well still. kprobe would not
>> use LINKTABLE_TEXT, instead it uses its own macro, however users of
>> LINKTABLE_TEXT would then have const declared. The implications are
>> that you *can* declare structs so long as everything is const.
>>
>
> No, such structures belong in .rodata not in .text.
>
> I have been thinking about it somewhat, and for text we really have text
> "ranges" rather than tables.  The big difference between the two are:
>
> 1. priority ordering doesn't make any sense for ranges.

I considered the possibility perhaps we want to just split out a
series of initial basic macros that enables *only* such basic
functionality and declarations, and linker tables would then build on
top of these. I think that ma help here.

> 2. ranges can be hierarchial, that is, range "bar" can be entirely
>    inside range "foo".

We'd want to enable this without having to modify the linker script
further -- I thought about how such things might be possible but not
for this case but rather, for the case where in linker tables a
developer may wish to get range addresses for a specific order level.
Although separate, the problem seems similar. At least in theory I was
convinced I had a solution to the order level problem, perhaps it may
suffice for this too:

When and if you need an intermediary set of range addresses you then
also declare another set of extern pointers, similar in nature with
the empty string and ~ ending delimiter, we just move one notch in.
SORT() will take care of ordering things for us, but the issue here is
the empty string doesn't allow hierarchies so perhaps to enable that
we may need another beginning delimiter that would enable recursive
hierarchies. If this seems to at least in theory make sense I can give
it a shot through a simple proof of concept and see if it works.

> 3. ranges aren't typed (although in C, that pretty much means they are
>    "char" or "unsigned char" as there really isn't any way to define an
>    "array of void".)

Sure. kprobe is a good example.

> 4. the only useful operator on a range is "is address X inside this
>    range"; this operator is likely *not* useful for a table, since
>    if you have to ever invoke it you are probably doing something very
>    wrong.

kprobe uses it :P

> For this to work, we need strings such that they will always sort in the
> appropriate order with the bracket symbols around subranges.  I need to
> think about this a few minutes before I actually say anything about it...

Ah seems we're in the same line of thought ?

 Luis

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

#1341200 — Re: [RFC v2 2/7] tables.h: add linker table support

From"H. Peter Anvin" <hpa@zytor.com>
Date2016-02-24 01:20 +0100
SubjectRe: [RFC v2 2/7] tables.h: add linker table support
Message-ID<r5vln-4Oy-9@gated-at.bofh.it>
In reply to#1341177
On 02/23/2016 03:36 PM, Luis R. Rodriguez wrote:
> 
>> 4. the only useful operator on a range is "is address X inside this
>>    range"; this operator is likely *not* useful for a table, since
>>    if you have to ever invoke it you are probably doing something very
>>    wrong.
> 
> kprobe uses it :P
> 

Could you explain how?

	-hpa

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

#1341215 — Re: [RFC v2 2/7] tables.h: add linker table support

From"Luis R. Rodriguez" <mcgrof@kernel.org>
Date2016-02-24 02:00 +0100
SubjectRe: [RFC v2 2/7] tables.h: add linker table support
Message-ID<r5vY7-54n-17@gated-at.bofh.it>
In reply to#1341200
On Tue, Feb 23, 2016 at 04:06:55PM -0800, H. Peter Anvin wrote:
> On 02/23/2016 03:36 PM, Luis R. Rodriguez wrote:
> > 
> >> 4. the only useful operator on a range is "is address X inside this
> >>    range"; this operator is likely *not* useful for a table, since
                                                     ^^^^^^^^^^^^
> >>    if you have to ever invoke it you are probably doing something very
> >>    wrong.
> > 
> > kprobe uses it :P
> > 
> 
> Could you explain how?

Sorry I misread this as "unless you are a table", kprobes has two
ranges, one is a table (blacklist) and the other just a range
(for kprobes); only kprobes uses "address inside this range",
as reflected below. So I agree with you.

index d10ab6b9b5e0..d816c659f358 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1328,8 +1328,7 @@ out:
 bool __weak arch_within_kprobe_blacklist(unsigned long addr)
 {
        /* The __kprobes marked functions and entry code must not be probed */
-       return addr >= (unsigned long)__kprobes_text_start &&
-              addr < (unsigned long)__kprobes_text_end;
+       return LINKTABLE_ADDR_WITHIN(kprobes, addr);
 }
 
 bool within_kprobe_blacklist(unsigned long addr)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d10ab6b9b5e0..d816c659f358 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1328,8 +1328,7 @@ out:
 bool __weak arch_within_kprobe_blacklist(unsigned long addr)
 {
        /* The __kprobes marked functions and entry code must not be probed */
-       return addr >= (unsigned long)__kprobes_text_start &&
-              addr < (unsigned long)__kprobes_text_end;
+       return LINKTABLE_ADDR_WITHIN(kprobes, addr);
 }
 
What about rebranding general section primitives under section.h

#define DECLARE_SECTION_TEXT_TYPE(type, name)                              \       
         extern const type name[], name##__end[];

#define DECLARE_SECTION_TEXT(name)                              \       
	DECLARE_SECTION_TEXT_TYPE(char, name)

Then tables.h would use the TYPE version:

#define DECLARE_LINKTABLE_TEXT(type, name)                              \       
         DECLARE_SECTION_TEXT_TYPE(type, name)

Since I've been making _TEXT the implicit type for section names(SECTION_INIT
is .init.text) the above could just be DECLARE_SECTION_TYPE() and DECLARE_SECTION()
for text if we prefer.

  Luis

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

#1338447 — Re: [RFC v2 2/7] tables.h: add linker table support

From"H. Peter Anvin" <hpa@zytor.com>
Date2016-02-19 21:40 +0100
SubjectRe: [RFC v2 2/7] tables.h: add linker table support
Message-ID<r400i-2qQ-15@gated-at.bofh.it>
In reply to#1338193
On 02/19/2016 05:45 AM, Luis R. Rodriguez wrote:
> +/**
> + * LINKTABLE_RUN_ERR - run each linker table entry func and return error if any
> + *
> + * @tbl: linker table
> + * @func: structure name for the function name we want to call.
> + * @args...: arguments to pass to func
> + *
> + * Example usage:
> + *
> + *   unsigned int err = LINKTABLE_RUN_ERR(frobnicator_fns, some_run,);
> + */
> +#define LINKTABLE_RUN_ERR(tbl, func, args...)				\
> +({									\
> +	size_t i;							\
> +	int err = 0;							\
> +	for (i = 0; !err && i < LINKTABLE_SIZE(tbl); i++)		\
> +		err = (tbl[i]).func (args);				\
> +		err; \
> +})

This is wrong and pointless.  As written it returns the error code of
the last instance.

What I suggested for this macro was that we ought to exit the loop on error.

Furthermore:

1. Using an advancing pointer would make more sense than a counter.
2. .func doesn't make any sense -- it ought to be "func"; otherwise you
    can't call into either a table containing pure function pointers,
    nor into a field of table *pointed to* by the table; which is
    likely to be quite common.

    So the idea is to use something like:

    /* Array of function pointers */
    LINKTABLE_RUN_ALL(frobnicator_fns, , arg1, arg2);

    /* Array of structures */
    LINKTABLE_RUN_ALL(frobnicator_fns, .some_run, arg1, arg2);

    /* Array of structure pointers */
    LINKTABLE_RUN_ALL(frobnicator_fns, ->some_run, arg1, arg2);

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

#1338475 — Re: [RFC v2 2/7] tables.h: add linker table support

From"Luis R. Rodriguez" <mcgrof@suse.com>
Date2016-02-19 22:20 +0100
SubjectRe: [RFC v2 2/7] tables.h: add linker table support
Message-ID<r40D0-2XK-23@gated-at.bofh.it>
In reply to#1338447
On Fri, Feb 19, 2016 at 12:33:27PM -0800, H. Peter Anvin wrote:
> On 02/19/2016 05:45 AM, Luis R. Rodriguez wrote:
> > +/**
> > + * LINKTABLE_RUN_ERR - run each linker table entry func and return error if any
> > + *
> > + * @tbl: linker table
> > + * @func: structure name for the function name we want to call.
> > + * @args...: arguments to pass to func
> > + *
> > + * Example usage:
> > + *
> > + *   unsigned int err = LINKTABLE_RUN_ERR(frobnicator_fns, some_run,);
> > + */
> > +#define LINKTABLE_RUN_ERR(tbl, func, args...)				\
> > +({									\
> > +	size_t i;							\
> > +	int err = 0;							\
> > +	for (i = 0; !err && i < LINKTABLE_SIZE(tbl); i++)		\
> > +		err = (tbl[i]).func (args);				\
> > +		err; \
> > +})
> 
> This is wrong and pointless.  As written it returns the error code of
> the last instance.
> 
> What I suggested for this macro was that we ought to exit the loop on error.
> 
> Furthermore:
> 
> 1. Using an advancing pointer would make more sense than a counter.

OK I'll modify to use LINKTABLE_FOR_EACH and much cleaner and obvious
code to bail on error.

> 2. .func doesn't make any sense -- it ought to be "func"; otherwise you
>     can't call into either a table containing pure function pointers,
>     nor into a field of table *pointed to* by the table; which is
>     likely to be quite common.
> 
>     So the idea is to use something like:
> 
>     /* Array of function pointers */
>     LINKTABLE_RUN_ALL(frobnicator_fns, , arg1, arg2);
> 
>     /* Array of structures */
>     LINKTABLE_RUN_ALL(frobnicator_fns, .some_run, arg1, arg2);
> 
>     /* Array of structure pointers */
>     LINKTABLE_RUN_ALL(frobnicator_fns, ->some_run, arg1, arg2);

Ah that's sexy, thanks. Will amend.

  Luis

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web