Hi Jay,
On Do, 2014-11-13 at 21:04 -0800, Jay Vosburgh wrote:
> [ adding Hannes to Cc, which I should've done initially ]
>
> David Miller <da...@davemloft.net> wrote:
>
> >From: Jay Vosburgh <jay.vosbu...@canonical.com>
> >Date: Thu, 13 Nov 2014 18:15:32 -0800
> >
> >> The "have feature" function, __intel_crc4_2_hash2, does not
> >> clobber %r8, and so the call does not panic on a system with
> >> X86_FEATURE_XMM4_2, although I'm not sure if that's a deliberate
> >> compiler action or just happenstance because __intel_crc4_2_hash2 uses
> >> fewer registers than __jhash2.
> >
> >Perhaps alternative calls can only be used with assembler routines
> >that use specific calling conventions, and they therefore generally
> >don't work with C functions?
>
> I don't know the answer to that, but a quick search suggests
> that arch_fast_hash and arch_fast_hash2 (both added by commit e5a2c899)
> may be the only cases of alternative calls that aren't supplying either
> single instructions or assembly language functions.
>
> From looking at how the alternative calls are implemented (code
> patching at boot or module load time from a table stored in a special
> section of the object file), I'm skeptical that the compiler could know
> what's the right thing to do.
>
> Hannes, can you shed any light on this?
Unfortunately I only tested this code with rhashtable, which itself
takes a function pointer to arch_fast_hash and thus doesn't need to care
about clobbering so much. As a first step, I am currently testing this
patch as a first step and check thoroughly. Thanks for the report:
diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
index a881d78..1b32c82 100644
--- a/arch/x86/include/asm/hash.h
+++ b/arch/x86/include/asm/hash.h
@@ -4,45 +4,12 @@
#include <linux/cpufeature.h>
#include <asm/alternative.h>
-u32 __intel_crc4_2_hash(const void *data, u32 len, u32 seed);
-u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed);
-
-/*
- * non-inline versions of jhash so gcc does not need to generate
- * duplicate code in every object file
- */
-u32 __jhash(const void *data, u32 len, u32 seed);
-u32 __jhash2(const u32 *data, u32 len, u32 seed);
-
/*
* for documentation of these functions please look into
* <include/asm-generic/hash.h>
*/
-static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
-{
- u32 hash;
-
- alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
-#ifdef CONFIG_X86_64
- "=a" (hash), "D" (data), "S" (len), "d" (seed));
-#else
- "=a" (hash), "a" (data), "d" (len), "c" (seed));
-#endif
- return hash;
-}
-
-static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
-{
- u32 hash;
-
- alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
-#ifdef CONFIG_X86_64
- "=a" (hash), "D" (data), "S" (len), "d" (seed));
-#else
- "=a" (hash), "a" (data), "d" (len), "c" (seed));
-#endif
- return hash;
-}
+u32 arch_fast_hash(const void *data, u32 len, u32 seed);
+u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed);
#endif /* __ASM_X86_HASH_H */
diff --git a/arch/x86/lib/hash.c b/arch/x86/lib/hash.c
index e143271..a0a7a7e 100644
--- a/arch/x86/lib/hash.c
+++ b/arch/x86/lib/hash.c
@@ -38,7 +38,7 @@
#include <linux/hash.h>
#include <linux/jhash.h>
-static inline u32 crc32_u32(u32 crc, u32 val)
+static u32 crc32_u32(u32 crc, u32 val)
{
#ifdef CONFIG_AS_CRC32
asm ("crc32l %1,%0\n" : "+r" (crc) : "rm" (val));
@@ -71,7 +71,6 @@ u32 __intel_crc4_2_hash(const void *data, u32 len, u32 seed)
return seed;
}
-EXPORT_SYMBOL(__intel_crc4_2_hash);
u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed)
{
@@ -82,16 +81,45 @@ u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed)
return seed;
}
-EXPORT_SYMBOL(__intel_crc4_2_hash2);
u32 __jhash(const void *data, u32 len, u32 seed)
{
return jhash(data, len, seed);
}
-EXPORT_SYMBOL(__jhash);
u32 __jhash2(const u32 *data, u32 len, u32 seed)
{
return jhash2(data, len, seed);
}
-EXPORT_SYMBOL(__jhash2);
+
+noinline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
+{
+ u32 hash;
+
+
+#ifdef CONFIG_X86_64
+ alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+ "=a" (hash), "D" (data), "S" (len), "d" (seed));
+#else
+ alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+ "=a" (hash), "a" (data), "d" (len), "c" (seed));
+#endif
+ return hash;
+}
+EXPORT_SYMBOL(arch_fast_hash);
+
+noinline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
+{
+ u32 hash;
+
+
+#ifdef CONFIG_X86_64
+ alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+ "=a" (hash), "D" (data), "S" (len), "d" (seed));
+#else
+ alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+ "=a" (hash), "a" (data), "d" (len), "c" (seed));
+#endif
+ return hash;
+}
+EXPORT_SYMBOL(arch_fast_hash2);
_______________________________________________
discuss mailing list
discuss@openvswitch.org
http://openvswitch.org/mailman/listinfo/discuss
