linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dm: writecache: add DAX dependency
@ 2018-05-28 15:38 Arnd Bergmann
  2018-05-28 18:18 ` Dan Williams
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Arnd Bergmann @ 2018-05-28 15:38 UTC (permalink / raw)
  To: Mikulas Patocka, Shaohua Li, Alasdair Kergon, Mike Snitzer, dm-devel
  Cc: Matthew Wilcox, Ross Zwisler, linux-fsdevel, Arnd Bergmann,
	Dan Williams, Heinz Mauelshagen, linux-raid, linux-kernel

The new dm-writecache driver inconditionally uses the dax
subsystem, leading to link errors in some configurations:

drivers/md/dm-writecache.o: In function `writecache_ctr':
dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'

It seems wrong to require DAX in order to build the writecache
driver, but that at least avoids randconfig build errors.

Fixes: bb15b431d650 ("dm: add writecache target")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/md/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 852c7ebe2902..f8ecf2da1edf 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -338,6 +338,7 @@ config DM_CACHE_SMQ
 config DM_WRITECACHE
 	tristate "Writecache target"
 	depends on BLK_DEV_DM
+	depends on DAX
 	---help---
 	   The writecache target caches writes on persistent memory or SSD.
 	   It is intended for databases or other programs that need extremely
-- 
2.9.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dm: writecache: add DAX dependency
  2018-05-28 15:38 [PATCH] dm: writecache: add DAX dependency Arnd Bergmann
@ 2018-05-28 18:18 ` Dan Williams
  2018-05-29 13:06   ` Mike Snitzer
  2018-05-29 15:17 ` [PATCH] " Ross Zwisler
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Dan Williams @ 2018-05-28 18:18 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mikulas Patocka, Shaohua Li, Alasdair Kergon, Mike Snitzer,
	device-mapper development, Matthew Wilcox, Ross Zwisler,
	linux-fsdevel, Heinz Mauelshagen, linux-raid,
	Linux Kernel Mailing List

On Mon, May 28, 2018 at 8:38 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> The new dm-writecache driver inconditionally uses the dax
> subsystem, leading to link errors in some configurations:
>
> drivers/md/dm-writecache.o: In function `writecache_ctr':
> dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
> dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
> dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'
>
> It seems wrong to require DAX in order to build the writecache
> driver, but that at least avoids randconfig build errors.
>
> Fixes: bb15b431d650 ("dm: add writecache target")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/md/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 852c7ebe2902..f8ecf2da1edf 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -338,6 +338,7 @@ config DM_CACHE_SMQ
>  config DM_WRITECACHE
>         tristate "Writecache target"
>         depends on BLK_DEV_DM
> +       depends on DAX

This should probably be depends on DAX && DAX_DRIVER as we at least
need pmem or dcssblk enabled to provide a dax capable block device for
DM to claim.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: dm: writecache: add DAX dependency
  2018-05-28 18:18 ` Dan Williams
@ 2018-05-29 13:06   ` Mike Snitzer
  0 siblings, 0 replies; 11+ messages in thread
From: Mike Snitzer @ 2018-05-29 13:06 UTC (permalink / raw)
  To: Dan Williams
  Cc: Arnd Bergmann, Mikulas Patocka, Shaohua Li, Alasdair Kergon,
	device-mapper development, Matthew Wilcox, Ross Zwisler,
	linux-fsdevel, Heinz Mauelshagen, linux-raid,
	Linux Kernel Mailing List

On Mon, May 28 2018 at  2:18pm -0400,
Dan Williams <dan.j.williams@intel.com> wrote:

> On Mon, May 28, 2018 at 8:38 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > The new dm-writecache driver inconditionally uses the dax
> > subsystem, leading to link errors in some configurations:
> >
> > drivers/md/dm-writecache.o: In function `writecache_ctr':
> > dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
> > dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
> > dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'
> >
> > It seems wrong to require DAX in order to build the writecache
> > driver, but that at least avoids randconfig build errors.
> >
> > Fixes: bb15b431d650 ("dm: add writecache target")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/md/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> > index 852c7ebe2902..f8ecf2da1edf 100644
> > --- a/drivers/md/Kconfig
> > +++ b/drivers/md/Kconfig
> > @@ -338,6 +338,7 @@ config DM_CACHE_SMQ
> >  config DM_WRITECACHE
> >         tristate "Writecache target"
> >         depends on BLK_DEV_DM
> > +       depends on DAX
> 
> This should probably be depends on DAX && DAX_DRIVER as we at least
> need pmem or dcssblk enabled to provide a dax capable block device for
> DM to claim.

But dm-writecache is meant to be used for normal SSD even if PMEM isn't
available in the kernel.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dm: writecache: add DAX dependency
  2018-05-28 15:38 [PATCH] dm: writecache: add DAX dependency Arnd Bergmann
  2018-05-28 18:18 ` Dan Williams
@ 2018-05-29 15:17 ` Ross Zwisler
  2018-05-29 17:52 ` [PATCH] dm-writecache: fix compilation issue with !DAX Ross Zwisler
  2018-05-30 12:21 ` [PATCH] dm: writecache: add DAX dependency Mikulas Patocka
  3 siblings, 0 replies; 11+ messages in thread
From: Ross Zwisler @ 2018-05-29 15:17 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mikulas Patocka, Shaohua Li, Alasdair Kergon, Mike Snitzer,
	dm-devel, Matthew Wilcox, Ross Zwisler, linux-fsdevel,
	Dan Williams, Heinz Mauelshagen, linux-raid, linux-kernel

On Mon, May 28, 2018 at 05:38:10PM +0200, Arnd Bergmann wrote:
> The new dm-writecache driver inconditionally uses the dax
			       unconditionally

> subsystem, leading to link errors in some configurations:
> 
> drivers/md/dm-writecache.o: In function `writecache_ctr':
> dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
> dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
> dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'
> 
> It seems wrong to require DAX in order to build the writecache
> driver, but that at least avoids randconfig build errors.
> 
> Fixes: bb15b431d650 ("dm: add writecache target")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/md/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 852c7ebe2902..f8ecf2da1edf 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -338,6 +338,7 @@ config DM_CACHE_SMQ
>  config DM_WRITECACHE
>  	tristate "Writecache target"
>  	depends on BLK_DEV_DM
> +	depends on DAX
>  	---help---
>  	   The writecache target caches writes on persistent memory or SSD.
>  	   It is intended for databases or other programs that need extremely

I think the right way to handle this is to instead wrap all the DAX code in
dm-writecache in "#if IS_ENABLED(CONFIG_DAX_DRIVER)" blocks and provide stubs
for the non-DAX case.  This prevents you from having to pull in all the
generic DAX code unnecessarily.

In looking at the file I think this is just the persistent_memory_claim()
function.

I'll send out a patch once I've tested a bit.

- Ross

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] dm-writecache: fix compilation issue with !DAX
  2018-05-28 15:38 [PATCH] dm: writecache: add DAX dependency Arnd Bergmann
  2018-05-28 18:18 ` Dan Williams
  2018-05-29 15:17 ` [PATCH] " Ross Zwisler
@ 2018-05-29 17:52 ` Ross Zwisler
  2018-05-29 18:08   ` Mike Snitzer
  2018-05-30 12:22   ` [PATCH] " Mikulas Patocka
  2018-05-30 12:21 ` [PATCH] dm: writecache: add DAX dependency Mikulas Patocka
  3 siblings, 2 replies; 11+ messages in thread
From: Ross Zwisler @ 2018-05-29 17:52 UTC (permalink / raw)
  To: Arnd Bergmann, Mikulas Patocka, Shaohua Li, Alasdair Kergon,
	Mike Snitzer, dm-devel
  Cc: Ross Zwisler, Matthew Wilcox, linux-fsdevel, Dan Williams,
	Heinz Mauelshagen, linux-raid, linux-kernel

As reported by Arnd (https://lkml.org/lkml/2018/5/28/1697), dm-writecache
will fail with link errors in configs where DAX isn't present:

drivers/md/dm-writecache.o: In function `writecache_ctr':
dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'

Fix this by following the lead of the other DM modules and wrapping calls
to the generic DAX code in #if IS_ENABLED(CONFIG_DAX_DRIVER) blocks.

We also expand the failure case for the 'p' (persistent memory) flag so
that fails on both architectures that don't support persistent memory and
on kernels that don't have DAX support configured.  This prevents us from
ever hitting the BUG() in the persistent_memory_claim() stub.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reported-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/md/dm-writecache.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
index 92af65fdf4af..1c2b53ae1a96 100644
--- a/drivers/md/dm-writecache.c
+++ b/drivers/md/dm-writecache.c
@@ -253,6 +253,7 @@ static void wc_unlock(struct dm_writecache *wc)
 	mutex_unlock(&wc->lock);
 }
 
+#if IS_ENABLED(CONFIG_DAX_DRIVER)
 static int persistent_memory_claim(struct dm_writecache *wc)
 {
 	int r;
@@ -337,6 +338,12 @@ static int persistent_memory_claim(struct dm_writecache *wc)
 err1:
 	return r;
 }
+#else
+static int persistent_memory_claim(struct dm_writecache *wc)
+{
+	BUG();
+}
+#endif
 
 static void persistent_memory_release(struct dm_writecache *wc)
 {
@@ -1901,16 +1908,17 @@ static int writecache_ctr(struct dm_target *ti, unsigned argc, char **argv)
 	if (!strcasecmp(string, "s")) {
 		wc->pmem_mode = false;
 	} else if (!strcasecmp(string, "p")) {
-#ifdef CONFIG_ARCH_HAS_PMEM_API
+#if defined(CONFIG_ARCH_HAS_PMEM_API) && IS_ENABLED(CONFIG_DAX_DRIVER)
 		wc->pmem_mode = true;
 		wc->writeback_fua = true;
 #else
 		/*
-		 * If the architecture doesn't support persistent memory, this
-		 * driver can only be used in SSD-only mode.
+		 * If the architecture doesn't support persistent memory or
+		 * the kernel doesn't support any DAX drivers, this driver can
+		 * only be used in SSD-only mode.
 		 */
 		r = -EOPNOTSUPP;
-		ti->error = "Persistent memory not supported on this architecture";
+		ti->error = "Persistent memory or DAX not supported on this system";
 		goto bad;
 #endif
 	} else {
-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: dm-writecache: fix compilation issue with !DAX
  2018-05-29 17:52 ` [PATCH] dm-writecache: fix compilation issue with !DAX Ross Zwisler
@ 2018-05-29 18:08   ` Mike Snitzer
  2018-05-29 18:40     ` Dan Williams
  2018-05-30 12:22   ` [PATCH] " Mikulas Patocka
  1 sibling, 1 reply; 11+ messages in thread
From: Mike Snitzer @ 2018-05-29 18:08 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Arnd Bergmann, Mikulas Patocka, Shaohua Li, Alasdair Kergon,
	dm-devel, Matthew Wilcox, linux-fsdevel, Dan Williams,
	Heinz Mauelshagen, linux-raid, linux-kernel

On Tue, May 29 2018 at  1:52pm -0400,
Ross Zwisler <ross.zwisler@linux.intel.com> wrote:

> As reported by Arnd (https://lkml.org/lkml/2018/5/28/1697), dm-writecache
> will fail with link errors in configs where DAX isn't present:
> 
> drivers/md/dm-writecache.o: In function `writecache_ctr':
> dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
> dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
> dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'
> 
> Fix this by following the lead of the other DM modules and wrapping calls
> to the generic DAX code in #if IS_ENABLED(CONFIG_DAX_DRIVER) blocks.
> 
> We also expand the failure case for the 'p' (persistent memory) flag so
> that fails on both architectures that don't support persistent memory and
> on kernels that don't have DAX support configured.  This prevents us from
> ever hitting the BUG() in the persistent_memory_claim() stub.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: Arnd Bergmann <arnd@arndb.de>

Thanks, I've picked this up.

Mike

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: dm-writecache: fix compilation issue with !DAX
  2018-05-29 18:08   ` Mike Snitzer
@ 2018-05-29 18:40     ` Dan Williams
  2018-05-29 19:57       ` Mike Snitzer
  0 siblings, 1 reply; 11+ messages in thread
From: Dan Williams @ 2018-05-29 18:40 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Ross Zwisler, Arnd Bergmann, Mikulas Patocka, Shaohua Li,
	Alasdair Kergon, device-mapper development, Matthew Wilcox,
	linux-fsdevel, Heinz Mauelshagen, linux-raid,
	Linux Kernel Mailing List

On Tue, May 29, 2018 at 11:08 AM, Mike Snitzer <snitzer@redhat.com> wrote:
> On Tue, May 29 2018 at  1:52pm -0400,
> Ross Zwisler <ross.zwisler@linux.intel.com> wrote:
>
>> As reported by Arnd (https://lkml.org/lkml/2018/5/28/1697), dm-writecache
>> will fail with link errors in configs where DAX isn't present:
>>
>> drivers/md/dm-writecache.o: In function `writecache_ctr':
>> dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
>> dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
>> dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'
>>
>> Fix this by following the lead of the other DM modules and wrapping calls
>> to the generic DAX code in #if IS_ENABLED(CONFIG_DAX_DRIVER) blocks.
>>
>> We also expand the failure case for the 'p' (persistent memory) flag so
>> that fails on both architectures that don't support persistent memory and
>> on kernels that don't have DAX support configured.  This prevents us from
>> ever hitting the BUG() in the persistent_memory_claim() stub.
>>
>> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>
> Thanks, I've picked this up.

...I assume you're also going to let the 'pmem api' discussion resolve
before this all goes upstream?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: dm-writecache: fix compilation issue with !DAX
  2018-05-29 18:40     ` Dan Williams
@ 2018-05-29 19:57       ` Mike Snitzer
  0 siblings, 0 replies; 11+ messages in thread
From: Mike Snitzer @ 2018-05-29 19:57 UTC (permalink / raw)
  To: Dan Williams
  Cc: Ross Zwisler, Arnd Bergmann, Mikulas Patocka, Shaohua Li,
	Alasdair Kergon, device-mapper development, Matthew Wilcox,
	linux-fsdevel, Heinz Mauelshagen, linux-raid,
	Linux Kernel Mailing List

On Tue, May 29 2018 at  2:40pm -0400,
Dan Williams <dan.j.williams@intel.com> wrote:

> On Tue, May 29, 2018 at 11:08 AM, Mike Snitzer <snitzer@redhat.com> wrote:
> > On Tue, May 29 2018 at  1:52pm -0400,
> > Ross Zwisler <ross.zwisler@linux.intel.com> wrote:
> >
> >> As reported by Arnd (https://lkml.org/lkml/2018/5/28/1697), dm-writecache
> >> will fail with link errors in configs where DAX isn't present:
> >>
> >> drivers/md/dm-writecache.o: In function `writecache_ctr':
> >> dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
> >> dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
> >> dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'
> >>
> >> Fix this by following the lead of the other DM modules and wrapping calls
> >> to the generic DAX code in #if IS_ENABLED(CONFIG_DAX_DRIVER) blocks.
> >>
> >> We also expand the failure case for the 'p' (persistent memory) flag so
> >> that fails on both architectures that don't support persistent memory and
> >> on kernels that don't have DAX support configured.  This prevents us from
> >> ever hitting the BUG() in the persistent_memory_claim() stub.
> >>
> >> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> >> Reported-by: Arnd Bergmann <arnd@arndb.de>
> >
> > Thanks, I've picked this up.
> 
> ...I assume you're also going to let the 'pmem api' discussion resolve
> before this all goes upstream?

Yeah, I'm going to pivot back to that and put time to it shortly.  If
dm-writecache has to wait another cycle (so 4.19 inclusion), while
unfortunate, it wouldn't be the end of the world.

I look forward to your continued help, thanks.
Mike

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dm: writecache: add DAX dependency
  2018-05-28 15:38 [PATCH] dm: writecache: add DAX dependency Arnd Bergmann
                   ` (2 preceding siblings ...)
  2018-05-29 17:52 ` [PATCH] dm-writecache: fix compilation issue with !DAX Ross Zwisler
@ 2018-05-30 12:21 ` Mikulas Patocka
  3 siblings, 0 replies; 11+ messages in thread
From: Mikulas Patocka @ 2018-05-30 12:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Shaohua Li, Alasdair Kergon, Mike Snitzer, dm-devel,
	Matthew Wilcox, Ross Zwisler, linux-fsdevel, Dan Williams,
	Heinz Mauelshagen, linux-raid, linux-kernel



On Mon, 28 May 2018, Arnd Bergmann wrote:

> The new dm-writecache driver inconditionally uses the dax
> subsystem, leading to link errors in some configurations:
> 
> drivers/md/dm-writecache.o: In function `writecache_ctr':
> dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
> dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
> dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'
> 
> It seems wrong to require DAX in order to build the writecache
> driver, but that at least avoids randconfig build errors.
> 
> Fixes: bb15b431d650 ("dm: add writecache target")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/md/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 852c7ebe2902..f8ecf2da1edf 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -338,6 +338,7 @@ config DM_CACHE_SMQ
>  config DM_WRITECACHE
>  	tristate "Writecache target"
>  	depends on BLK_DEV_DM
> +	depends on DAX
>  	---help---
>  	   The writecache target caches writes on persistent memory or SSD.
>  	   It is intended for databases or other programs that need extremely
> -- 
> 2.9.0

dm-writecache may be used without DAX in SSD-only mode.

So, I'd fix this by changing the code in dm-writecache.c to

#if !defined(CONFIG_ARCH_HAS_PMEM_API) || !defined(CONFIG_DAX)
#define DM_WRITECACHE_ONLY_SSD
#endif

Mikulas

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dm-writecache: fix compilation issue with !DAX
  2018-05-29 17:52 ` [PATCH] dm-writecache: fix compilation issue with !DAX Ross Zwisler
  2018-05-29 18:08   ` Mike Snitzer
@ 2018-05-30 12:22   ` Mikulas Patocka
  2018-05-30 13:13     ` Mike Snitzer
  1 sibling, 1 reply; 11+ messages in thread
From: Mikulas Patocka @ 2018-05-30 12:22 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Arnd Bergmann, Shaohua Li, Alasdair Kergon, Mike Snitzer,
	dm-devel, Matthew Wilcox, linux-fsdevel, Dan Williams,
	Heinz Mauelshagen, linux-raid, linux-kernel



On Tue, 29 May 2018, Ross Zwisler wrote:

> As reported by Arnd (https://lkml.org/lkml/2018/5/28/1697), dm-writecache
> will fail with link errors in configs where DAX isn't present:
> 
> drivers/md/dm-writecache.o: In function `writecache_ctr':
> dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
> dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
> dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'
> 
> Fix this by following the lead of the other DM modules and wrapping calls
> to the generic DAX code in #if IS_ENABLED(CONFIG_DAX_DRIVER) blocks.
> 
> We also expand the failure case for the 'p' (persistent memory) flag so
> that fails on both architectures that don't support persistent memory and
> on kernels that don't have DAX support configured.  This prevents us from
> ever hitting the BUG() in the persistent_memory_claim() stub.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/md/dm-writecache.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
> index 92af65fdf4af..1c2b53ae1a96 100644
> --- a/drivers/md/dm-writecache.c
> +++ b/drivers/md/dm-writecache.c
> @@ -253,6 +253,7 @@ static void wc_unlock(struct dm_writecache *wc)
>  	mutex_unlock(&wc->lock);
>  }
>  
> +#if IS_ENABLED(CONFIG_DAX_DRIVER)

We already have #if(n)def DM_WRITECACHE_ONLY_SSD

There is no need to use another macro for the same purpose.

Mikulas

>  static int persistent_memory_claim(struct dm_writecache *wc)
>  {
>  	int r;
> @@ -337,6 +338,12 @@ static int persistent_memory_claim(struct dm_writecache *wc)
>  err1:
>  	return r;
>  }
> +#else
> +static int persistent_memory_claim(struct dm_writecache *wc)
> +{
> +	BUG();
> +}
> +#endif
>  
>  static void persistent_memory_release(struct dm_writecache *wc)
>  {
> @@ -1901,16 +1908,17 @@ static int writecache_ctr(struct dm_target *ti, unsigned argc, char **argv)
>  	if (!strcasecmp(string, "s")) {
>  		wc->pmem_mode = false;
>  	} else if (!strcasecmp(string, "p")) {
> -#ifdef CONFIG_ARCH_HAS_PMEM_API
> +#if defined(CONFIG_ARCH_HAS_PMEM_API) && IS_ENABLED(CONFIG_DAX_DRIVER)
>  		wc->pmem_mode = true;
>  		wc->writeback_fua = true;
>  #else
>  		/*
> -		 * If the architecture doesn't support persistent memory, this
> -		 * driver can only be used in SSD-only mode.
> +		 * If the architecture doesn't support persistent memory or
> +		 * the kernel doesn't support any DAX drivers, this driver can
> +		 * only be used in SSD-only mode.
>  		 */
>  		r = -EOPNOTSUPP;
> -		ti->error = "Persistent memory not supported on this architecture";
> +		ti->error = "Persistent memory or DAX not supported on this system";
>  		goto bad;
>  #endif
>  	} else {
> -- 
> 2.14.3
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: dm-writecache: fix compilation issue with !DAX
  2018-05-30 12:22   ` [PATCH] " Mikulas Patocka
@ 2018-05-30 13:13     ` Mike Snitzer
  0 siblings, 0 replies; 11+ messages in thread
From: Mike Snitzer @ 2018-05-30 13:13 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: Ross Zwisler, Arnd Bergmann, Shaohua Li, Alasdair Kergon,
	dm-devel, Matthew Wilcox, linux-fsdevel, Dan Williams,
	Heinz Mauelshagen, linux-raid, linux-kernel

On Wed, May 30 2018 at  8:22am -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> 
> 
> On Tue, 29 May 2018, Ross Zwisler wrote:
> 
> > As reported by Arnd (https://lkml.org/lkml/2018/5/28/1697), dm-writecache
> > will fail with link errors in configs where DAX isn't present:
> > 
> > drivers/md/dm-writecache.o: In function `writecache_ctr':
> > dm-writecache.c:(.text+0x1fdc): undefined reference to `dax_read_lock'
> > dm-writecache.c:(.text+0x2004): undefined reference to `dax_direct_access'
> > dm-writecache.c:(.text+0x21cc): undefined reference to `dax_read_unlock'
> > 
> > Fix this by following the lead of the other DM modules and wrapping calls
> > to the generic DAX code in #if IS_ENABLED(CONFIG_DAX_DRIVER) blocks.
> > 
> > We also expand the failure case for the 'p' (persistent memory) flag so
> > that fails on both architectures that don't support persistent memory and
> > on kernels that don't have DAX support configured.  This prevents us from
> > ever hitting the BUG() in the persistent_memory_claim() stub.
> > 
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Reported-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/md/dm-writecache.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
> > index 92af65fdf4af..1c2b53ae1a96 100644
> > --- a/drivers/md/dm-writecache.c
> > +++ b/drivers/md/dm-writecache.c
> > @@ -253,6 +253,7 @@ static void wc_unlock(struct dm_writecache *wc)
> >  	mutex_unlock(&wc->lock);
> >  }
> >  
> > +#if IS_ENABLED(CONFIG_DAX_DRIVER)
> 
> We already have #if(n)def DM_WRITECACHE_ONLY_SSD
> 
> There is no need to use another macro for the same purpose.

I removed DM_WRITECACHE_ONLY_SSD because it wasn't needed, this is the
split out commit that I have since folded in:
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/commit/?h=dm-4.18-writecache-wip&id=e7fd3d1c05659f7e1295178290fbdaebf36becdc

Ross's patch effectively builds ontop of that.

Mike

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-05-30 13:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-28 15:38 [PATCH] dm: writecache: add DAX dependency Arnd Bergmann
2018-05-28 18:18 ` Dan Williams
2018-05-29 13:06   ` Mike Snitzer
2018-05-29 15:17 ` [PATCH] " Ross Zwisler
2018-05-29 17:52 ` [PATCH] dm-writecache: fix compilation issue with !DAX Ross Zwisler
2018-05-29 18:08   ` Mike Snitzer
2018-05-29 18:40     ` Dan Williams
2018-05-29 19:57       ` Mike Snitzer
2018-05-30 12:22   ` [PATCH] " Mikulas Patocka
2018-05-30 13:13     ` Mike Snitzer
2018-05-30 12:21 ` [PATCH] dm: writecache: add DAX dependency Mikulas Patocka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).