Re: [PATCH v4 6/6] block: Update blkdev_dax_capable() for consistency

[Toshi Kani <toshi.kani@hpe.com>]

On Tue, 2016-05-17 at 15:07 -0700, Dan Williams wrote:
> On Tue, May 10, 2016 at 9:23 AM, Toshi Kani <toshi.kani@hpe.com> wrote:
> > 
> > blkdev_dax_capable() is similar to bdev_dax_supported(), but needs
> > to remain as a separate interface for checking dax capability of
> > a raw block device.
> > 
> > Rename and relocate blkdev_dax_capable() to keep them maintained
> > consistently, and call bdev_direct_access() for the dax capability
> > check.
> > 
> > There is no change in the behavior.
> > 
 :
> > +/**
> > + * bdev_dax_capable() - Return if the raw device is capable for dax
> > + * @bdev: The device for raw block device access
> > + */
> > +bool bdev_dax_capable(struct block_device *bdev)
> > +{
> > +       struct gendisk *disk = bdev->bd_disk;
> > +       struct blk_dax_ctl dax = {
> > +               .size = PAGE_SIZE,
> > +       };
> > +
> > +       if (!IS_ENABLED(CONFIG_FS_DAX))
> > +               return false;
> > +
> > +       dax.sector = 0;
> > +       if (bdev_direct_access(bdev, &dax) < 0)
> > +               return false;
> > +
> > +       dax.sector = bdev->bd_part->nr_sects - (PAGE_SIZE / 512);
> > +       if (bdev_direct_access(bdev, &dax) < 0)
> > +               return false;
>
> So I just noticed that this new implementation of bdev_dax_capable()
> prevents us from enabling DAX in the presence of errors, which was the
> goal of Vishal's pending patches for 4.7.
> 
> I like the goal of centralizing checks, but this collides the base DAX
> capability with the ability to perform an actual accesses at a given
> address.
> 
> All the immediate solutions that come to mind right now are pretty
> ugly, like an ignore errors flag...
> 
> Hmm, what about explicitly returning -EBADBLOCK and updating size to
> return the offset of the error?  That might be useful information to
> have in general...

The function only checks a PAGE_SIZE range at the start and the end, but I
agree that it can handle -EBADBLOCK (or -EIO) from bdev_direct_access() as
an error but the device is DAX-capable.  bdev_dax_supported() may need the
same change as well (although I wonder how a bad block at the start is
handled).  Shall I send you a patch for this change?  (BTW, do we have a
patch that defines EBADBLOCK already?)  I think the error offset would be
useful, but that change can be made when we use it.

Thanks,
-Toshi
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm