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

From: Jason Baron
Date: Mon Feb 27 2017 - 17:57:29 EST




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@xxxxxxxxxxxxxxxxxx> 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
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