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 <[email protected]>
> Suggested-by: Jan Kara <[email protected]>
> Signed-off-by: Coly Li <[email protected]>
> Reviewed-by: Jan Kara <[email protected]>
Yes this looks good to me as well.
Reviewed-by: Ira Weiny <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Anthony Iliopoulos <[email protected]>
> ---
> 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 -- [email protected]
> To unsubscribe send an email to [email protected]
> _______________________________________________
> Linux-nvdimm mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]
