| Message ID | 20180529175216.24937-1-ross.zwisler@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 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 >
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
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 {
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(-)