[PATCH 0/2] RESEND vmlinux.lds.h tweaks

[PATCH 0/2] RESEND vmlinux.lds.h tweaks

[Jim Cromie]

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 <surenb@google.com>
CC: Kent Overstreet <kent.overstreet@linux.dev>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

-- 
2.37.3

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

[Jim Cromie]

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 <surenb@google.com>
CC: Kent Overstreet <kent.overstreet@linux.dev>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---

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

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

[Jim Cromie]

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 <jim.cromie@gmail.com>
---
? 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

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

[Greg KH]

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

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

[jim.cromie]

On Thu, Nov 10, 2022 at 11:09 AM Greg KH <gregkh@linuxfoundation.org> 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.

[driver-core-next] vmlinux.lds.h fix

[Jim Cromie]

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

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

[Jim Cromie]

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 <jim.cromie@gmail.com>
---
 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

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

[Greg KH]

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 <jim.cromie@gmail.com>
> ---
>  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

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

[jim.cromie]

On Wed, Nov 16, 2022 at 11:30 PM Greg KH <gregkh@linuxfoundation.org> 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 <jim.cromie@gmail.com>
> > ---
> >  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

[driver-core-next] vmlinux.lds.h fix (corrected)

[Jim Cromie]

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

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

[Jim Cromie]

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 <jim.cromie@gmail.com>
---
 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

[PATCH 2/2] vmlinux.lds.h: add HEADERED_SECTION_* macros

[Jim Cromie]

These macros elaborate on BOUNDED_SECTION_(PRE|POST)_LABEL macros,
prepending an optional KEEP(.gnu.linkonce##_sec_) reservation, and a
linker-symbol to address it.

This allows a developer to define a header struct (which must fit with
the section's base struct-type), and could contain:

1- fields whose value is common to the entire set of data-records.
   This allows the header & data structs to specialize, complement
   each other, and shrink.

2- an uplink pointer to an organizing struct
   which refs other related/sub data-tables
   header record is addressable via the extern'd header linker-symbol

Once the linker-symbols created by the macro are ref'd extern in code,
that code can compute a record's index (ptr - start) in the "primary"
table, then use it to index into the related/sub tables.  Adding a
primary.map_* field foreach sub-table would then allow deduplication
and remapping of that sub-table.

This is aimed at dyndbg's struct _ddebug __dyndbg[] section, whose 3
columns: function, file, module are 50%, 90%, 100% redundant.  The
module column is fully recoverable after dynamic_debug_init() saves it
to each ddebug_table.module as the builtin __dyndbg[] table is parsed.

Given that those 3 columns use 24/56 of a _ddebug record, a dyndbg=y
kernel with ~5k callsites could reduce kernel memory substantially.
Returning that memory to the kernel buddy-allocator? is then possible.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
---
 include/asm-generic/vmlinux.lds.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index c17f94785253..c9a475a30803 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -208,6 +208,21 @@
 
 #define BOUNDED_SECTION(_sec)	 BOUNDED_SECTION_BY(_sec, _sec)
 
+#define HEADERED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
+	_HDR_##_label_	= .;						\
+	KEEP(*(.gnu.linkonce.##_sec_))					\
+	BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_)
+
+#define HEADERED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
+	_label_##_HDR_ = .;						\
+	KEEP(*(.gnu.linkonce.##_sec_))					\
+	BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_)
+
+#define HEADERED_SECTION_BY(_sec_, _label_)				\
+	HEADERED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
+
+#define HEADERED_SECTION(_sec)	 HEADERED_SECTION_BY(_sec, _sec)
+
 #ifdef CONFIG_TRACE_BRANCH_PROFILING
 #define LIKELY_PROFILE()						\
 	BOUNDED_SECTION_BY(_ftrace_annotated_branch, _annotated_branch_profile)
-- 
2.38.1

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

[Greg KH]

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

[PATCH 2/2] vmlinux.lds.h: add HEADERED_SECTION_* macros

[Jim Cromie]

These macros elaborate on BOUNDED_SECTION_(PRE|POST)_LABEL macros,
prepending an optional KEEP(.gnu.linkonce##_sec_) reservation, and a
linker-symbol to address it.

This allows a developer to define a header struct (which must fit with
the section's base struct-type), and could contain:

1- fields whose value is common to the entire set of data-records.
   This allows the header & data structs to specialize, complement
   each other, and shrink.

2- an uplink pointer to an organizing struct
   which refs other related/sub data-tables
   header record is addressable via the extern'd header linker-symbol

Once the linker-symbols created by the macro are ref'd extern in code,
that code can compute a record's index (ptr - start) in the "primary"
table, then use it to index into the related/sub tables.  Adding a
primary.map_* field foreach sub-table would then allow deduplication
and remapping of that sub-table.

This is aimed at dyndbg's struct _ddebug __dyndbg[] section, whose 3
columns: function, file, module are 50%, 90%, 100% redundant.  The
module column is fully recoverable after dynamic_debug_init() saves it
to each ddebug_table.module as the builtin __dyndbg[] table is parsed.

Given that those 3 columns use 24/56 of a prdbg record, a dyndbg=y
kernel with ~5k callsites could save substantially.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
---
 include/asm-generic/vmlinux.lds.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 85d5d5b203dc..a3b6aa30a525 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -214,6 +214,21 @@
 
 #define BOUNDED_SECTION(_sec)	 BOUNDED_SECTION_BY(_sec, _sec)
 
+#define HEADERED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
+	_HDR_##_label_	= .;						\
+	KEEP(*(.gnu.linkonce.##_sec_))					\
+	BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_)
+
+#define HEADERED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
+	_label_##_HDR_ = .;						\
+	KEEP(*(.gnu.linkonce.##_sec_))					\
+	BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_)
+
+#define HEADERED_SECTION_BY(_sec_, _label_)				\
+	HEADERED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
+
+#define HEADERED_SECTION(_sec)	 HEADERED_SECTION_BY(_sec, _sec)
+
 #ifdef CONFIG_TRACE_BRANCH_PROFILING
 #define LIKELY_PROFILE()						\
 	BOUNDED_SECTION_BY(_ftrace_annotated_branch, _annotated_branch_profile)
-- 
2.38.1

Re: [PATCH 2/2] vmlinux.lds.h: add HEADERED_SECTION_* macros

[Alexander Lobakin]

From: Jim Cromie <jim.cromie@gmail.com>
Date: Wed, 16 Nov 2022 17:20:22 -0700

> These macros elaborate on BOUNDED_SECTION_(PRE|POST)_LABEL macros,
> prepending an optional KEEP(.gnu.linkonce##_sec_) reservation, and a
> linker-symbol to address it.
> 
> This allows a developer to define a header struct (which must fit with
> the section's base struct-type), and could contain:

[...]

> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 85d5d5b203dc..a3b6aa30a525 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -214,6 +214,21 @@
>  
>  #define BOUNDED_SECTION(_sec)	 BOUNDED_SECTION_BY(_sec, _sec)
>  
> +#define HEADERED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
> +	_HDR_##_label_	= .;						\
> +	KEEP(*(.gnu.linkonce.##_sec_))					\
> +	BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_)
> +
> +#define HEADERED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
> +	_label_##_HDR_ = .;						\
> +	KEEP(*(.gnu.linkonce.##_sec_))					\
> +	BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_)
> +
> +#define HEADERED_SECTION_BY(_sec_, _label_)				\
> +	HEADERED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)

Now HEADERED_SECTION_PRE_LABEL() takes 5 arguments, but this line
passes only 4 to it. This went unnoticed probably due to that the
macro is not used anywhere, thus can't trigger a compiler error.
Would you prefer to fix it yourself or me to send the fix?[*]

> +
> +#define HEADERED_SECTION(_sec)	 HEADERED_SECTION_BY(_sec, _sec)
> +
>  #ifdef CONFIG_TRACE_BRANCH_PROFILING
>  #define LIKELY_PROFILE()						\
>  	BOUNDED_SECTION_BY(_ftrace_annotated_branch, _annotated_branch_profile)
> -- 
> 2.38.1

[*] If it needs fixing at all -- some people over the MLs say that
    if there's no trigger, then there's nothing to fix :clownface:

Thanks,
Olek

Re: [PATCH 2/2] vmlinux.lds.h: add HEADERED_SECTION_* macros

[Alexander Lobakin]

> From: Jim Cromie <jim.cromie@gmail.com>
> Date: Wed, 16 Nov 2022 17:20:22 -0700
> 
> > These macros elaborate on BOUNDED_SECTION_(PRE|POST)_LABEL macros,
> > prepending an optional KEEP(.gnu.linkonce##_sec_) reservation, and a
> > linker-symbol to address it.
> > 
> > This allows a developer to define a header struct (which must fit with
> > the section's base struct-type), and could contain:
> 
> [...]
> 
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index 85d5d5b203dc..a3b6aa30a525 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -214,6 +214,21 @@
> >  
> >  #define BOUNDED_SECTION(_sec)	 BOUNDED_SECTION_BY(_sec, _sec)
> >  

Also, those two pasting ops below:

> > +#define HEADERED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
> > +	_HDR_##_label_	= .;						\
> > +	KEEP(*(.gnu.linkonce.##_sec_))					\

                             ^^

> > +	BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_)
> > +
> > +#define HEADERED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
> > +	_label_##_HDR_ = .;						\
> > +	KEEP(*(.gnu.linkonce.##_sec_))					\

                             ^^

will produce incorrect results when @_sec_ starts with a dot (not
a rare thing for section names):

	HEADERED_SECTION_BY(.entry_sites, _entry_sites)

vmlinux.map:

.entry_sites    0xffffffff83c89ca0    0x37118 load address 0x0000000003c89ca0
                0xffffffff83c89ca0                __header_entry_sites = .
 *(.gnu.linkonce. .entry_sites)

here            ^^^

 .entry_sites   0xffffffff83c89ca0    0x37118 vmlinux.o
                0xffffffff83cc0db8                __start_entry_sites = .
 *(.entry_sites)
                0xffffffff83cc0db8                __stop_entry_sites = .

^ as a result, all .entry_sites entries went to the "header" instead
of the "body".

Sorta dangerous stuff as for me, maybe that could be handled
differently?

> > +	BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_)
> > +
> > +#define HEADERED_SECTION_BY(_sec_, _label_)				\
> > +	HEADERED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
> 
> Now HEADERED_SECTION_PRE_LABEL() takes 5 arguments, but this line
> passes only 4 to it. This went unnoticed probably due to that the
> macro is not used anywhere, thus can't trigger a compiler error.
> Would you prefer to fix it yourself or me to send the fix?[*]
> 
> > +
> > +#define HEADERED_SECTION(_sec)	 HEADERED_SECTION_BY(_sec, _sec)
> > +
> >  #ifdef CONFIG_TRACE_BRANCH_PROFILING
> >  #define LIKELY_PROFILE()						\
> >  	BOUNDED_SECTION_BY(_ftrace_annotated_branch, _annotated_branch_profile)
> > -- 
> > 2.38.1
> 
> [*] If it needs fixing at all -- some people over the MLs say that
>     if there's no trigger, then there's nothing to fix :clownface:
> 
> Thanks,
> Olek

Thanks,
Olek