[PATCH 0/2] Support DAX for device-mapper dm-linear devices

STALE3688d

Revision v1 of 3 in this series.

10 messages, 3 authors, 2016-06-29 · open the first message on its own page

[PATCH 0/2] Support DAX for device-mapper dm-linear devices

From: Toshi Kani <hidden>
Date: 2016-06-28 19:38:34 

I noticed that dm-snap reloads DM table of target mapped-device, which
fails for dax-capable device after dax support is added.  Ideally,
adding dax support to dm-snap solves the issue, but it cannot be done
easily.  This patch-set allows dm-snap to work with dax-capable target
devices when bio-based operation is used.

dax operation is unsupported with dm-snap, such that:
 a) After snapshot is taken, mount with dax option to a target device
    or a snapshot device fails.  They can be mounted without dax.
 b) After snapshot is taken to a dax-mounted target device, any writes 
    to the target device fails (EIO).

b) can be protected by changing lvcreate to fail when snapshot is
requested to a dax-mounted target device. 

- Patch 1 solves an error when lvremove is made to a snapshot device.
- Patch 2 solves an error when lvcreate --snapshot is made to a dax-
  capable device.

---
Toshi Kani (2):
 1/2 dm: update table type check for dax
 2/2 dm snap: add fake origin_direct_access

---
 drivers/md/dm-ioctl.c | 11 ++++++++++-
 drivers/md/dm-snap.c  |  8 ++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

[PATCH 1/2] dm: update table type check for dax

From: Toshi Kani <hidden>
Date: 2016-06-28 19:38:36 

Allow table type DM_TYPE_BIO_BASED to extend with
DM_TYPE_DAX_BIO_BASED since DM_TYPE_DAX_BIO_BASED
supports bio-based requests.

Signed-off-by: Toshi Kani <redacted>
Cc: Mike Snitzer <redacted>
Cc: Alasdair Kergon <redacted>
Cc: Dan Williams <redacted>
---
 drivers/md/dm-ioctl.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index b59e3459..0f32791 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1267,6 +1267,15 @@ static int populate_table(struct dm_table *table,
 	return dm_table_complete(table);
 }
 
+static bool is_valid_type(unsigned cur, unsigned new)
+{
+	if (cur == new ||
+	    (cur == DM_TYPE_BIO_BASED && new == DM_TYPE_DAX_BIO_BASED))
+		return true;
+
+	return false;
+}
+
 static int table_load(struct dm_ioctl *param, size_t param_size)
 {
 	int r;
@@ -1309,7 +1318,7 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
 			DMWARN("unable to set up device queue for new table.");
 			goto err_unlock_md_type;
 		}
-	} else if (dm_get_md_type(md) != dm_table_get_type(t)) {
+	} else if (!is_valid_type(dm_get_md_type(md), dm_table_get_type(t))) {
 		DMWARN("can't change device type after initial table load.");
 		r = -EINVAL;
 		goto err_unlock_md_type;

[PATCH 2/2] dm snap: add fake origin_direct_access

From: Toshi Kani <hidden>
Date: 2016-06-28 19:39:06 

dax-capable mapped-device is marked as DM_TYPE_DAX_BIO_BASED,
which supports both dax and bio-based operations.  dm-snap
needs to work with dax-capable device when bio-based operation
is used.

Add fake origin_direct_access() to origin device so that its
origin device is also marked as DM_TYPE_DAX_BIO_BASED for
dax-capable device.  This allows to extend target's DM table.
dm-snap works normally when bio-based operation is used.

dm-snap does not support dax operation, and mount with dax
option to a target device or snapshot device fails.

Signed-off-by: Toshi Kani <redacted>
Cc: Mike Snitzer <redacted>
Cc: Alasdair Kergon <redacted>
Cc: Dan Williams <redacted>
---
 drivers/md/dm-snap.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 69ab1ff..c472f04 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -2301,6 +2301,13 @@ static int origin_map(struct dm_target *ti, struct bio *bio)
 	return do_origin(o->dev, bio);
 }
 
+static long origin_direct_access(struct dm_target *ti, sector_t sector,
+		void __pmem **kaddr, pfn_t *pfn, long size)
+{
+	DMWARN("device does not support dax.");
+	return -EIO;
+}
+
 /*
  * Set the target "max_io_len" field to the minimum of all the snapshots'
  * chunk sizes.
@@ -2360,6 +2367,7 @@ static struct target_type origin_target = {
 	.postsuspend = origin_postsuspend,
 	.status  = origin_status,
 	.iterate_devices = origin_iterate_devices,
+	.direct_access = origin_direct_access,
 };
 
 static struct target_type snapshot_target = {

Re: [PATCH 0/2] fix dm-snap for dax

From: Kani, Toshimitsu <hidden>
Date: 2016-06-28 19:43:33 

Sorry, subject line should be "[PATCH 0/2] fix dm-snap for dax"
-Toshi

On Tue, 2016-06-28 at 13:37 -0600, Toshi Kani wrote:
I noticed that dm-snap reloads DM table of target mapped-device, which
fails for dax-capable device after dax support is added.  Ideally,
adding dax support to dm-snap solves the issue, but it cannot be done
easily.  This patch-set allows dm-snap to work with dax-capable target
devices when bio-based operation is used.

dax operation is unsupported with dm-snap, such that:
 a) After snapshot is taken, mount with dax option to a target device
    or a snapshot device fails.  They can be mounted without dax.
 b) After snapshot is taken to a dax-mounted target device, any writes 
    to the target device fails (EIO).

b) can be protected by changing lvcreate to fail when snapshot is
requested to a dax-mounted target device. 

- Patch 1 solves an error when lvremove is made to a snapshot device.
- Patch 2 solves an error when lvcreate --snapshot is made to a dax-
  capable device.

---
Toshi Kani (2):
 1/2 dm: update table type check for dax
 2/2 dm snap: add fake origin_direct_access

---
 drivers/md/dm-ioctl.c | 11 ++++++++++-
 drivers/md/dm-snap.c  |  8 ++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH 1/2] dm: update table type check for dax

From: Mike Snitzer <hidden>
Date: 2016-06-28 20:07:19 

On Tue, Jun 28 2016 at  3:37pm -0400,
Toshi Kani [off-list ref] wrote:
quoted hunk
Allow table type DM_TYPE_BIO_BASED to extend with
DM_TYPE_DAX_BIO_BASED since DM_TYPE_DAX_BIO_BASED
supports bio-based requests.

Signed-off-by: Toshi Kani <redacted>
Cc: Mike Snitzer <redacted>
Cc: Alasdair Kergon <redacted>
Cc: Dan Williams <redacted>
---
 drivers/md/dm-ioctl.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index b59e3459..0f32791 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1267,6 +1267,15 @@ static int populate_table(struct dm_table *table,
 	return dm_table_complete(table);
 }
 
+static bool is_valid_type(unsigned cur, unsigned new)
+{
+	if (cur == new ||
+	    (cur == DM_TYPE_BIO_BASED && new == DM_TYPE_DAX_BIO_BASED))
+		return true;
+
+	return false;
+}
+
 static int table_load(struct dm_ioctl *param, size_t param_size)
 {
 	int r;
@@ -1309,7 +1318,7 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
 			DMWARN("unable to set up device queue for new table.");
 			goto err_unlock_md_type;
 		}
-	} else if (dm_get_md_type(md) != dm_table_get_type(t)) {
+	} else if (!is_valid_type(dm_get_md_type(md), dm_table_get_type(t))) {
 		DMWARN("can't change device type after initial table load.");
 		r = -EINVAL;
 		goto err_unlock_md_type;
You said in the 0th header: "Patch 1 solves an error when lvremove is
made to a snapshot device."

I'm not seeing why this patch 1 fixes anything specific to snapshot
device removal (but I can see why patch 2 makes snapshot creation
"work").  I'll apply your 2nd patch and see if I can see what you mean.

I actually see this error, without either of your 2 proposed patches
applied, when I try to create a snapshot of a DAX capable LV:

# lvcreate -s -n snap -L 100M pmem/lv
  device-mapper: reload ioctl on (253:7) failed: Invalid argument
  Failed to lock logical volume pmem/lv.
  Aborting. Manual intervention required.
Jun 28 15:57:28 rhel-storage-02 kernel: device-mapper: ioctl: can't change device type after initial table load.

Re: [PATCH 1/2] dm: update table type check for dax

From: Kani, Toshimitsu <hidden>
Date: 2016-06-28 20:23:30 

On Tue, 2016-06-28 at 16:07 -0400, Mike Snitzer wrote:
On Tue, Jun 28 2016 at  3:37pm -0400,
Toshi Kani [off-list ref] wrote:
 :
You said in the 0th header: "Patch 1 solves an error when lvremove is
made to a snapshot device."

I'm not seeing why this patch 1 fixes anything specific to snapshot
device removal (but I can see why patch 2 makes snapshot creation
"work").  I'll apply your 2nd patch and see if I can see what you mean.

I actually see this error, without either of your 2 proposed patches
applied, when I try to create a snapshot of a DAX capable LV:

# lvcreate -s -n snap -L 100M pmem/lv
  device-mapper: reload ioctl on (253:7) failed: Invalid argument
  Failed to lock logical volume pmem/lv.
  Aborting. Manual intervention required.
Jun 28 15:57:28 rhel-storage-02 kernel: device-mapper: ioctl: can't change
device type after initial table load.
Yes, patch 2 fixes this error.

I have not looked into why lvremove does this, but lvremove to a snapshot
device fails to reload DM table of "<dev>-lvsnap" device (which is marked as
DM_TYPE_BIO_BASED) with DM_TYPE_DAX_BIO_BASED.  Patch 1 fixes this error.  I
think it also generally makes sense to allow this case.

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

Re: [PATCH 2/2] dm snap: add fake origin_direct_access

From: kbuild test robot <hidden>
Date: 2016-06-28 20:28:04 

Hi,

[auto build test WARNING on dm/for-next]
[also build test WARNING on v4.7-rc5 next-20160628]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Toshi-Kani/Support-DAX-for-device-mapper-dm-linear-devices/20160629-034110
base:   https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git for-next
config: x86_64-lkp (attached as .config)
compiler: gcc-4.9 (Debian 4.9.3-14) 4.9.3
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/md/dm-snap.c:2370:2: error: unknown field 'direct_access' specified in initializer
     .direct_access = origin_direct_access,
     ^
quoted
drivers/md/dm-snap.c:2370:2: warning: initialization from incompatible pointer type
   drivers/md/dm-snap.c:2370:2: warning: (near initialization for 'origin_target.io_hints')

vim +2370 drivers/md/dm-snap.c

  2354		struct dm_origin *o = ti->private;
  2355	
  2356		return fn(ti, o->dev, 0, ti->len, data);
  2357	}
  2358	
  2359	static struct target_type origin_target = {
  2360		.name    = "snapshot-origin",
  2361		.version = {1, 9, 0},
  2362		.module  = THIS_MODULE,
  2363		.ctr     = origin_ctr,
  2364		.dtr     = origin_dtr,
  2365		.map     = origin_map,
  2366		.resume  = origin_resume,
  2367		.postsuspend = origin_postsuspend,
  2368		.status  = origin_status,
  2369		.iterate_devices = origin_iterate_devices,
2370		.direct_access = origin_direct_access,
  2371	};
  2372	
  2373	static struct target_type snapshot_target = {
  2374		.name    = "snapshot",
  2375		.version = {1, 15, 0},
  2376		.module  = THIS_MODULE,
  2377		.ctr     = snapshot_ctr,
  2378		.dtr     = snapshot_dtr,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH 1/2] dm: update table type check for dax

From: kbuild test robot <hidden>
Date: 2016-06-28 20:38:09 

Hi,

[auto build test ERROR on dm/for-next]
[also build test ERROR on v4.7-rc5 next-20160628]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Toshi-Kani/Support-DAX-for-device-mapper-dm-linear-devices/20160629-034110
base:   https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git for-next
config: x86_64-randconfig-s1-06290340 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/md/dm-ioctl.c: In function 'is_valid_type':
quoted
drivers/md/dm-ioctl.c:1273:42: error: 'DM_TYPE_DAX_BIO_BASED' undeclared (first use in this function)
         (cur == DM_TYPE_BIO_BASED && new == DM_TYPE_DAX_BIO_BASED))
                                             ^~~~~~~~~~~~~~~~~~~~~
   drivers/md/dm-ioctl.c:1273:42: note: each undeclared identifier is reported only once for each function it appears in

vim +/DM_TYPE_DAX_BIO_BASED +1273 drivers/md/dm-ioctl.c

  1267		return dm_table_complete(table);
  1268	}
  1269	
  1270	static bool is_valid_type(unsigned cur, unsigned new)
  1271	{
  1272		if (cur == new ||
1273		    (cur == DM_TYPE_BIO_BASED && new == DM_TYPE_DAX_BIO_BASED))
  1274			return true;
  1275	
  1276		return false;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH 1/2] dm: update table type check for dax

From: Mike Snitzer <hidden>
Date: 2016-06-29 00:41:31 

On Tue, Jun 28 2016 at  4:23pm -0400,
Kani, Toshimitsu [off-list ref] wrote:

On Tue, 2016-06-28 at 16:07 -0400, Mike Snitzer wrote:
quoted
On Tue, Jun 28 2016 at  3:37pm -0400,
Toshi Kani [off-list ref] wrote:
 :
quoted
You said in the 0th header: "Patch 1 solves an error when lvremove is
made to a snapshot device."

I'm not seeing why this patch 1 fixes anything specific to snapshot
device removal (but I can see why patch 2 makes snapshot creation
"work").  I'll apply your 2nd patch and see if I can see what you mean.

I actually see this error, without either of your 2 proposed patches
applied, when I try to create a snapshot of a DAX capable LV:

# lvcreate -s -n snap -L 100M pmem/lv
  device-mapper: reload ioctl on (253:7) failed: Invalid argument
  Failed to lock logical volume pmem/lv.
  Aborting. Manual intervention required.
Jun 28 15:57:28 rhel-storage-02 kernel: device-mapper: ioctl: can't change
device type after initial table load.
Yes, patch 2 fixes this error.

I have not looked into why lvremove does this, but lvremove to a snapshot
device fails to reload DM table of "<dev>-lvsnap" device (which is marked as
DM_TYPE_BIO_BASED) with DM_TYPE_DAX_BIO_BASED.  Patch 1 fixes this error.
It looks like a strange intermediate state that lvm2 uses during
snapshot removal.

Full listing of snapshot related DM tables (before lvremove):

pmem-lv-real: 0 6086656 linear 259:0 2048
pmem-lv: 0 6086656 snapshot-origin 253:5
pmem-snap-cow: 0 204800 linear 259:0 6088704
pmem-snap: 0 6086656 snapshot 253:5 253:6 P 8

When removing this snapshot we're wanting to be left with:

pmem-lv: 0 6086656 linear 259:0 2048

I augmented the DM core error to be more expressive, resulting in:
  device-mapper: ioctl: 253:7: can't change device type (from 1 to 4) after initial table load.

1 is DM_TYPE_BIO_BASED and 4 is DM_TYPE_DAX_BIO_BASED -- which makes
sense given the linear target is DM_TYPE_DAX_BIO_BASED.

The previous DM table for 253:7 was:
  pmem-snap: 0 6086656 snapshot 253:5 253:6 P 8

The intermediate table that lvm2 is trying to load for 253:7 is:
  0 204800 linear 259:0 6088704

(this linear target was previously pmem-snap-cow)

I think it also generally makes sense to allow this case.
You're probably right but I need to think about it a little bit more.

Re: [PATCH 1/2] dm: update table type check for dax

From: Kani, Toshimitsu <hidden>
Date: 2016-06-29 15:19:13 

On Tue, 2016-06-28 at 20:40 -0400, Mike Snitzer wrote:
On Tue, Jun 28 2016 at  4:23pm -0400,
Kani, Toshimitsu [off-list ref] wrote:
quoted
On Tue, 2016-06-28 at 16:07 -0400, Mike Snitzer wrote:
quoted
On Tue, Jun 28 2016 at  3:37pm -0400,
Toshi Kani [off-list ref] wrote:
 :
quoted
You said in the 0th header: "Patch 1 solves an error when lvremove is
made to a snapshot device."

I'm not seeing why this patch 1 fixes anything specific to snapshot
device removal (but I can see why patch 2 makes snapshot creation
"work").  I'll apply your 2nd patch and see if I can see what you mean.

I actually see this error, without either of your 2 proposed patches
applied, when I try to create a snapshot of a DAX capable LV:

# lvcreate -s -n snap -L 100M pmem/lv
  device-mapper: reload ioctl on (253:7) failed: Invalid argument
  Failed to lock logical volume pmem/lv.
  Aborting. Manual intervention required.
Jun 28 15:57:28 rhel-storage-02 kernel: device-mapper: ioctl: can't
change device type after initial table load.
Yes, patch 2 fixes this error.

I have not looked into why lvremove does this, but lvremove to a snapshot
device fails to reload DM table of "<dev>-lvsnap" device (which is marked
as DM_TYPE_BIO_BASED) with DM_TYPE_DAX_BIO_BASED.  Patch 1 fixes this
error.
It looks like a strange intermediate state that lvm2 uses during
snapshot removal.

Full listing of snapshot related DM tables (before lvremove):

pmem-lv-real: 0 6086656 linear 259:0 2048
pmem-lv: 0 6086656 snapshot-origin 253:5
pmem-snap-cow: 0 204800 linear 259:0 6088704
pmem-snap: 0 6086656 snapshot 253:5 253:6 P 8

When removing this snapshot we're wanting to be left with:

pmem-lv: 0 6086656 linear 259:0 2048

I augmented the DM core error to be more expressive, resulting in:
  device-mapper: ioctl: 253:7: can't change device type (from 1 to 4) after
initial table load.

1 is DM_TYPE_BIO_BASED and 4 is DM_TYPE_DAX_BIO_BASED -- which makes
sense given the linear target is DM_TYPE_DAX_BIO_BASED.

The previous DM table for 253:7 was:
  pmem-snap: 0 6086656 snapshot 253:5 253:6 P 8

The intermediate table that lvm2 is trying to load for 253:7 is:
  0 204800 linear 259:0 6088704

(this linear target was previously pmem-snap-cow)
Yes, that is consistent with what I saw.
quoted
I think it also generally makes sense to allow this case.
You're probably right but I need to think about it a little bit more.
Sounds good.

Thanks!
-Toshi
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help