Re: [PATCH net-next v3] fast_hash: clobber registers correctly for inline function use

[Jay Vosburgh <jay.vosburgh@canonical.com>]

Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:

>In case the arch_fast_hash call gets inlined we need to tell gcc which
>registers are clobbered with. rhashtable was fine, because it used
>arch_fast_hash via function pointer and thus the compiler took care of
>that. In case of openvswitch the call got inlined and arch_fast_hash
>touched registeres which gcc didn't know about.
>
>Also don't use conditional compilation inside arguments, as this confuses
>sparse.
>
>Fixes: e5a2c899957659c ("fast_hash: avoid indirect function calls")
>Reported-by: Jay Vosburgh <jay.vosburgh@canonical.com>
>Cc: Pravin Shelar <pshelar@nicira.com>
>Cc: Jesse Gross <jesse@nicira.com>
>Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>---
>
>
>v2)
>After studying gcc documentation again, it occured to me that I need to
>specificy all input operands in the clobber section, too. Otherwise gcc
>can expect that the inline assembler section won't modify the inputs,
>which is not true.
>
>v3)
>added Fixes tag

	This patch does not compile for me when applied to today's
net-next:

  CC [M]  fs/nfsd/nfs4state.o
In file included from ./arch/x86/include/asm/bitops.h:16:0,
                 from include/linux/bitops.h:33,
                 from include/linux/kernel.h:10,
                 from include/linux/list.h:8,
                 from include/linux/wait.h:6,
                 from include/linux/fs.h:6,
                 from fs/nfsd/nfs4state.c:36:
fs/nfsd/nfs4state.c: In function ‘nfsd4_cb_recall_prepare’:
./arch/x86/include/asm/alternative.h:185:2: error: ‘asm’ operand has impossible constraints
  asm volatile (ALTERNATIVE("call %P[old]", "call %P[new]", feature) \
  ^
./arch/x86/include/asm/hash.h:27:2: note: in expansion of macro ‘alternative_call’
  alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
  ^
make[2]: *** [fs/nfsd/nfs4state.o] Error 1
make[1]: *** [fs/nfsd] Error 2
make: *** [fs] Error 2

	-J


>Bye,
>Hannes
>
> arch/x86/include/asm/hash.h | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
>diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
>index a881d78..a25c45a 100644
>--- a/arch/x86/include/asm/hash.h
>+++ b/arch/x86/include/asm/hash.h
>@@ -23,11 +23,15 @@ 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));
>+	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
>+			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
>+			 : "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11",
>+			   "cc", "memory");
> #else
>-			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
>+	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
>+			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
>+			 : "edx", "ecx", "cc", "memory");
> #endif
> 	return hash;
> }
>@@ -36,11 +40,15 @@ 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));
>+	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
>+			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
>+			 : "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11",
>+			   "cc", "memory");
> #else
>-			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
>+	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
>+			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
>+			 : "edx", "ecx", "cc", "memory");
> #endif
> 	return hash;
> }
>-- 
>1.9.3

---
	-Jay Vosburgh, jay.vosburgh@canonical.com