| Message ID | 9e4e6445-9d57-3b7f-6a2a-ddad750ef11c@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RESEND,v2] dax: print error message by pr_info() in __generic_fsdax_supported() | expand |
On Fri, Jul 17, 2020 at 10:52:26AM +0800, Coly Li wrote: > In struct dax_operations, the callback routine dax_supported() returns > a bool type result. For false return value, the caller has no idea > whether the device does not support dax at all, or it is just some mis- > configuration issue. > > An example is formatting an Ext4 file system on pmem device on top of > a NVDIMM namespace by, > # mkfs.ext4 /dev/pmem0 > If the fs block size does not match kernel space memory page size (which > is possible on non-x86 platform), mount this Ext4 file system will fail, > # mount -o dax /dev/pmem0 /mnt > mount: /mnt: wrong fs type, bad option, bad superblock on /dev/pmem0, > missing codepage or helper program, or other error. > And from the dmesg output there is only the following information, > [ 307.853148] EXT4-fs (pmem0): DAX unsupported by block device. > > The above information is quite confusing. Because definiately the pmem0 > device supports dax operation, and the super block is consistent as how > it was created by mkfs.ext4. > > Indeed the failure is from __generic_fsdax_supported() by the following > code piece, > if (blocksize != PAGE_SIZE) { > pr_debug("%s: error: unsupported blocksize for dax\n", > bdevname(bdev, buf)); > return false; > } > It is because the Ext4 block size is 4KB and kernel page size is 8KB or > 16KB. > > It is not simple to make dax_supported() from struct dax_operations > or __generic_fsdax_supported() to return exact failure type right now. > So the simplest fix is to use pr_info() to print all the error messages > inside __generic_fsdax_supported(). Then users may find informative clue > from the kernel message at least. > > Message printed by pr_debug() is very easy to be ignored by users. This > patch prints error message by pr_info() in __generic_fsdax_supported(), > when then mount fails, following lines can be found from dmesg output, > [ 2705.500885] pmem0: error: unsupported blocksize for dax > [ 2705.500888] EXT4-fs (pmem0): DAX unsupported by block device. > Now the users may have idea the mount failure is from pmem driver for > unsupported block size. > > Reported-by: Michal Suchanek <msuchanek@suse.com> > Suggested-by: Jan Kara <jack@suse.com> > Signed-off-by: Coly Li <colyli@suse.de> > Reviewed-by: Jan Kara <jack@suse.com> Yes this looks good to me as well. Reviewed-by: Ira Weiny <ira.weiny@intel.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Anthony Iliopoulos <ailiopoulos@suse.com> > --- > Changelog: > v2: Add reviewed-by from Jan Kara > v1: initial version. > > drivers/dax/super.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/dax/super.c b/drivers/dax/super.c > index 8e32345be0f7..de0d02ec0347 100644 > --- a/drivers/dax/super.c > +++ b/drivers/dax/super.c > @@ -80,14 +80,14 @@ bool __generic_fsdax_supported(struct dax_device > *dax_dev, > int err, id; > if (blocksize != PAGE_SIZE) { > - pr_debug("%s: error: unsupported blocksize for dax\n", > + pr_info("%s: error: unsupported blocksize for dax\n", > bdevname(bdev, buf)); > return false; > } > err = bdev_dax_pgoff(bdev, start, PAGE_SIZE, &pgoff); > if (err) { > - pr_debug("%s: error: unaligned partition for dax\n", > + pr_info("%s: error: unaligned partition for dax\n", > bdevname(bdev, buf)); > return false; > } > @@ -95,7 +95,7 @@ bool __generic_fsdax_supported(struct dax_device *dax_dev, > last_page = PFN_DOWN((start + sectors - 1) * 512) * PAGE_SIZE / 512; > err = bdev_dax_pgoff(bdev, last_page, PAGE_SIZE, &pgoff_end); > if (err) { > - pr_debug("%s: error: unaligned partition for dax\n", > + pr_info("%s: error: unaligned partition for dax\n", > bdevname(bdev, buf)); > return false; > } > @@ -106,7 +106,7 @@ bool __generic_fsdax_supported(struct dax_device > *dax_dev, > dax_read_unlock(id); > if (len < 1 || len2 < 1) { > - pr_debug("%s: error: dax access failed (%ld)\n", > + pr_info("%s: error: dax access failed (%ld)\n", > bdevname(bdev, buf), len < 1 ? len : len2); > return false; > } > @@ -139,7 +139,7 @@ bool __generic_fsdax_supported(struct dax_device > *dax_dev, > } > if (!dax_enabled) { > - pr_debug("%s: error: dax support not enabled\n", > + pr_info("%s: error: dax support not enabled\n", > bdevname(bdev, buf)); > return false; > } > -- > 2.26.2 > _______________________________________________ > Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org > To unsubscribe send an email to linux-nvdimm-leave@lists.01.org > _______________________________________________ > Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org > To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
> In struct dax_operations, the callback routine dax_supported() returns > a bool type result. For false return value, the caller has no idea > whether the device does not support dax at all, or it is just some mis- > configuration issue. > > An example is formatting an Ext4 file system on pmem device on top of > a NVDIMM namespace by, > # mkfs.ext4 /dev/pmem0 > If the fs block size does not match kernel space memory page size (which > is possible on non-x86 platform), mount this Ext4 file system will fail, > # mount -o dax /dev/pmem0 /mnt > mount: /mnt: wrong fs type, bad option, bad superblock on /dev/pmem0, > missing codepage or helper program, or other error. > And from the dmesg output there is only the following information, > [ 307.853148] EXT4-fs (pmem0): DAX unsupported by block device. > > The above information is quite confusing. Because definiately the pmem0 s/definiately/definitely > device supports dax operation, and the super block is consistent as how > it was created by mkfs.ext4. > > Indeed the failure is from __generic_fsdax_supported() by the following > code piece, > if (blocksize != PAGE_SIZE) { > pr_debug("%s: error: unsupported blocksize for dax\n", > bdevname(bdev, buf)); > return false; > } > It is because the Ext4 block size is 4KB and kernel page size is 8KB or > 16KB. > > It is not simple to make dax_supported() from struct dax_operations > or __generic_fsdax_supported() to return exact failure type right now. > So the simplest fix is to use pr_info() to print all the error messages > inside __generic_fsdax_supported(). Then users may find informative clue > from the kernel message at least. > > Message printed by pr_debug() is very easy to be ignored by users. This > patch prints error message by pr_info() in __generic_fsdax_supported(), > when then mount fails, following lines can be found from dmesg output, > [ 2705.500885] pmem0: error: unsupported blocksize for dax > [ 2705.500888] EXT4-fs (pmem0): DAX unsupported by block device. > Now the users may have idea the mount failure is from pmem driver for > unsupported block size. > > Reported-by: Michal Suchanek <msuchanek@suse.com> > Suggested-by: Jan Kara <jack@suse.com> > Signed-off-by: Coly Li <colyli@suse.de> > Reviewed-by: Jan Kara <jack@suse.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Anthony Iliopoulos <ailiopoulos@suse.com> > --- > Changelog: > v2: Add reviewed-by from Jan Kara > v1: initial version. > > drivers/dax/super.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/dax/super.c b/drivers/dax/super.c > index 8e32345be0f7..de0d02ec0347 100644 > --- a/drivers/dax/super.c > +++ b/drivers/dax/super.c > @@ -80,14 +80,14 @@ bool __generic_fsdax_supported(struct dax_device > *dax_dev, > int err, id; > if (blocksize != PAGE_SIZE) { > - pr_debug("%s: error: unsupported blocksize for dax\n", > + pr_info("%s: error: unsupported blocksize for dax\n", > bdevname(bdev, buf)); > return false; > } > err = bdev_dax_pgoff(bdev, start, PAGE_SIZE, &pgoff); > if (err) { > - pr_debug("%s: error: unaligned partition for dax\n", > + pr_info("%s: error: unaligned partition for dax\n", > bdevname(bdev, buf)); > return false; > } > @@ -95,7 +95,7 @@ bool __generic_fsdax_supported(struct dax_device *dax_dev, > last_page = PFN_DOWN((start + sectors - 1) * 512) * PAGE_SIZE / 512; > err = bdev_dax_pgoff(bdev, last_page, PAGE_SIZE, &pgoff_end); > if (err) { > - pr_debug("%s: error: unaligned partition for dax\n", > + pr_info("%s: error: unaligned partition for dax\n", > bdevname(bdev, buf)); > return false; > } > @@ -106,7 +106,7 @@ bool __generic_fsdax_supported(struct dax_device > *dax_dev, > dax_read_unlock(id); > if (len < 1 || len2 < 1) { > - pr_debug("%s: error: dax access failed (%ld)\n", > + pr_info("%s: error: dax access failed (%ld)\n", > bdevname(bdev, buf), len < 1 ? len : len2); > return false; > } > @@ -139,7 +139,7 @@ bool __generic_fsdax_supported(struct dax_device > *dax_dev, > } > if (!dax_enabled) { > - pr_debug("%s: error: dax support not enabled\n", > + pr_info("%s: error: dax support not enabled\n", > bdevname(bdev, buf)); > return false; > } > -- > 2.26.2 Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
diff --git a/drivers/dax/super.c b/drivers/dax/super.c index 8e32345be0f7..de0d02ec0347 100644 --- a/drivers/dax/super.c +++ b/drivers/dax/super.c @@ -80,14 +80,14 @@ bool __generic_fsdax_supported(struct dax_device *dax_dev, int err, id; if (blocksize != PAGE_SIZE) { - pr_debug("%s: error: unsupported blocksize for dax\n", + pr_info("%s: error: unsupported blocksize for dax\n", bdevname(bdev, buf)); return false; } err = bdev_dax_pgoff(bdev, start, PAGE_SIZE, &pgoff); if (err) { - pr_debug("%s: error: unaligned partition for dax\n", + pr_info("%s: error: unaligned partition for dax\n", bdevname(bdev, buf)); return false; } @@ -95,7 +95,7 @@ bool __generic_fsdax_supported(struct dax_device *dax_dev,
In struct dax_operations, the callback routine dax_supported() returns a bool type result. For false return value, the caller has no idea whether the device does not support dax at all, or it is just some mis- configuration issue. An example is formatting an Ext4 file system on pmem device on top of a NVDIMM namespace by, # mkfs.ext4 /dev/pmem0 If the fs block size does not match kernel space memory page size (which is possible on non-x86 platform), mount this Ext4 file system will fail, # mount -o dax /dev/pmem0 /mnt mount: /mnt: wrong fs type, bad option, bad superblock on /dev/pmem0, missing codepage or helper program, or other error. And from the dmesg output there is only the following information, [ 307.853148] EXT4-fs (pmem0): DAX unsupported by block device. The above information is quite confusing. Because definiately the pmem0 device supports dax operation, and the super block is consistent as how it was created by mkfs.ext4. Indeed the failure is from __generic_fsdax_supported() by the following code piece, if (blocksize != PAGE_SIZE) { pr_debug("%s: error: unsupported blocksize for dax\n", bdevname(bdev, buf)); return false; } It is because the Ext4 block size is 4KB and kernel page size is 8KB or 16KB. It is not simple to make dax_supported() from struct dax_operations or __generic_fsdax_supported() to return exact failure type right now. So the simplest fix is to use pr_info() to print all the error messages inside __generic_fsdax_supported(). Then users may find informative clue from the kernel message at least. Message printed by pr_debug() is very easy to be ignored by users. This patch prints error message by pr_info() in __generic_fsdax_supported(), when then mount fails, following lines can be found from dmesg output, [ 2705.500885] pmem0: error: unsupported blocksize for dax [ 2705.500888] EXT4-fs (pmem0): DAX unsupported by block device. Now the users may have idea the mount failure is from pmem driver for unsupported block size. Reported-by: Michal Suchanek <msuchanek@suse.com> Suggested-by: Jan Kara <jack@suse.com> Signed-off-by: Coly Li <colyli@suse.de> Reviewed-by: Jan Kara <jack@suse.com> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Anthony Iliopoulos <ailiopoulos@suse.com> --- Changelog: v2: Add reviewed-by from Jan Kara v1: initial version. drivers/dax/super.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) last_page = PFN_DOWN((start + sectors - 1) * 512) * PAGE_SIZE / 512; err = bdev_dax_pgoff(bdev, last_page, PAGE_SIZE, &pgoff_end); if (err) { - pr_debug("%s: error: unaligned partition for dax\n", + pr_info("%s: error: unaligned partition for dax\n", bdevname(bdev, buf)); return false; } @@ -106,7 +106,7 @@ bool __generic_fsdax_supported(struct dax_device *dax_dev, dax_read_unlock(id); if (len < 1 || len2 < 1) { - pr_debug("%s: error: dax access failed (%ld)\n", + pr_info("%s: error: dax access failed (%ld)\n", bdevname(bdev, buf), len < 1 ? len : len2); return false; } @@ -139,7 +139,7 @@ bool __generic_fsdax_supported(struct dax_device *dax_dev, } if (!dax_enabled) { - pr_debug("%s: error: dax support not enabled\n", + pr_info("%s: error: dax support not enabled\n", bdevname(bdev, buf)); return false; }