diff mbox

[RFC] linkage: new macros for functions and data

Message ID 20170307172404.27374-1-jslaby@suse.cz (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Jiri Slaby March 7, 2017, 5:24 p.m. UTC
SYM_LOCAL_ALIAS_START -- use where there are two local names for one
  code
SYM_ALIAS_START -- use where there are two global names for one code
SYM_LOCAL_FUNC_START -- use for local functions
SYM_FUNCTION_START -- use for global functions
SYM_WEAK_FUNC_START -- use for weak functions
SYM_ALIAS_END -- the end of LOCALALIASed or ALIASed code
SYM_FUNCTION_END -- the end of SYM_LOCAL_FUNC_START, SYM_FUNCTION_START,
  SYM_WEAK_FUNC_START, ...
SYM_DATA_START -- global data symbol
SYM_DATA_END -- the end of SYM_DATA_START symbol

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: hpa@zytor.com
Cc: Ingo Molnar <mingo@kernel.org>
Cc: jpoimboe@redhat.com
Cc: Juergen Gross <jgross@suse.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: mingo@redhat.com
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel@lists.xenproject.org
Cc: x86@kernel.org
---

So this is what I have ATM.

 include/linux/linkage.h | 87 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 15 deletions(-)

Comments

Ingo Molnar March 16, 2017, 8:02 a.m. UTC | #1
* Jiri Slaby <jslaby@suse.cz> wrote:

> SYM_LOCAL_ALIAS_START -- use where there are two local names for one code
> SYM_ALIAS_START -- use where there are two global names for one code
> SYM_LOCAL_FUNC_START -- use for local functions
> SYM_FUNCTION_START -- use for global functions
> SYM_WEAK_FUNC_START -- use for weak functions
> SYM_ALIAS_END -- the end of LOCALALIASed or ALIASed code
> SYM_FUNCTION_END -- the end of SYM_LOCAL_FUNC_START, SYM_FUNCTION_START, SYM_WEAK_FUNC_START, ...
> SYM_DATA_START -- global data symbol
> SYM_DATA_END -- the end of SYM_DATA_START symbol

This looks mostly good to me, with minor details:

- The mixed 'FUNC' and 'FUNCTION' naming looks a bit confusing - I'd pick one and
  use that consistently.

- I'd also make the START/END constructs look more symmetric, i.e. make the 
  attribute a postfix, not a prefix.

- In fact I'd make the 'alias' notion an attribute as well - what matters mostly 
  is that it's a function symbol, and that fact is lost from the SYM_ALIAS naming.

I.e. something like this:

	SYM_FUNCTION_START
	SYM_FUNCTION_START_WEAK
	SYM_FUNCTION_START_LOCAL
	SYM_FUNCTION_START_ALIAS
	SYM_FUNCTION_START_LOCAL_ALIAS
	...
	SYM_FUNCTION_END

	SYM_DATA_START
	SYM_DATA_END

Note how simple and structured looking the nomenclature is when grouped in such a 
way, how easy it is to verify at a glance, and how easy it is to extend with new 
postfixes.

( In theory we could also make the attributes a real macro argument in the future,
  should the number of attributes increase significantly. )

Does this look good to everyone?

Thanks,

	Ingo
Jiri Slaby March 16, 2017, 8:13 a.m. UTC | #2
On 03/16/2017, 09:02 AM, Ingo Molnar wrote:
> 
> * Jiri Slaby <jslaby@suse.cz> wrote:
> 
>> SYM_LOCAL_ALIAS_START -- use where there are two local names for one code
>> SYM_ALIAS_START -- use where there are two global names for one code
>> SYM_LOCAL_FUNC_START -- use for local functions
>> SYM_FUNCTION_START -- use for global functions
>> SYM_WEAK_FUNC_START -- use for weak functions
>> SYM_ALIAS_END -- the end of LOCALALIASed or ALIASed code
>> SYM_FUNCTION_END -- the end of SYM_LOCAL_FUNC_START, SYM_FUNCTION_START, SYM_WEAK_FUNC_START, ...
>> SYM_DATA_START -- global data symbol
>> SYM_DATA_END -- the end of SYM_DATA_START symbol
> 
> This looks mostly good to me, with minor details:
> 
> - The mixed 'FUNC' and 'FUNCTION' naming looks a bit confusing - I'd pick one and
>   use that consistently.

Of course, I did it later after sending the RFC. I stuck to FUNC.

...
> Does this look good to everyone?

Fine by me. I will send patches for that, so that you can comment on
that on real uses.

thanks,
diff mbox

Patch

diff --git a/include/linux/linkage.h b/include/linux/linkage.h
index a6a42dd02466..79f634a57466 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -78,33 +78,90 @@ 
 #define ALIGN __ALIGN
 #define ALIGN_STR __ALIGN_STR
 
-#ifndef ENTRY
-#define ENTRY(name) \
-	.globl name ASM_NL \
+#ifndef ENTRY /* deprecated, use SYM_FUNCTION_START */
+#define ENTRY(name)	SYM_FUNCTION_START(name)
+#endif
+#endif /* LINKER_SCRIPT */
+
+/* === code annotations === */
+
+/* SYM_LOCAL_ALIAS_START -- use where there are two local names for one code */
+#ifndef SYM_LOCAL_ALIAS_START
+#define SYM_LOCAL_ALIAS_START(name) \
 	ALIGN ASM_NL \
 	name:
 #endif
-#endif /* LINKER_SCRIPT */
 
-#ifndef WEAK
-#define WEAK(name)	   \
+/* SYM_ALIAS_START -- use where there are two global names for one code */
+#ifndef SYM_ALIAS_START
+#define SYM_ALIAS_START(name) \
+	.globl name ASM_NL \
+	SYM_LOCAL_ALIAS_START(name)
+#endif
+
+/*
+ * so far the same as SYM_LOCAL_ALIAS_START, but we will need to distinguish
+ * them later
+ */
+/* SYM_LOCAL_FUNC_START -- use for local functions */
+#ifndef SYM_LOCAL_FUNC_START
+#define SYM_LOCAL_FUNC_START(name) \
+	SYM_LOCAL_ALIAS_START(name)
+#endif
+
+/* SYM_FUNCTION_START -- use for global functions */
+#ifndef SYM_FUNCTION_START
+#define SYM_FUNCTION_START(name) \
+	.globl name ASM_NL \
+	SYM_LOCAL_FUNC_START(name)
+#endif
+
+/* SYM_WEAK_FUNC_START -- use for weak functions */
+#ifndef SYM_WEAK_FUNC_START
+#define SYM_WEAK_FUNC_START(name) \
 	.weak name ASM_NL   \
 	name:
 #endif
 
-#ifndef END
-#define END(name) \
-	.size name, .-name
+/* SYM_ALIAS_END -- the end of LOCALALIASed or ALIASed code */
+#ifndef SYM_ALIAS_END
+#define SYM_ALIAS_END(name) \
+	.type name, @function ASM_NL \
+	SYM_DATA_END(name)
 #endif
 
 /* If symbol 'name' is treated as a subroutine (gets called, and returns)
- * then please use ENDPROC to mark 'name' as STT_FUNC for the benefit of
- * static analysis tools such as stack depth analyzer.
+ * then please use SYM_FUNCTION_END to mark 'name' as STT_FUNC for the benefit
+ * of static analysis tools such as stack depth analyzer.
  */
-#ifndef ENDPROC
-#define ENDPROC(name) \
-	.type name, @function ASM_NL \
-	END(name)
+/*
+ * SYM_FUNCTION_END -- the end of SYM_LOCAL_FUNC_START, SYM_FUNCTION_START,
+ * SYM_WEAK_FUNC_START, ...
+ */
+#ifndef SYM_FUNCTION_END
+#define SYM_FUNCTION_END(name) \
+	SYM_ALIAS_END(name) /* the same as for SYM_LOCAL_FUNC_START */
+#endif
+
+#ifndef WEAK /* deprecated, use SYM_WEAK_FUNC_START */
+#define WEAK(name)	SYM_WEAK_FUNC_START(name)
+#endif
+
+/* === data annotations === */
+
+/* SYM_DATA_START -- global data symbol */
+#ifndef SYM_DATA_START
+#define SYM_DATA_START(name)	SYM_FUNCTION_START(name)
+#endif
+
+/* SYM_DATA_END -- the end of SYM_DATA_START symbol */
+#ifndef SYM_DATA_END
+#define SYM_DATA_END(name) \
+	.size name, .-name
+#endif
+
+#ifndef END /* deprecated, use SYM_DATA_END */
+#define END(name) SYM_DATA_END(name)
 #endif
 
 #endif