Re: [patch 3/4] dm-writecache

[Christoph Hellwig <hch@infradead.org>]

On Sat, May 19, 2018 at 07:25:06AM +0200, Mikulas Patocka wrote:
> The dm-writecache target.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

You'll need to actually describe your new code in the changelog.

> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6/drivers/md/dm-writecache.c	2018-05-17 02:46:44.000000000 +0200
> @@ -0,0 +1,2414 @@
> +/*
> + * Copyright (C) 2018 Red Hat. All rights reserved.
> + *
> + * This file is released under the GPL.
> + */

New code needs a SPDX header.

> +/*
> + * On X86, non-temporal stores are more efficient than cache flushing.
> + * On ARM64, cache flushing is more efficient.
> + */
> +#if defined(CONFIG_X86_64)
> +#define EAGER_DATA_FLUSH
> +#define NT_STORE(dest, src)				\
> +do {							\
> +	typeof(src) val = (src);			\
> +	memcpy_flushcache(&(dest), &val, sizeof(src));	\
> +} while (0)
> +#else
> +#define NT_STORE(dest, src)	WRITE_ONCE(dest, src)
> +#endif

No per-arch hacks in the driver please, this needs a proper Kconfig
symbol provided from the architectures.

> +struct wc_entry {
> +	struct rb_node rb_node;
> +	struct list_head lru;
> +	unsigned short wc_list_contiguous;
> +	bool write_in_progress
> +#if BITS_PER_LONG == 64
> +		:1
> +#endif
> +	;
> +	unsigned long index
> +#if BITS_PER_LONG == 64
> +		:47
> +#endif

Hacks like this shouldn't normally exist, but if you absolutely
need them you need to explain why in a comment.

Haven't had time to do a full review due to my backlog, I'll try to
find some time later today or tomorrow.