Re: [PATCH v2 1/2] xxHash: create arch dependent 32/64-bit xxhash()

[Matthew Wilcox]

On Fri, Sep 22, 2017 at 02:18:17AM +0300, Timofey Titovets wrote:
> diff --git a/include/linux/xxhash.h b/include/linux/xxhash.h
> index 9e1f42cb57e9..195a0ae10e9b 100644
> --- a/include/linux/xxhash.h
> +++ b/include/linux/xxhash.h
> @@ -76,6 +76,7 @@
>  #define XXHASH_H
> 
>  #include <linux/types.h>
> +#include <linux/bitops.h> /* BITS_PER_LONG */
> 
>  /*-****************************
>   * Simple Hash Functions

Huh?  linux/types.h already brings in BITS_PER_LONG.  Look:

linux/types.h
  uapi/linux/types.h
  uapi/asm/types.h
  uapi/asm-generic/types.h
  uapi/asm-generic/int-ll64.h
  asm/bitsperlong.h

> @@ -107,6 +108,29 @@ uint32_t xxh32(const void *input, size_t length, uint32_t seed);
>   */
>  uint64_t xxh64(const void *input, size_t length, uint64_t seed);
> 
> +#if BITS_PER_LONG == 64
> +typedef	u64	xxhash_t;
> +#else
> +typedef	u32	xxhash_t;
> +#endif

This is a funny way to spell 'unsigned long' ...

> +/**
> + * xxhash() - calculate 32/64-bit hash based on cpu word size
> + *
> + * @input:  The data to hash.
> + * @length: The length of the data to hash.
> + * @seed:   The seed can be used to alter the result predictably.
> + *
> + * This function always work as xxh32() for 32-bit systems
> + * and as xxh64() for 64-bit systems.
> + * Because result depends on cpu work size,
> + * the main proporse of that function is for  in memory hashing.
> + *
> + * Return:  32/64-bit hash of the data.
> + */
> +

> +xxhash_t xxhash(const void *input, size_t length, uint64_t seed)
> +{
> +#if BITS_PER_LONG == 64
> +	return xxh64(input, length, seed);
> +#else
> +	return xxh32(input, length, seed);
> +#endif
> +}

Let's move that to the header file and make it a static inline.  That way
it doesn't need to be an EXPORT_SYMBOL.

Also, I think the kerneldoc could do with a bit of work.  Try this:

/**
 * xxhash() - calculate wordsize hash of the input with a given seed
 * @input:  The data to hash.
 * @length: The length of the data to hash.
 * @seed:   The seed can be used to alter the result predictably.
 *
 * If the hash does not need to be comparable between machines with
 * different word sizes, this function will call whichever of xxh32()
 * or xxh64() is faster.
 *
 * Return:  wordsize hash of the data.
 */
static inline
unsigned long xxhash(const void *input, size_t length, unsigned long seed)
{
#if BITS_PER_LONG == 64
	return xxh64(input, length, seed);
#else
	return xxh32(input, length, seed);
#endif
}