| From foo@baz Sun Jun 17 12:07:34 CEST 2018 |
| From: Amir Goldstein <amir73il@gmail.com> |
| Date: Mon, 5 Feb 2018 19:32:18 +0200 |
| Subject: <linux/stringhash.h>: fix end_name_hash() for 64bit long |
| |
| From: Amir Goldstein <amir73il@gmail.com> |
| |
| [ Upstream commit 19b9ad67310ed2f685062a00aec602bec33835f0 ] |
| |
| The comment claims that this helper will try not to loose bits, but for |
| 64bit long it looses the high bits before hashing 64bit long into 32bit |
| int. Use the helper hash_long() to do the right thing for 64bit long. |
| For 32bit long, there is no change. |
| |
| All the callers of end_name_hash() either assign the result to |
| qstr->hash, which is u32 or return the result as an int value (e.g. |
| full_name_hash()). Change the helper return type to int to conform to |
| its users. |
| |
| [ It took me a while to apply this, because my initial reaction to it |
| was - incorrectly - that it could make for slower code. |
| |
| After having looked more at it, I take back all my complaints about |
| the patch, Amir was right and I was mis-reading things or just being |
| stupid. |
| |
| I also don't worry too much about the possible performance impact of |
| this on 64-bit, since most architectures that actually care about |
| performance end up not using this very much (the dcache code is the |
| most performance-critical, but the word-at-a-time case uses its own |
| hashing anyway). |
| |
| So this ends up being mostly used for filesystems that do their own |
| degraded hashing (usually because they want a case-insensitive |
| comparison function). |
| |
| A _tiny_ worry remains, in that not everybody uses DCACHE_WORD_ACCESS, |
| and then this potentially makes things more expensive on 64-bit |
| architectures with slow or lacking multipliers even for the normal |
| case. |
| |
| That said, realistically the only such architecture I can think of is |
| PA-RISC. Nobody really cares about performance on that, it's more of a |
| "look ma, I've got warts^W an odd machine" platform. |
| |
| So the patch is fine, and all my initial worries were just misplaced |
| from not looking at this properly. - Linus ] |
| |
| Signed-off-by: Amir Goldstein <amir73il@gmail.com> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| include/linux/stringhash.h | 4 ++-- |
| 1 file changed, 2 insertions(+), 2 deletions(-) |
| |
| --- a/include/linux/stringhash.h |
| +++ b/include/linux/stringhash.h |
| @@ -50,9 +50,9 @@ partial_name_hash(unsigned long c, unsig |
| * losing bits). This also has the property (wanted by the dcache) |
| * that the msbits make a good hash table index. |
| */ |
| -static inline unsigned long end_name_hash(unsigned long hash) |
| +static inline unsigned int end_name_hash(unsigned long hash) |
| { |
| - return __hash_32((unsigned int)hash); |
| + return hash_long(hash, 32); |
| } |
| |
| /* |