2018-06-27 01:44:29

by Ross Zwisler

[permalink] [raw]
Subject: [PATCH v5 0/3] Fix DM DAX handling

This series fixes a few issues that I found with DM's handling of DAX
devices. Here are some of the issues I found:

* We can create a dm-stripe or dm-linear device which is made up of an
fsdax PMEM namespace and a raw PMEM namespace but which can hold a
filesystem mounted with the -o dax mount option. DAX operations to
the raw PMEM namespace part lack struct page and can fail in
interesting/unexpected ways when doing things like fork(), examining
memory with gdb, etc.

* We can create a dm-stripe or dm-linear device which is made up of an
fsdax PMEM namespace and a BRD ramdisk which can hold a filesystem
mounted with the -o dax mount option. All I/O to this filesystem
will fail.

---

Changes since v4:
* No code changes.
* Updated the changelogs for patches 1 and 3.
* Removed the Cc: stable from patch 1.
* Added a Fixes: tag to patch 2.

Ross Zwisler (3):
pmem: only set QUEUE_FLAG_DAX for fsdax mode
dax: bdev_dax_supported() check for QUEUE_FLAG_DAX
dm: prevent DAX mounts if not supported

drivers/dax/super.c | 8 ++++++++
drivers/md/dm-table.c | 7 ++++---
drivers/md/dm.c | 3 +--
drivers/nvdimm/pmem.c | 3 ++-
4 files changed, 15 insertions(+), 6 deletions(-)

--
2.14.4



2018-06-27 01:44:33

by Ross Zwisler

[permalink] [raw]
Subject: [PATCH v5 1/3] pmem: only set QUEUE_FLAG_DAX for fsdax mode

QUEUE_FLAG_DAX is an indication that a given block device supports
filesystem DAX and should not be set for PMEM namespaces which are in "raw"
mode. These namespaces lack struct page and are prevented from
participating in filesystem DAX as of commit 569d0365f571 "dax: require
'struct page' by default for filesystem dax".

Signed-off-by: Ross Zwisler <[email protected]>
Suggested-by: Mike Snitzer <[email protected]>
---
drivers/nvdimm/pmem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 68940356cad3..8b1fd7f1a224 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -414,7 +414,8 @@ static int pmem_attach_disk(struct device *dev,
blk_queue_logical_block_size(q, pmem_sector_size(ndns));
blk_queue_max_hw_sectors(q, UINT_MAX);
blk_queue_flag_set(QUEUE_FLAG_NONROT, q);
- blk_queue_flag_set(QUEUE_FLAG_DAX, q);
+ if (pmem->pfn_flags & PFN_MAP)
+ blk_queue_flag_set(QUEUE_FLAG_DAX, q);
q->queuedata = pmem;

disk = alloc_disk_node(0, nid);
--
2.14.4


2018-06-27 01:45:59

by Ross Zwisler

[permalink] [raw]
Subject: [PATCH v5 3/3] dm: prevent DAX mounts if not supported

Currently device_supports_dax() just checks to see if the QUEUE_FLAG_DAX
flag is set on the device's request queue to decide whether or not the
device supports filesystem DAX. Really we should be using
bdev_dax_supported() like filesystems do at mount time. This performs
other tests like checking to make sure the dax_direct_access() path works.

We also explicitly clear QUEUE_FLAG_DAX on the DM device's request queue if
any of the underlying devices do not support DAX. This makes the handling
of QUEUE_FLAG_DAX consistent with the setting/clearing of most other flags
in dm_table_set_restrictions().

Now that bdev_dax_supported() explicitly checks for QUEUE_FLAG_DAX, this
will ensure that filesystems built upon DM devices will only be able to
mount with DAX if all underlying devices also support DAX.

Signed-off-by: Ross Zwisler <[email protected]>
Fixes: commit 545ed20e6df6 ("dm: add infrastructure for DAX support")
Cc: [email protected]
---
drivers/md/dm-table.c | 7 ++++---
drivers/md/dm.c | 3 +--
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 938766794c2e..3d0e2c198f06 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -885,9 +885,7 @@ EXPORT_SYMBOL_GPL(dm_table_set_type);
static int device_supports_dax(struct dm_target *ti, struct dm_dev *dev,
sector_t start, sector_t len, void *data)
{
- struct request_queue *q = bdev_get_queue(dev->bdev);
-
- return q && blk_queue_dax(q);
+ return bdev_dax_supported(dev->bdev, PAGE_SIZE);
}

static bool dm_table_supports_dax(struct dm_table *t)
@@ -1907,6 +1905,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,

if (dm_table_supports_dax(t))
blk_queue_flag_set(QUEUE_FLAG_DAX, q);
+ else
+ blk_queue_flag_clear(QUEUE_FLAG_DAX, q);
+
if (dm_table_supports_dax_write_cache(t))
dax_write_cache(t->md->dax_dev, true);

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index e65429a29c06..bef5a3f243ed 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1056,8 +1056,7 @@ static long dm_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
if (len < 1)
goto out;
nr_pages = min(len, nr_pages);
- if (ti->type->direct_access)
- ret = ti->type->direct_access(ti, pgoff, nr_pages, kaddr, pfn);
+ ret = ti->type->direct_access(ti, pgoff, nr_pages, kaddr, pfn);

out:
dm_put_live_table(md, srcu_idx);
--
2.14.4


2018-06-27 01:46:51

by Ross Zwisler

[permalink] [raw]
Subject: [PATCH v5 2/3] dax: bdev_dax_supported() check for QUEUE_FLAG_DAX

Add an explicit check for QUEUE_FLAG_DAX to __bdev_dax_supported(). This
is needed for DM configurations where the first element in the dm-linear or
dm-stripe target supports DAX, but other elements do not. Without this
check __bdev_dax_supported() will pass for such devices, letting a
filesystem on that device mount with the DAX option.

Signed-off-by: Ross Zwisler <[email protected]>
Suggested-by: Mike Snitzer <[email protected]>
Fixes: commit 545ed20e6df6 ("dm: add infrastructure for DAX support")
Cc: [email protected]
---
drivers/dax/super.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 903d9c473749..45276abf03aa 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -86,6 +86,7 @@ bool __bdev_dax_supported(struct block_device *bdev, int blocksize)
{
struct dax_device *dax_dev;
bool dax_enabled = false;
+ struct request_queue *q;
pgoff_t pgoff;
int err, id;
void *kaddr;
@@ -99,6 +100,13 @@ bool __bdev_dax_supported(struct block_device *bdev, int blocksize)
return false;
}

+ q = bdev_get_queue(bdev);
+ if (!q || !blk_queue_dax(q)) {
+ pr_debug("%s: error: request queue doesn't support dax\n",
+ bdevname(bdev, buf));
+ return false;
+ }
+
err = bdev_dax_pgoff(bdev, 0, PAGE_SIZE, &pgoff);
if (err) {
pr_debug("%s: error: unaligned partition for dax\n",
--
2.14.4


2018-06-27 01:49:40

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v5 0/3] Fix DM DAX handling

On Tue, Jun 26, 2018 at 3:30 PM, Ross Zwisler
<[email protected]> wrote:
> This series fixes a few issues that I found with DM's handling of DAX
> devices. Here are some of the issues I found:
>
> * We can create a dm-stripe or dm-linear device which is made up of an
> fsdax PMEM namespace and a raw PMEM namespace but which can hold a
> filesystem mounted with the -o dax mount option. DAX operations to
> the raw PMEM namespace part lack struct page and can fail in
> interesting/unexpected ways when doing things like fork(), examining
> memory with gdb, etc.
>
> * We can create a dm-stripe or dm-linear device which is made up of an
> fsdax PMEM namespace and a BRD ramdisk which can hold a filesystem
> mounted with the -o dax mount option. All I/O to this filesystem
> will fail.
>

For the series:

Acked-by: Dan Williams <[email protected]>