diff mbox series

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

Message ID 20221117002022.860237-2-jim.cromie@gmail.com
State New
Headers
Series [1/2] vmlinux.lds.h: fix BOUNDED_SECTION_(PRE|POST)_LABEL macros |

Commit Message

Jim Cromie Nov. 17, 2022, 12:20 a.m. UTC 
  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(-)

Comments

Greg KH Nov. 17, 2022, 6:29 a.m. UTC | #1 
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
Jim Cromie Nov. 17, 2022, 4:43 p.m. UTC | #2 
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
Jim Cromie Nov. 17, 2022, 5:16 p.m. UTC | #3 
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(-)
Greg KH Nov. 17, 2022, 6:38 p.m. UTC | #4 
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
diff mbox series

Patch

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)