diff mbox series

[v2,12/30] dax: remove block device dependencies

Message ID 20190515192715.18000-13-vgoyal@redhat.com (mailing list archive)
State New, archived
Headers show
Series virtio-fs: shared file system for virtual machines | expand

Commit Message

Vivek Goyal May 15, 2019, 7:26 p.m. UTC
From: Stefan Hajnoczi <stefanha@redhat.com>

Although struct dax_device itself is not tied to a block device, some
DAX code assumes there is a block device.  Make block devices optional
by allowing bdev to be NULL in commonly used DAX APIs.

When there is no block device:
 * Skip the partition offset calculation in bdev_dax_pgoff()
 * Skip the blkdev_issue_zeroout() optimization

Note that more block device assumptions remain but I haven't reach those
code paths yet.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 drivers/dax/super.c | 3 ++-
 fs/dax.c            | 7 ++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

Comments

Dan Williams May 16, 2019, 12:21 a.m. UTC | #1
On Wed, May 15, 2019 at 12:28 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> From: Stefan Hajnoczi <stefanha@redhat.com>
>
> Although struct dax_device itself is not tied to a block device, some
> DAX code assumes there is a block device.  Make block devices optional
> by allowing bdev to be NULL in commonly used DAX APIs.
>
> When there is no block device:
>  * Skip the partition offset calculation in bdev_dax_pgoff()
>  * Skip the blkdev_issue_zeroout() optimization
>
> Note that more block device assumptions remain but I haven't reach those
> code paths yet.
>

Is there a generic object that non-block-based filesystems reference
for physical storage as a bdev stand-in? I assume "sector_t" is still
the common type for addressing filesystem capacity?

It just seems to me that we should stop pretending that the
filesystem-dax facility requires block devices and try to move this
functionality to generically use a dax device across all interfaces.
Stefan Hajnoczi May 16, 2019, 10:07 a.m. UTC | #2
On Wed, May 15, 2019 at 05:21:51PM -0700, Dan Williams wrote:
> On Wed, May 15, 2019 at 12:28 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > From: Stefan Hajnoczi <stefanha@redhat.com>
> >
> > Although struct dax_device itself is not tied to a block device, some
> > DAX code assumes there is a block device.  Make block devices optional
> > by allowing bdev to be NULL in commonly used DAX APIs.
> >
> > When there is no block device:
> >  * Skip the partition offset calculation in bdev_dax_pgoff()
> >  * Skip the blkdev_issue_zeroout() optimization
> >
> > Note that more block device assumptions remain but I haven't reach those
> > code paths yet.
> >
> 
> Is there a generic object that non-block-based filesystems reference
> for physical storage as a bdev stand-in? I assume "sector_t" is still
> the common type for addressing filesystem capacity?
> 
> It just seems to me that we should stop pretending that the
> filesystem-dax facility requires block devices and try to move this
> functionality to generically use a dax device across all interfaces.

virtio-fs uses a PCI BAR called the DAX Window to access data.  This
object is internal to the virtio_fs.ko driver, not really a generic
object that DAX code can reference.

But does the DAX code need to reference any object at all?  It seems
like block device users just want callbacks for the partition offset
calculation and blkdev_issue_zeroout().
Vivek Goyal May 16, 2019, 2:23 p.m. UTC | #3
On Wed, May 15, 2019 at 05:21:51PM -0700, Dan Williams wrote:

[..]
> It just seems to me that we should stop pretending that the
> filesystem-dax facility requires block devices and try to move this
> functionality to generically use a dax device across all interfaces.

That sounds reasonable and will help with our use case where we don't
have the block device at all.

Vivek
diff mbox series

Patch

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 0a339b85133e..cb44ec663991 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -53,7 +53,8 @@  EXPORT_SYMBOL_GPL(dax_read_unlock);
 int bdev_dax_pgoff(struct block_device *bdev, sector_t sector, size_t size,
 		pgoff_t *pgoff)
 {
-	phys_addr_t phys_off = (get_start_sect(bdev) + sector) * 512;
+	sector_t start_sect = bdev ? get_start_sect(bdev) : 0;
+	phys_addr_t phys_off = (start_sect + sector) * 512;
 
 	if (pgoff)
 		*pgoff = PHYS_PFN(phys_off);
diff --git a/fs/dax.c b/fs/dax.c
index e5e54da1715f..815bc32fd967 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1042,7 +1042,12 @@  static vm_fault_t dax_load_hole(struct xa_state *xas,
 static bool dax_range_is_aligned(struct block_device *bdev,
 				 unsigned int offset, unsigned int length)
 {
-	unsigned short sector_size = bdev_logical_block_size(bdev);
+	unsigned short sector_size;
+
+	if (!bdev)
+		return false;
+
+	sector_size = bdev_logical_block_size(bdev);
 
 	if (!IS_ALIGNED(offset, sector_size))
 		return false;