| Message ID | 20180528153834.2268557-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | dm: writecache: add DAX dependency | expand |
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.
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.
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
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
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?
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
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
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
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(+) -- 2.9.0