Re: [PATCH] jump_label: align jump_entry table to at least 4-bytes

[Jason Baron <jbaron@akamai.com>]

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]



On 02/27/2017 05:45 PM, David Daney wrote:
> On 02/27/2017 02:36 PM, Steven Rostedt wrote:
>> On Mon, 27 Feb 2017 14:21:21 -0800
>> David Daney <ddaney@caviumnetworks.com> wrote:
>>
>>> See attached for mips.  It seems to do the right thing.
>>>
>>> I leave it as an exercise to the reader to fix the other architectures.
>>>
>>> Consult your own  binutils experts to verify that what I say is true.
>>
>> It may still just be safer to do the pointers instead. That way we
>> don't need to worry about some strange arch or off by one binutils
>> messing it up.
>
> Obviously it is your choice, but this is bog standard ELF linking.  In
> theory even the arrays of power-of-2 sized objects should also supply an
> entity size.  Think __ex_table and its ilk.
>
>
> The benefit of supplying an entsize is that you don't have to change the
> structure of the existing code and risk breaking something in the process.
>
> David Daney
>
>

Thanks for the suggestion! I would like to see if this resolves the ppc 
issue we had. I'm attaching a powerpc patch based on your suggestion. 
Hopefully, Sachin can try it.

Thanks,

-Jason

[-- Attachment #2: ppc-pack.patch --]
[-- Type: text/x-patch, Size: 1975 bytes --]

diff --git a/arch/powerpc/include/asm/jump_label.h b/arch/powerpc/include/asm/jump_label.h
index 9a287e0ac8b1..3c5660e50f9a 100644
--- a/arch/powerpc/include/asm/jump_label.h
+++ b/arch/powerpc/include/asm/jump_label.h
@@ -19,14 +19,26 @@
 #define JUMP_ENTRY_TYPE		stringify_in_c(FTR_ENTRY_LONG)
 #define JUMP_LABEL_NOP_SIZE	4
 
+#ifdef CONFIG_PPC64
+typedef u64 jump_label_t;
+#else
+typedef u32 jump_label_t;
+#endif
+
+struct jump_entry {
+	jump_label_t code;
+	jump_label_t target;
+	jump_label_t key;
+};
+
 static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
 {
 	asm_volatile_goto("1:\n\t"
 		 "nop # arch_static_branch\n\t"
-		 ".pushsection __jump_table,  \"aw\"\n\t"
+		 ".pushsection __jump_table,  \"awM\",@progbits, %1\n\t"
 		 JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
 		 ".popsection \n\t"
-		 : :  "i" (&((char *)key)[branch]) : : l_yes);
+		 : :  "i" (&((char *)key)[branch]), "i" (sizeof(struct jump_entry)) : : l_yes);
 
 	return false;
 l_yes:
@@ -37,32 +49,24 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
 {
 	asm_volatile_goto("1:\n\t"
 		 "b %l[l_yes] # arch_static_branch_jump\n\t"
-		 ".pushsection __jump_table,  \"aw\"\n\t"
+		 ".pushsection __jump_table,  \"awM\",@progbits, %1\n\t"
 		 JUMP_ENTRY_TYPE "1b, %l[l_yes], %c0\n\t"
 		 ".popsection \n\t"
-		 : :  "i" (&((char *)key)[branch]) : : l_yes);
+		 : :  "i" (&((char *)key)[branch]), "i" (sizeof(struct jump_entry)) : : l_yes);
 
 	return false;
 l_yes:
 	return true;
 }
 
-#ifdef CONFIG_PPC64
-typedef u64 jump_label_t;
+
 #else
-typedef u32 jump_label_t;
-#endif
 
-struct jump_entry {
-	jump_label_t code;
-	jump_label_t target;
-	jump_label_t key;
-};
+#define ENTRY_SIZE (ULONG_SIZE * 3)
 
-#else
 #define ARCH_STATIC_BRANCH(LABEL, KEY)		\
 1098:	nop;					\
-	.pushsection __jump_table, "aw";	\
+	.pushsection __jump_table, "awM",@progbits,ENTRY_SIZE; \
 	FTR_ENTRY_LONG 1098b, LABEL, KEY;	\
 	.popsection
 #endif