2022-10-22 23:09:17

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 0/2] RESEND vmlinux.lds.h tweaks

hi Greg,

this time w/o the stale patch 2.

These 2 patches are "no functional change", but they are a simple step
towards de-duplicating the repetitive columms in the __dyndbg section.

For a DYNAMIC_DEBUG=y kernel with 5k pr_debugs/drm.debugs, the
footprint reduction should be ~100 KiB

Jim Cromie (2):
vmlinux.lds.h: add BOUNDED_SECTION* macros
vmlinux.lds.h: place optional header space in BOUNDED_SECTION

include/asm-generic/vmlinux.lds.h | 221 +++++++++++-------------------
1 file changed, 81 insertions(+), 140 deletions(-)

CC: Suren Baghdasaryan <[email protected]>
CC: Kent Overstreet <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>

--
2.37.3


2022-10-22 23:09:22

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 2/2] vmlinux.lds.h: place optional header space in BOUNDED_SECTION

Extend recently added BOUNDED_SECTION(_name) macro by adding a
KEEP(*(.gnu.linkonce.##_name)) before the KEEP(*(_name)).

This does nothing by itself, vmlinux is the same before and after this
patch. But if a developer adds a .gnu.linkonce.foo record, that
record is placed in the front of the section, where it can be used as
a header for the table.

The intent is to create an up-link to another organizing struct, from
where related tables can be referenced. And since every item in a
table has a known offset from its header, that same offset can be used
to fetch records from the related tables.

By itself, this doesnt gain much, unless maybe the pattern of access
is to scan 1 or 2 fields in each fat record, but with 2 16 bit .map*
fields added, we could de-duplicate 2 related tables.

The use case here is struct _ddebug, which has 3 pointers (function,
file, module) with substantial repetition; respectively 53%, 90%, and
the module column is fully recoverable after dynamic_debug_init()
splits the table into a linked list of "module" chunks.

On a DYNAMIC_DEBUG=y kernel with 5k pr_debugs, the memory savings
should be ~100 KiB.

Signed-off-by: Jim Cromie <[email protected]>
---
? does this need a new name ? HEADERED_SECTION ?
---
include/asm-generic/vmlinux.lds.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 9f6352171f88..b3ca56ac163f 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -195,11 +195,13 @@

#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _s_, _e_) \
_s_##_label_ = .; \
+ KEEP(*(.gnu.linkonce.##_sec_)) \
KEEP(*(_sec_)) \
_e_##_label_ = .;

#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _s_, _e_) \
_label_##_s_ = .; \
+ KEEP(*(.gnu.linkonce.##_sec_)) \
KEEP(*(_sec_)) \
_label_##_e_ = .;

--
2.37.3

2022-10-22 23:09:28

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 1/2] vmlinux.lds.h: add BOUNDED_SECTION* macros

vmlinux.lds.h has ~45 occurrences of this general pattern:

__start_foo = .;
KEEP(*(foo))
__stop_foo = .;

Reduce this pattern to a (group of 4) macros, and use them to reduce
linecount. This was inspired by the codetag patchset.

no functional change.

CC: Suren Baghdasaryan <[email protected]>
CC: Kent Overstreet <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
---

Shall we bikeshed the names ?
---
include/asm-generic/vmlinux.lds.h | 219 +++++++++++-------------------
1 file changed, 79 insertions(+), 140 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index c15de165ec8f..9f6352171f88 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -193,100 +193,99 @@
# endif
#endif

+#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _s_, _e_) \
+ _s_##_label_ = .; \
+ KEEP(*(_sec_)) \
+ _e_##_label_ = .;
+
+#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _s_, _e_) \
+ _label_##_s_ = .; \
+ KEEP(*(_sec_)) \
+ _label_##_e_ = .;
+
+#define BOUNDED_SECTION_BY(_sec_, _label_) \
+ BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
+
+#define BOUNDED_SECTION(_sec) BOUNDED_SECTION_BY(_sec, _sec)
+
#ifdef CONFIG_TRACE_BRANCH_PROFILING
-#define LIKELY_PROFILE() __start_annotated_branch_profile = .; \
- KEEP(*(_ftrace_annotated_branch)) \
- __stop_annotated_branch_profile = .;
+#define LIKELY_PROFILE() \
+ BOUNDED_SECTION_BY(_ftrace_annotated_branch, _annotated_branch_profile)
#else
#define LIKELY_PROFILE()
#endif

#ifdef CONFIG_PROFILE_ALL_BRANCHES
-#define BRANCH_PROFILE() __start_branch_profile = .; \
- KEEP(*(_ftrace_branch)) \
- __stop_branch_profile = .;
+#define BRANCH_PROFILE() \
+ BOUNDED_SECTION_BY(_ftrace_branch, _branch_profile)
#else
#define BRANCH_PROFILE()
#endif

#ifdef CONFIG_KPROBES
-#define KPROBE_BLACKLIST() . = ALIGN(8); \
- __start_kprobe_blacklist = .; \
- KEEP(*(_kprobe_blacklist)) \
- __stop_kprobe_blacklist = .;
+#define KPROBE_BLACKLIST() \
+ . = ALIGN(8); \
+ BOUNDED_SECTION(_kprobe_blacklist)
#else
#define KPROBE_BLACKLIST()
#endif

#ifdef CONFIG_FUNCTION_ERROR_INJECTION
-#define ERROR_INJECT_WHITELIST() STRUCT_ALIGN(); \
- __start_error_injection_whitelist = .; \
- KEEP(*(_error_injection_whitelist)) \
- __stop_error_injection_whitelist = .;
+#define ERROR_INJECT_WHITELIST() \
+ STRUCT_ALIGN(); \
+ BOUNDED_SECTION(_error_injection_whitelist)
#else
#define ERROR_INJECT_WHITELIST()
#endif

#ifdef CONFIG_EVENT_TRACING
-#define FTRACE_EVENTS() . = ALIGN(8); \
- __start_ftrace_events = .; \
- KEEP(*(_ftrace_events)) \
- __stop_ftrace_events = .; \
- __start_ftrace_eval_maps = .; \
- KEEP(*(_ftrace_eval_map)) \
- __stop_ftrace_eval_maps = .;
+#define FTRACE_EVENTS() \
+ . = ALIGN(8); \
+ BOUNDED_SECTION(_ftrace_events) \
+ BOUNDED_SECTION_BY(_ftrace_eval_map, _ftrace_eval_maps)
#else
#define FTRACE_EVENTS()
#endif

#ifdef CONFIG_TRACING
-#define TRACE_PRINTKS() __start___trace_bprintk_fmt = .; \
- KEEP(*(__trace_printk_fmt)) /* Trace_printk fmt' pointer */ \
- __stop___trace_bprintk_fmt = .;
-#define TRACEPOINT_STR() __start___tracepoint_str = .; \
- KEEP(*(__tracepoint_str)) /* Trace_printk fmt' pointer */ \
- __stop___tracepoint_str = .;
+#define TRACE_PRINTKS() BOUNDED_SECTION_BY(__trace_printk_fmt, ___trace_bprintk_fmt)
+#define TRACEPOINT_STR() BOUNDED_SECTION_BY(__tracepoint_str, ___tracepoint_str)
#else
#define TRACE_PRINTKS()
#define TRACEPOINT_STR()
#endif

#ifdef CONFIG_FTRACE_SYSCALLS
-#define TRACE_SYSCALLS() . = ALIGN(8); \
- __start_syscalls_metadata = .; \
- KEEP(*(__syscalls_metadata)) \
- __stop_syscalls_metadata = .;
+#define TRACE_SYSCALLS() \
+ . = ALIGN(8); \
+ BOUNDED_SECTION_BY(__syscalls_metadata, _syscalls_metadata)
#else
#define TRACE_SYSCALLS()
#endif

#ifdef CONFIG_BPF_EVENTS
-#define BPF_RAW_TP() STRUCT_ALIGN(); \
- __start__bpf_raw_tp = .; \
- KEEP(*(__bpf_raw_tp_map)) \
- __stop__bpf_raw_tp = .;
+#define BPF_RAW_TP() STRUCT_ALIGN(); \
+ BOUNDED_SECTION_BY(__bpf_raw_tp_map, __bpf_raw_tp)
#else
#define BPF_RAW_TP()
#endif

#ifdef CONFIG_SERIAL_EARLYCON
-#define EARLYCON_TABLE() . = ALIGN(8); \
- __earlycon_table = .; \
- KEEP(*(__earlycon_table)) \
- __earlycon_table_end = .;
+#define EARLYCON_TABLE() \
+ . = ALIGN(8); \
+ BOUNDED_SECTION_POST_LABEL(__earlycon_table, __earlycon_table, , _end)
#else
#define EARLYCON_TABLE()
#endif

#ifdef CONFIG_SECURITY
-#define LSM_TABLE() . = ALIGN(8); \
- __start_lsm_info = .; \
- KEEP(*(.lsm_info.init)) \
- __end_lsm_info = .;
-#define EARLY_LSM_TABLE() . = ALIGN(8); \
- __start_early_lsm_info = .; \
- KEEP(*(.early_lsm_info.init)) \
- __end_early_lsm_info = .;
+#define LSM_TABLE() \
+ . = ALIGN(8); \
+ BOUNDED_SECTION_PRE_LABEL(.lsm_info.init, _lsm_info, __start, __end)
+
+#define EARLY_LSM_TABLE() \
+ . = ALIGN(8); \
+ BOUNDED_SECTION_PRE_LABEL(.early_lsm_info.init, _early_lsm_info, __start, __end)
#else
#define LSM_TABLE()
#define EARLY_LSM_TABLE()
@@ -312,9 +311,8 @@
#ifdef CONFIG_ACPI
#define ACPI_PROBE_TABLE(name) \
. = ALIGN(8); \
- __##name##_acpi_probe_table = .; \
- KEEP(*(__##name##_acpi_probe_table)) \
- __##name##_acpi_probe_table_end = .;
+ BOUNDED_SECTION_POST_LABEL(__##name##_acpi_probe_table, \
+ __##name##_acpi_probe_table,, _end)
#else
#define ACPI_PROBE_TABLE(name)
#endif
@@ -322,9 +320,8 @@
#ifdef CONFIG_THERMAL
#define THERMAL_TABLE(name) \
. = ALIGN(8); \
- __##name##_thermal_table = .; \
- KEEP(*(__##name##_thermal_table)) \
- __##name##_thermal_table_end = .;
+ BOUNDED_SECTION_POST_LABEL(__##name##_thermal_table, \
+ __##name##_thermal_table,, _end)
#else
#define THERMAL_TABLE(name)
#endif
@@ -353,12 +350,8 @@
*(__tracepoints) \
/* implement dynamic printk debug */ \
. = ALIGN(8); \
- __start___dyndbg_classes = .; \
- KEEP(*(__dyndbg_classes)) \
- __stop___dyndbg_classes = .; \
- __start___dyndbg = .; \
- KEEP(*(__dyndbg)) \
- __stop___dyndbg = .; \
+ BOUNDED_SECTION_BY(__dyndbg_classes, ___dyndbg_classes) \
+ BOUNDED_SECTION_BY(__dyndbg, ___dyndbg) \
LIKELY_PROFILE() \
BRANCH_PROFILE() \
TRACE_PRINTKS() \
@@ -401,19 +394,13 @@

#define JUMP_TABLE_DATA \
. = ALIGN(8); \
- __start___jump_table = .; \
- KEEP(*(__jump_table)) \
- __stop___jump_table = .;
+ BOUNDED_SECTION_BY(__jump_table, ___jump_table)

#ifdef CONFIG_HAVE_STATIC_CALL_INLINE
#define STATIC_CALL_DATA \
. = ALIGN(8); \
- __start_static_call_sites = .; \
- KEEP(*(.static_call_sites)) \
- __stop_static_call_sites = .; \
- __start_static_call_tramp_key = .; \
- KEEP(*(.static_call_tramp_key)) \
- __stop_static_call_tramp_key = .;
+ BOUNDED_SECTION_BY(.static_call_sites, _static_call_sites) \
+ BOUNDED_SECTION_BY(.static_call_tramp_key, _static_call_tramp_key)
#else
#define STATIC_CALL_DATA
#endif
@@ -439,9 +426,7 @@
#ifdef CONFIG_ARCH_USES_CFI_TRAPS
#define KCFI_TRAPS \
__kcfi_traps : AT(ADDR(__kcfi_traps) - LOAD_OFFSET) { \
- __start___kcfi_traps = .; \
- KEEP(*(.kcfi_traps)) \
- __stop___kcfi_traps = .; \
+ BOUNDED_SECTION_BY(.kcfi_traps, ___kcfi_traps) \
}
#else
#define KCFI_TRAPS
@@ -459,9 +444,7 @@
SCHED_DATA \
RO_AFTER_INIT_DATA /* Read only after init */ \
. = ALIGN(8); \
- __start___tracepoints_ptrs = .; \
- KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \
- __stop___tracepoints_ptrs = .; \
+ BOUNDED_SECTION_BY(__tracepoints_ptrs, ___tracepoints_ptrs) \
*(__tracepoints_strings)/* Tracepoints: strings */ \
} \
\
@@ -471,30 +454,14 @@
\
/* PCI quirks */ \
.pci_fixup : AT(ADDR(.pci_fixup) - LOAD_OFFSET) { \
- __start_pci_fixups_early = .; \
- KEEP(*(.pci_fixup_early)) \
- __end_pci_fixups_early = .; \
- __start_pci_fixups_header = .; \
- KEEP(*(.pci_fixup_header)) \
- __end_pci_fixups_header = .; \
- __start_pci_fixups_final = .; \
- KEEP(*(.pci_fixup_final)) \
- __end_pci_fixups_final = .; \
- __start_pci_fixups_enable = .; \
- KEEP(*(.pci_fixup_enable)) \
- __end_pci_fixups_enable = .; \
- __start_pci_fixups_resume = .; \
- KEEP(*(.pci_fixup_resume)) \
- __end_pci_fixups_resume = .; \
- __start_pci_fixups_resume_early = .; \
- KEEP(*(.pci_fixup_resume_early)) \
- __end_pci_fixups_resume_early = .; \
- __start_pci_fixups_suspend = .; \
- KEEP(*(.pci_fixup_suspend)) \
- __end_pci_fixups_suspend = .; \
- __start_pci_fixups_suspend_late = .; \
- KEEP(*(.pci_fixup_suspend_late)) \
- __end_pci_fixups_suspend_late = .; \
+ BOUNDED_SECTION_PRE_LABEL(.pci_fixup_early, _pci_fixups_early, __start, __end) \
+ BOUNDED_SECTION_PRE_LABEL(.pci_fixup_header, _pci_fixups_header, __start, __end) \
+ BOUNDED_SECTION_PRE_LABEL(.pci_fixup_final, _pci_fixups_final, __start, __end) \
+ BOUNDED_SECTION_PRE_LABEL(.pci_fixup_enable, _pci_fixups_enable, __start, __end) \
+ BOUNDED_SECTION_PRE_LABEL(.pci_fixup_resume, _pci_fixups_resume, __start, __end) \
+ BOUNDED_SECTION_PRE_LABEL(.pci_fixup_suspend, _pci_fixups_suspend, __start, __end) \
+ BOUNDED_SECTION_PRE_LABEL(.pci_fixup_resume_early, _pci_fixups_resume_early, __start, __end) \
+ BOUNDED_SECTION_PRE_LABEL(.pci_fixup_suspend_late, _pci_fixups_suspend_late, __start, __end) \
} \
\
FW_LOADER_BUILT_IN_DATA \
@@ -544,16 +511,12 @@
\
/* Built-in module parameters. */ \
__param : AT(ADDR(__param) - LOAD_OFFSET) { \
- __start___param = .; \
- KEEP(*(__param)) \
- __stop___param = .; \
+ BOUNDED_SECTION_BY(__param, ___param) \
} \
\
/* Built-in module versions. */ \
__modver : AT(ADDR(__modver) - LOAD_OFFSET) { \
- __start___modver = .; \
- KEEP(*(__modver)) \
- __stop___modver = .; \
+ BOUNDED_SECTION_BY(__modver, ___modver) \
} \
\
KCFI_TRAPS \
@@ -663,9 +626,7 @@
#define EXCEPTION_TABLE(align) \
. = ALIGN(align); \
__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) { \
- __start___ex_table = .; \
- KEEP(*(__ex_table)) \
- __stop___ex_table = .; \
+ BOUNDED_SECTION_BY(__ex_table, ___ex_table) \
}

/*
@@ -674,9 +635,7 @@
#ifdef CONFIG_DEBUG_INFO_BTF
#define BTF \
.BTF : AT(ADDR(.BTF) - LOAD_OFFSET) { \
- __start_BTF = .; \
- KEEP(*(.BTF)) \
- __stop_BTF = .; \
+ BOUNDED_SECTION_BY(.BTF, _BTF) \
} \
. = ALIGN(4); \
.BTF_ids : AT(ADDR(.BTF_ids) - LOAD_OFFSET) { \
@@ -853,9 +812,7 @@
#define BUG_TABLE \
. = ALIGN(8); \
__bug_table : AT(ADDR(__bug_table) - LOAD_OFFSET) { \
- __start___bug_table = .; \
- KEEP(*(__bug_table)) \
- __stop___bug_table = .; \
+ BOUNDED_SECTION_BY(__bug_table, ___bug_table) \
}
#else
#define BUG_TABLE
@@ -865,15 +822,11 @@
#define ORC_UNWIND_TABLE \
. = ALIGN(4); \
.orc_unwind_ip : AT(ADDR(.orc_unwind_ip) - LOAD_OFFSET) { \
- __start_orc_unwind_ip = .; \
- KEEP(*(.orc_unwind_ip)) \
- __stop_orc_unwind_ip = .; \
+ BOUNDED_SECTION_BY(.orc_unwind_ip, _orc_unwind_ip) \
} \
. = ALIGN(2); \
.orc_unwind : AT(ADDR(.orc_unwind) - LOAD_OFFSET) { \
- __start_orc_unwind = .; \
- KEEP(*(.orc_unwind)) \
- __stop_orc_unwind = .; \
+ BOUNDED_SECTION_BY(.orc_unwind, _orc_unwind) \
} \
text_size = _etext - _stext; \
. = ALIGN(4); \
@@ -891,9 +844,7 @@
#ifdef CONFIG_FW_LOADER
#define FW_LOADER_BUILT_IN_DATA \
.builtin_fw : AT(ADDR(.builtin_fw) - LOAD_OFFSET) ALIGN(8) { \
- __start_builtin_fw = .; \
- KEEP(*(.builtin_fw)) \
- __end_builtin_fw = .; \
+ BOUNDED_SECTION_PRE_LABEL(.builtin_fw, _builtin_fw, __start, __end) \
}
#else
#define FW_LOADER_BUILT_IN_DATA
@@ -903,9 +854,7 @@
#define TRACEDATA \
. = ALIGN(4); \
.tracedata : AT(ADDR(.tracedata) - LOAD_OFFSET) { \
- __tracedata_start = .; \
- KEEP(*(.tracedata)) \
- __tracedata_end = .; \
+ BOUNDED_SECTION_POST_LABEL(.tracedata, __tracedata, _start, _end) \
}
#else
#define TRACEDATA
@@ -914,9 +863,7 @@
#ifdef CONFIG_PRINTK_INDEX
#define PRINTK_INDEX \
.printk_index : AT(ADDR(.printk_index) - LOAD_OFFSET) { \
- __start_printk_index = .; \
- *(.printk_index) \
- __stop_printk_index = .; \
+ BOUNDED_SECTION_BY(.printk_index, _printk_index) \
}
#else
#define PRINTK_INDEX
@@ -924,17 +871,13 @@

#define NOTES \
.notes : AT(ADDR(.notes) - LOAD_OFFSET) { \
- __start_notes = .; \
- KEEP(*(.note.*)) \
- __stop_notes = .; \
+ BOUNDED_SECTION_BY(.note.*, _notes) \
} NOTES_HEADERS \
NOTES_HEADERS_RESTORE

#define INIT_SETUP(initsetup_align) \
. = ALIGN(initsetup_align); \
- __setup_start = .; \
- KEEP(*(.init.setup)) \
- __setup_end = .;
+ BOUNDED_SECTION_POST_LABEL(.init.setup, __setup, _start, _end)

#define INIT_CALLS_LEVEL(level) \
__initcall##level##_start = .; \
@@ -956,16 +899,12 @@
__initcall_end = .;

#define CON_INITCALL \
- __con_initcall_start = .; \
- KEEP(*(.con_initcall.init)) \
- __con_initcall_end = .;
+ BOUNDED_SECTION_POST_LABEL(.con_initcall.init, __con_initcall, _start, _end)

/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
#define KUNIT_TABLE() \
. = ALIGN(8); \
- __kunit_suites_start = .; \
- KEEP(*(.kunit_test_suites)) \
- __kunit_suites_end = .;
+ BOUNDED_SECTION_POST_LABEL(.kunit_test_suites, __kunit_suites, _start, _end)

#ifdef CONFIG_BLK_DEV_INITRD
#define INIT_RAM_FS \
--
2.37.3

2022-11-10 18:35:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 0/2] RESEND vmlinux.lds.h tweaks

On Sat, Oct 22, 2022 at 04:56:35PM -0600, Jim Cromie wrote:
> hi Greg,
>
> this time w/o the stale patch 2.
>
> These 2 patches are "no functional change", but they are a simple step
> towards de-duplicating the repetitive columms in the __dyndbg section.
>
> For a DYNAMIC_DEBUG=y kernel with 5k pr_debugs/drm.debugs, the
> footprint reduction should be ~100 KiB

Cool stuff, let me add it to my tree and see what breaks! :)

greg k-h

2022-11-11 23:08:17

by Jim Cromie

[permalink] [raw]
Subject: Re: [PATCH 0/2] RESEND vmlinux.lds.h tweaks

On Thu, Nov 10, 2022 at 11:09 AM Greg KH <[email protected]> wrote:
>
> On Sat, Oct 22, 2022 at 04:56:35PM -0600, Jim Cromie wrote:
> > hi Greg,
> >
> > this time w/o the stale patch 2.
> >
> > These 2 patches are "no functional change", but they are a simple step
> > towards de-duplicating the repetitive columms in the __dyndbg section.
> >
> > For a DYNAMIC_DEBUG=y kernel with 5k pr_debugs/drm.debugs, the
> > footprint reduction should be ~100 KiB
>
> Cool stuff, let me add it to my tree and see what breaks! :)
>
> greg k-h

very good, thnks.

on a rev2, I'd change _s_ _e_ macro vars,
maybe _BEGIN_, _END_, or _1ST_, _LAST_

and maybe expand the commit-msgs for more explanation.

2022-11-17 00:33:33

by Jim Cromie

[permalink] [raw]
Subject: [driver-core-next] vmlinux.lds.h fix

hi Greg,

You recently applied to driver-core-next: 2 vmlinux.lds.h patches from
me. The 2nd has a subtle error, placing the optional header KEEP in
with the data.

1st patch fixes it by dropping the extra KEEP, restoring basic behavior.

2nd redoes the HEADERD_SECTION idea with separate macros, so it cant
affect the basic case. HEADERD_SECTION is not the name in code, maybe
it should be ?

Also, 2nd is NOT in real/purposeful use yet.

I should have stared at this patchset longer before sending.
sorry about that.

Jim Cromie (2):
vmlinux.lds.h: fix BOUNDED_SECTION_(PRE|POST)_LABEL macros
vmlinux.lds.h: add BOUNDED_SECTION_(PRE|POST)_LABEL_HDR macros

include/asm-generic/vmlinux.lds.h | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)

--
2.38.1


2022-11-17 00:33:53

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 1/2] vmlinux.lds.h: fix BOUNDED_SECTION_(PRE|POST)_LABEL macros

commit foo added BOUNDED_SECTION_(PRE|POST)_LABEL macros,
encapsulating the basic boilerplate to: KEEP/pack records into a
section, and to mark the begin and end of the section with
linker-symbols.

But it tried to do extra, adding KEEP(*(.gnu.linkonce.##_sec_)) to
optionally reserve a header record in front of the data. It wrongly
placed the KEEP after the linker-symbol starting the section,
so if a header was added, it would wind up in the data.

Putting the KEEP in the "correct" place proved brittle, and too clever
by half. The obvious safe fix is to remove the KEEP, and provide
separate macros to do the extra work.

While here, the macro var-names: _s_, _e_ are nearly invisible, change
them to more obvious names: _BEGIN_, _END_

Signed-off-by: Jim Cromie <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 50851425b229..85d5d5b203dc 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -199,17 +199,15 @@
# endif
#endif

-#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _s_, _e_) \
- _s_##_label_ = .; \
- KEEP(*(.gnu.linkonce.##_sec_)) \
+#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+ _BEGIN_##_label_ = .; \
KEEP(*(_sec_)) \
- _e_##_label_ = .;
+ _END_##_label_ = .;

-#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _s_, _e_) \
- _label_##_s_ = .; \
- KEEP(*(.gnu.linkonce.##_sec_)) \
+#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+ _label_##_BEGIN_ = .; \
KEEP(*(_sec_)) \
- _label_##_e_ = .;
+ _label_##_END_ = .;

#define BOUNDED_SECTION_BY(_sec_, _label_) \
BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
--
2.38.1


2022-11-17 06:54:25

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] vmlinux.lds.h: fix BOUNDED_SECTION_(PRE|POST)_LABEL macros

On Wed, Nov 16, 2022 at 05:20:21PM -0700, Jim Cromie wrote:
> commit foo added BOUNDED_SECTION_(PRE|POST)_LABEL macros,
> encapsulating the basic boilerplate to: KEEP/pack records into a
> section, and to mark the begin and end of the section with
> linker-symbols.
>
> But it tried to do extra, adding KEEP(*(.gnu.linkonce.##_sec_)) to
> optionally reserve a header record in front of the data. It wrongly
> placed the KEEP after the linker-symbol starting the section,
> so if a header was added, it would wind up in the data.
>
> Putting the KEEP in the "correct" place proved brittle, and too clever
> by half. The obvious safe fix is to remove the KEEP, and provide
> separate macros to do the extra work.
>
> While here, the macro var-names: _s_, _e_ are nearly invisible, change
> them to more obvious names: _BEGIN_, _END_
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> include/asm-generic/vmlinux.lds.h | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)

This change fails to apply to my driver-core-next tree. Are you sure it
is correct?

confused,

greg k-h

2022-11-17 17:39:54

by Jim Cromie

[permalink] [raw]
Subject: [driver-core-next] vmlinux.lds.h fix (corrected)

hi Greg,

Im not quite sure what went wrong with last rev.
Im intrinsically noisy. Its hard to fix permamently.

1st patch restores basic BOUNDED_SECTION wo header reservation.

2nd redoes the HEADERED_SECTION idea with separate macros, so it cant
affect the basic case. I havent actually used this yet.

I should have stared at this patchset longer before sending.
sorry about that.

Jim Cromie (2):
vmlinux.lds.h: fix BOUNDED_SECTION_(PRE|POST)_LABEL macros
vmlinux.lds.h: add HEADERED_SECTION_* macros

include/asm-generic/vmlinux.lds.h | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)

--
2.38.1


2022-11-17 17:49:45

by Jim Cromie

[permalink] [raw]
Subject: Re: [PATCH 1/2] vmlinux.lds.h: fix BOUNDED_SECTION_(PRE|POST)_LABEL macros

On Wed, Nov 16, 2022 at 11:30 PM Greg KH <[email protected]> wrote:
>
> On Wed, Nov 16, 2022 at 05:20:21PM -0700, Jim Cromie wrote:
> > commit foo added BOUNDED_SECTION_(PRE|POST)_LABEL macros,
> > encapsulating the basic boilerplate to: KEEP/pack records into a
> > section, and to mark the begin and end of the section with
> > linker-symbols.
> >
> > But it tried to do extra, adding KEEP(*(.gnu.linkonce.##_sec_)) to
> > optionally reserve a header record in front of the data. It wrongly
> > placed the KEEP after the linker-symbol starting the section,
> > so if a header was added, it would wind up in the data.
> >
> > Putting the KEEP in the "correct" place proved brittle, and too clever
> > by half. The obvious safe fix is to remove the KEEP, and provide
> > separate macros to do the extra work.
> >
> > While here, the macro var-names: _s_, _e_ are nearly invisible, change
> > them to more obvious names: _BEGIN_, _END_
> >
> > Signed-off-by: Jim Cromie <[email protected]>
> > ---
> > include/asm-generic/vmlinux.lds.h | 14 ++++++--------
> > 1 file changed, 6 insertions(+), 8 deletions(-)
>
> This change fails to apply to my driver-core-next tree. Are you sure it
> is correct?
>

meh - it seems I missed a failure of a git commit --amend,
since im sure I edited a fixes: tag into this commit-msg
or something :-/

I'll send an update shortly, heres the HEAD of it:

b8b7f5a7a624 (HEAD -> bounded-5) vmlinux.lds.h: add HEADERED_SECTION_* macros
d712ed004b64 vmlinux.lds.h: fix BOUNDED_SECTION_(PRE|POST)_LABEL macros
f613facc82cf (driver-core/driver-core-testing,
driver-core/driver-core-next) mfd: vexpress-sysreg: Fix resource
compound literal assignments
2f465b921bb8 vmlinux.lds.h: place optional header space in BOUNDED_SECTION
9b351be25360 vmlinux.lds.h: add BOUNDED_SECTION* macros


> confused,
>
> greg k-h

2022-11-17 17:53:24

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 1/2] vmlinux.lds.h: fix BOUNDED_SECTION_(PRE|POST)_LABEL macros

Commit 2f465b921bb8 ("vmlinux.lds.h: place optional header space in BOUNDED_SECTION")

added BOUNDED_SECTION_(PRE|POST)_LABEL macros, encapsulating the basic
boilerplate to KEEP/pack records into a section, and to mark the begin
and end of the section with linker-symbols.

But it tried to do extra, adding KEEP(*(.gnu.linkonce.##_sec_)) to
optionally reserve a header record in front of the data. It wrongly
placed the KEEP after the linker-symbol starting the section,
so if a header was added, it would wind up in the data.

Moving the KEEP to the "correct" place proved brittle, and too clever
by half. The obvious safe fix is to remove the KEEP and restore the
plain old boilerplate. The header can be added later, with separate
macros.

Also, the macro var-names: _s_, _e_ are nearly invisible, change them
to more obvious names: _BEGIN_, _END_

Fixes: 2f465b921bb8 ("vmlinux.lds.h: place optional header space in BOUNDED_SECTION")
Signed-off-by: Jim Cromie <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index b3ca56ac163f..c17f94785253 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -193,17 +193,15 @@
# endif
#endif

-#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _s_, _e_) \
- _s_##_label_ = .; \
- KEEP(*(.gnu.linkonce.##_sec_)) \
+#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+ _BEGIN_##_label_ = .; \
KEEP(*(_sec_)) \
- _e_##_label_ = .;
+ _END_##_label_ = .;

-#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _s_, _e_) \
- _label_##_s_ = .; \
- KEEP(*(.gnu.linkonce.##_sec_)) \
+#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+ _label_##_BEGIN_ = .; \
KEEP(*(_sec_)) \
- _label_##_e_ = .;
+ _label_##_END_ = .;

#define BOUNDED_SECTION_BY(_sec_, _label_) \
BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
--
2.38.1


2022-11-17 19:29:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [driver-core-next] vmlinux.lds.h fix (corrected)

On Thu, Nov 17, 2022 at 10:16:31AM -0700, Jim Cromie wrote:
> hi Greg,
>
> Im not quite sure what went wrong with last rev.
> Im intrinsically noisy. Its hard to fix permamently.
>
> 1st patch restores basic BOUNDED_SECTION wo header reservation.
>
> 2nd redoes the HEADERED_SECTION idea with separate macros, so it cant
> affect the basic case. I havent actually used this yet.
>
> I should have stared at this patchset longer before sending.
> sorry about that.

Worked better, thanks, now applied.

greg k-h