Re: [dm-devel] [PATCH] dm-writecache

[Mikulas Patocka <mpatocka@redhat.com>]

On Thu, 8 Mar 2018, Christoph Hellwig wrote:

> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6/drivers/md/dm-writecache.c	2018-03-08 14:23:31.059999000 +0100
> > @@ -0,0 +1,2417 @@
> > +#include <linux/device-mapper.h>
> 
> missing copyright statement, or for those new-fashioned SPDX statement.
> 
> > +#define WRITEBACK_FUA			true
> 
> no business having this around.

It's the default setting of the flag wc->writeback_fua (it can be changed 
with target parameters). The flag selects whether the target uses FUA 
requests when doing writeback or whether it uses non-FUA requests and 
FLUSH afterwards. For some block devices, FUA is faster, for some 
nonFUA+FLUSH is faster.

What's wrong with this? Why can't default settings be #defined at the 
beginning of a file?

> > +#ifndef bio_set_dev
> > +#define	bio_set_dev(bio, dev)	((bio)->bi_bdev = (dev))
> > +#endif
> > +#ifndef timer_setup
> > +#define timer_setup(t, c, f)	setup_timer(t, c, (unsigned long)(t))
> > +#endif
> 
> no business in mainline.

People removed dax support for ramdisk in 4.15.

If I need to test it on non-x86 architecture, I need ramdisk as a fake dax 
device - and that only works up to 4.14. These defines are for 4.14 
compatibility.

> > +/*
> > + * On X86, non-temporal stores are more efficient than cache flushing.
> > + * On ARM64, cache flushing is more efficient.
> > + */
> > +#if defined(CONFIG_X86_64)
> > +#define NT_STORE(dest, src)				\
> > +do {							\
> > +	typeof(src) val = (src);			\
> > +	memcpy_flushcache(&(dest), &val, sizeof(src));	\
> > +} while (0)
> > +#define COMMIT_FLUSHED()	wmb()
> > +#else
> > +#define NT_STORE(dest, src)	WRITE_ONCE(dest, src)
> > +#define FLUSH_RANGE		dax_flush
> > +#define COMMIT_FLUSHED()	do { } while (0)
> > +#endif
> 
> Please use proper APIs for this, this has no business in a driver.
> 
> And that's it for now.  This is clearly not submission ready, and I
> should got back to my backlog of other things.

Why is memcpy_flushcache and dax_flush "improper"? What should I use 
instead of them?

Mikulas
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm