All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] NVMe 1.4 Identify Namespace Support
@ 2019-06-27 14:32 Bart Van Assche
  2019-06-27 14:32 ` [PATCH v4 1/3] nvme: Introduce NVMe 1.4 Identify Namespace fields in struct nvme_id_ns Bart Van Assche
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Bart Van Assche @ 2019-06-27 14:32 UTC (permalink / raw)


Hi Christoph,

This patch series adds support for several of the new parameters introduced
in the NVMe 1.4 Identify Namespace command. Please consider these patches
for kernel version 5.3.

Thanks,

Bart.

Changes compared to v3:
- In the file backend code, do not set any of the namespace atomic properties.

Changes compared to v2:
- In the initiator, use AWUPF instead NAWUPF if NAWUPF == 0.

Changes compared to v1:
- Added a patch for the NVMe target code that exports these parameters.
- Limited the physical block size to the value of AWUPF/NAWUPF as appropriate.

Bart Van Assche (3):
  nvme: Introduce NVMe 1.4 Identify Namespace fields in struct
    nvme_id_ns
  nvmet: Export NVMe namespace attributes
  nvme: Set physical block size and optimal I/O size according to NVMe
    1.4

Bart Van Assche (3):
  nvme: Introduce NVMe 1.4 Identify Namespace fields in struct
    nvme_id_ns
  nvmet: Export NVMe namespace attributes
  nvme: Set physical block size and optimal I/O size according to NVMe
    1.4

 drivers/nvme/host/core.c          | 30 +++++++++++++++++++--
 drivers/nvme/host/nvme.h          |  1 +
 drivers/nvme/target/admin-cmd.c   |  5 ++++
 drivers/nvme/target/io-cmd-bdev.c | 45 +++++++++++++++++++++++++++++++
 drivers/nvme/target/io-cmd-file.c | 12 +++++++++
 drivers/nvme/target/nvmet.h       |  2 ++
 include/linux/nvme.h              | 12 ++++++---
 7 files changed, 102 insertions(+), 5 deletions(-)

-- 
2.21.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/3] nvme: Introduce NVMe 1.4 Identify Namespace fields in struct nvme_id_ns
  2019-06-27 14:32 [PATCH v4 0/3] NVMe 1.4 Identify Namespace Support Bart Van Assche
@ 2019-06-27 14:32 ` Bart Van Assche
  2019-06-28  7:00   ` Christoph Hellwig
  2019-06-27 14:32 ` [PATCH v4 2/3] nvmet: Export NVMe namespace attributes Bart Van Assche
  2019-06-27 14:32 ` [PATCH v4 3/3] nvme: Set physical block size and optimal I/O size according to NVMe 1.4 Bart Van Assche
  2 siblings, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2019-06-27 14:32 UTC (permalink / raw)


Several new fields have been introduced in version 1.4 of the NVMe spec
at offsets that were defined as reserved in version 1.3d of the NVMe
spec. Update the definition of the nvme_id_ns data structure such that
it is in sync with version 1.4 of the NVMe spec. This change preserves
backwards compatibility.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
Reviewed-by: Martin K. Petersen <martin.petersen at oracle.com>
Reviewed-by: Keith Busch <kbusch at kernel.org>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Hannes Reinecke <hare at suse.com>
Signed-off-by: Bart Van Assche <bvanassche at acm.org>
---
 include/linux/nvme.h | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index d98b2d8baf4e..01aa6a6c241d 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -315,7 +315,7 @@ struct nvme_id_ns {
 	__u8			nmic;
 	__u8			rescap;
 	__u8			fpi;
-	__u8			rsvd33;
+	__u8			dlfeat;
 	__le16			nawun;
 	__le16			nawupf;
 	__le16			nacwu;
@@ -324,11 +324,17 @@ struct nvme_id_ns {
 	__le16			nabspf;
 	__le16			noiob;
 	__u8			nvmcap[16];
-	__u8			rsvd64[28];
+	__le16			npwg;
+	__le16			npwa;
+	__le16			npdg;
+	__le16			npda;
+	__le16			nows;
+	__u8			rsvd74[18];
 	__le32			anagrpid;
 	__u8			rsvd96[3];
 	__u8			nsattr;
-	__u8			rsvd100[4];
+	__le16			nvmsetid;
+	__le16			endgid;
 	__u8			nguid[16];
 	__u8			eui64[8];
 	struct nvme_lbaf	lbaf[16];
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/3] nvmet: Export NVMe namespace attributes
  2019-06-27 14:32 [PATCH v4 0/3] NVMe 1.4 Identify Namespace Support Bart Van Assche
  2019-06-27 14:32 ` [PATCH v4 1/3] nvme: Introduce NVMe 1.4 Identify Namespace fields in struct nvme_id_ns Bart Van Assche
@ 2019-06-27 14:32 ` Bart Van Assche
  2019-06-27 17:11   ` Chaitanya Kulkarni
  2019-06-28  7:02   ` Christoph Hellwig
  2019-06-27 14:32 ` [PATCH v4 3/3] nvme: Set physical block size and optimal I/O size according to NVMe 1.4 Bart Van Assche
  2 siblings, 2 replies; 8+ messages in thread
From: Bart Van Assche @ 2019-06-27 14:32 UTC (permalink / raw)


Make the NVMe NAWUN, NAWUPF, NACWU, NPWG, NPWA, NPDG and NOWS attributes
available to initator systems for the block backend.

Cc: Keith Busch <kbusch at kernel.org>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Martin K. Petersen <martin.petersen at oracle.com>
Cc: Hannes Reinecke <hare at suse.com>
Cc: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
Signed-off-by: Bart Van Assche <bvanassche at acm.org>
---
 drivers/nvme/target/admin-cmd.c   |  5 ++++
 drivers/nvme/target/io-cmd-bdev.c | 45 +++++++++++++++++++++++++++++++
 drivers/nvme/target/io-cmd-file.c | 12 +++++++++
 drivers/nvme/target/nvmet.h       |  2 ++
 4 files changed, 64 insertions(+)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 9f72d515fc4b..2558ed333d29 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -442,6 +442,11 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 		break;
         }
 
+	if (ns->file)
+		nvmet_file_set_limits(ns->file, id);
+	else if (ns->bdev)
+		nvmet_bdev_set_limits(ns->bdev, id);
+
 	/*
 	 * We just provide a single LBA format that matches what the
 	 * underlying device reports.
diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
index 7a1cf6437a6a..6e57d0d4f4e9 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -8,6 +8,51 @@
 #include <linux/module.h>
 #include "nvmet.h"
 
+/* Convert a 32-bit number to a 16-bit 0's based number */
+static __le16 to0based(u32 a)
+{
+	return cpu_to_le16(max(1U, min(1U << 16, a)) - 1);
+}
+
+void nvmet_bdev_set_limits(struct block_device *bdev, struct nvme_id_ns *id)
+{
+	const struct queue_limits *ql = &bdev_get_queue(bdev)->limits;
+	/* Number of physical blocks per logical block. */
+	const u32 ppl = ql->physical_block_size / ql->logical_block_size;
+	/* Physical blocks per logical block, 0's based. */
+	const __le16 ppl0b = to0based(ppl);
+
+	/*
+	 * For NVMe 1.2 and later, bit 1 indicates that the fields NAWUN,
+	 * NAWUPF, and NACWU are defined for this namespace and should be
+	 * used by the host for this namespace instead of the AWUN, AWUPF,
+	 * and ACWU fields in the Identify Controller data structure. If
+	 * any of these fields are zero that means that the corresponding
+	 * field from the identify controller data structure should be used.
+	 */
+	id->nsfeat |= 1 << 1;
+	id->nawun = ppl0b;
+	id->nawupf = ppl0b;
+	id->nacwu = ppl0b;
+
+	/*
+	 * For NVMe 1.4 and later, bit 4 indicates that the fields NPWG,
+	 * NPWA, NPDG, NPDA, and NOWS are defined for this namespace and
+	 * should be used by the host for I/O optimization.
+	 */
+	id->nsfeat |= 1 << 4;
+	/* NPWG = Namespace Preferred Write Granularity. 0's based */
+	id->npwg = ppl0b;
+	/* NPWA = Namespace Preferred Write Alignment. 0's based */
+	id->npwa = id->npwg;
+	/* NPDG = Namespace Preferred Deallocate Granularity. 0's based */
+	id->npdg = to0based(ql->discard_granularity / ql->logical_block_size);
+	/* NPDG = Namespace Preferred Deallocate Alignment */
+	id->npda = id->npdg;
+	/* NOWS = Namespace Optimal Write Size */
+	id->nows = to0based(ql->io_opt / ql->logical_block_size);
+}
+
 int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
 {
 	int ret;
diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 05453f5d1448..8e59c107ec38 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -13,6 +13,18 @@
 #define NVMET_MAX_MPOOL_BVEC		16
 #define NVMET_MIN_MPOOL_OBJ		16
 
+void nvmet_file_set_limits(struct file *file, struct nvme_id_ns *id)
+{
+	/*
+	 * Since the file backend uses buffered I/O and since the NVMe spec
+	 * does not allow to report that atomic commands are not supported by
+	 * a namespace, do not set any of the namespace specific atomic
+	 * properties. This will cause the initiator to rely on the atomic
+	 * properties reported by the controller. To do: discuss whether or
+	 * not the file backend should use direct I/O instead of buffered I/O.
+	 */
+}
+
 void nvmet_file_ns_disable(struct nvmet_ns *ns)
 {
 	if (ns->file) {
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index dc270944bb25..552af334fe1d 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -365,6 +365,8 @@ u16 nvmet_set_feat_async_event(struct nvmet_req *req, u32 mask);
 void nvmet_execute_async_event(struct nvmet_req *req);
 
 u16 nvmet_parse_connect_cmd(struct nvmet_req *req);
+void nvmet_bdev_set_limits(struct block_device *bdev, struct nvme_id_ns *id);
+void nvmet_file_set_limits(struct file *file, struct nvme_id_ns *id);
 u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req);
 u16 nvmet_file_parse_io_cmd(struct nvmet_req *req);
 u16 nvmet_parse_admin_cmd(struct nvmet_req *req);
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 3/3] nvme: Set physical block size and optimal I/O size according to NVMe 1.4
  2019-06-27 14:32 [PATCH v4 0/3] NVMe 1.4 Identify Namespace Support Bart Van Assche
  2019-06-27 14:32 ` [PATCH v4 1/3] nvme: Introduce NVMe 1.4 Identify Namespace fields in struct nvme_id_ns Bart Van Assche
  2019-06-27 14:32 ` [PATCH v4 2/3] nvmet: Export NVMe namespace attributes Bart Van Assche
@ 2019-06-27 14:32 ` Bart Van Assche
  2019-06-28  7:06   ` Christoph Hellwig
  2 siblings, 1 reply; 8+ messages in thread
From: Bart Van Assche @ 2019-06-27 14:32 UTC (permalink / raw)


>From the NVMe 1.4 spec:

NSFEAT bit 4 if set to 1: indicates that the fields NPWG, NPWA, NPDG, NPDA,
and NOWS are defined for this namespace and should be used by the host for
I/O optimization;
[ ... ]
Namespace Preferred Write Granularity (NPWG): This field indicates the
smallest recommended write granularity in logical blocks for this namespace.
This is a 0's based value. The size indicated should be less than or equal
to Maximum Data Transfer Size (MDTS) that is specified in units of minimum
memory page size. The value of this field may change if the namespace is
reformatted. The size should be a multiple of Namespace Preferred Write
Alignment (NPWA). Refer to section 8.25 for how this field is utilized to
improve performance and endurance.
[ ... ]
Each Write, Write Uncorrectable, or Write Zeroes commands should address a
multiple of Namespace Preferred Write Granularity (NPWG) (refer to Figure
245) and Stream Write Size (SWS) (refer to Figure 515) logical blocks (as
expressed in the NLB field), and the SLBA field of the command should be
aligned to Namespace Preferred Write Alignment (NPWA) (refer to Figure 245)
for best performance.

Cc: Keith Busch <kbusch at kernel.org>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Hannes Reinecke <hare at suse.com>
Signed-off-by: Bart Van Assche <bvanassche at acm.org>
---
 drivers/nvme/host/core.c | 30 ++++++++++++++++++++++++++++--
 drivers/nvme/host/nvme.h |  1 +
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b2dd4e391f5c..366492253c4e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1626,6 +1626,7 @@ static void nvme_update_disk_info(struct gendisk *disk,
 {
 	sector_t capacity = le64_to_cpu(id->nsze) << (ns->lba_shift - 9);
 	unsigned short bs = 1 << ns->lba_shift;
+	u32 atomic_bs, phys_bs, io_opt;
 
 	if (ns->lba_shift > PAGE_SHIFT) {
 		/* unsupported block size, set capacity to 0 later */
@@ -1634,9 +1635,33 @@ static void nvme_update_disk_info(struct gendisk *disk,
 	blk_mq_freeze_queue(disk->queue);
 	blk_integrity_unregister(disk);
 
+	/*
+	 * Bit 1 indicates whether NAWUPF is defined for this namespace
+	 * and whether it should be used instead of AWUPF. If NAWUPF == 0
+	 * then AWUPF must be used instead.
+	 */
+	if (id->nsfeat & (1 << 1) && id->nawupf)
+		atomic_bs = (1 + id->nawupf) * bs;
+	else
+		atomic_bs = (1 + ns->ctrl->subsys->awupf) * bs;
+	phys_bs = bs;
+	io_opt = bs;
+	if (id->nsfeat & (1 << 4)) {
+		/* NPWG = Namespace Preferred Write Granularity */
+		phys_bs *= 1 + le16_to_cpu(id->npwg);
+		/* NOWS = Namespace Optimal Write Size */
+		io_opt *= 1 + le16_to_cpu(id->nows);
+	}
+
 	blk_queue_logical_block_size(disk->queue, bs);
-	blk_queue_physical_block_size(disk->queue, bs);
-	blk_queue_io_min(disk->queue, bs);
+	/*
+	 * Linux filesystems assume writing a single physical block is
+	 * an atomic operation. Hence limit the physical block size to the
+	 * value of the Atomic Write Unit Power Fail parameter.
+	 */
+	blk_queue_physical_block_size(disk->queue, min(phys_bs, atomic_bs));
+	blk_queue_io_min(disk->queue, phys_bs);
+	blk_queue_io_opt(disk->queue, io_opt);
 
 	if (ns->ms && !ns->ext &&
 	    (ns->ctrl->ops->flags & NVME_F_METADATA_SUPPORTED))
@@ -2433,6 +2458,7 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 	memcpy(subsys->firmware_rev, id->fr, sizeof(subsys->firmware_rev));
 	subsys->vendor_id = le16_to_cpu(id->vid);
 	subsys->cmic = id->cmic;
+	subsys->awupf = le16_to_cpu(id->awupf);
 #ifdef CONFIG_NVME_MULTIPATH
 	subsys->iopolicy = NVME_IOPOLICY_NUMA;
 #endif
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index ea45d7d393ad..716a876119c8 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -283,6 +283,7 @@ struct nvme_subsystem {
 	char			firmware_rev[8];
 	u8			cmic;
 	u16			vendor_id;
+	u16			awupf;	/* 0's based awupf value. */
 	struct ida		ns_ida;
 #ifdef CONFIG_NVME_MULTIPATH
 	enum nvme_iopolicy	iopolicy;
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/3] nvmet: Export NVMe namespace attributes
  2019-06-27 14:32 ` [PATCH v4 2/3] nvmet: Export NVMe namespace attributes Bart Van Assche
@ 2019-06-27 17:11   ` Chaitanya Kulkarni
  2019-06-28  7:02   ` Christoph Hellwig
  1 sibling, 0 replies; 8+ messages in thread
From: Chaitanya Kulkarni @ 2019-06-27 17:11 UTC (permalink / raw)


Thanks for keeping the comment in the file-backend, very useful.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>


On 6/27/19 7:32 AM, Bart Van Assche wrote:
> Make the NVMe NAWUN, NAWUPF, NACWU, NPWG, NPWA, NPDG and NOWS attributes
> available to initator systems for the block backend.
> 
> Cc: Keith Busch <kbusch at kernel.org>
> Cc: Sagi Grimberg <sagi at grimberg.me>
> Cc: Martin K. Petersen <martin.petersen at oracle.com>
> Cc: Hannes Reinecke <hare at suse.com>
> Cc: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
> Signed-off-by: Bart Van Assche <bvanassche at acm.org>
> ---
>   drivers/nvme/target/admin-cmd.c   |  5 ++++
>   drivers/nvme/target/io-cmd-bdev.c | 45 +++++++++++++++++++++++++++++++
>   drivers/nvme/target/io-cmd-file.c | 12 +++++++++
>   drivers/nvme/target/nvmet.h       |  2 ++
>   4 files changed, 64 insertions(+)
> 
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index 9f72d515fc4b..2558ed333d29 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -442,6 +442,11 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
>   		break;
>           }
>   
> +	if (ns->file)
> +		nvmet_file_set_limits(ns->file, id);
> +	else if (ns->bdev)
> +		nvmet_bdev_set_limits(ns->bdev, id);
> +
>   	/*
>   	 * We just provide a single LBA format that matches what the
>   	 * underlying device reports.
> diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
> index 7a1cf6437a6a..6e57d0d4f4e9 100644
> --- a/drivers/nvme/target/io-cmd-bdev.c
> +++ b/drivers/nvme/target/io-cmd-bdev.c
> @@ -8,6 +8,51 @@
>   #include <linux/module.h>
>   #include "nvmet.h"
>   
> +/* Convert a 32-bit number to a 16-bit 0's based number */
> +static __le16 to0based(u32 a)
> +{
> +	return cpu_to_le16(max(1U, min(1U << 16, a)) - 1);
> +}
> +
> +void nvmet_bdev_set_limits(struct block_device *bdev, struct nvme_id_ns *id)
> +{
> +	const struct queue_limits *ql = &bdev_get_queue(bdev)->limits;
> +	/* Number of physical blocks per logical block. */
> +	const u32 ppl = ql->physical_block_size / ql->logical_block_size;
> +	/* Physical blocks per logical block, 0's based. */
> +	const __le16 ppl0b = to0based(ppl);
> +
> +	/*
> +	 * For NVMe 1.2 and later, bit 1 indicates that the fields NAWUN,
> +	 * NAWUPF, and NACWU are defined for this namespace and should be
> +	 * used by the host for this namespace instead of the AWUN, AWUPF,
> +	 * and ACWU fields in the Identify Controller data structure. If
> +	 * any of these fields are zero that means that the corresponding
> +	 * field from the identify controller data structure should be used.
> +	 */
> +	id->nsfeat |= 1 << 1;
> +	id->nawun = ppl0b;
> +	id->nawupf = ppl0b;
> +	id->nacwu = ppl0b;
> +
> +	/*
> +	 * For NVMe 1.4 and later, bit 4 indicates that the fields NPWG,
> +	 * NPWA, NPDG, NPDA, and NOWS are defined for this namespace and
> +	 * should be used by the host for I/O optimization.
> +	 */
> +	id->nsfeat |= 1 << 4;
> +	/* NPWG = Namespace Preferred Write Granularity. 0's based */
> +	id->npwg = ppl0b;
> +	/* NPWA = Namespace Preferred Write Alignment. 0's based */
> +	id->npwa = id->npwg;
> +	/* NPDG = Namespace Preferred Deallocate Granularity. 0's based */
> +	id->npdg = to0based(ql->discard_granularity / ql->logical_block_size);
> +	/* NPDG = Namespace Preferred Deallocate Alignment */
> +	id->npda = id->npdg;
> +	/* NOWS = Namespace Optimal Write Size */
> +	id->nows = to0based(ql->io_opt / ql->logical_block_size);
> +}
> +
>   int nvmet_bdev_ns_enable(struct nvmet_ns *ns)
>   {
>   	int ret;
> diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
> index 05453f5d1448..8e59c107ec38 100644
> --- a/drivers/nvme/target/io-cmd-file.c
> +++ b/drivers/nvme/target/io-cmd-file.c
> @@ -13,6 +13,18 @@
>   #define NVMET_MAX_MPOOL_BVEC		16
>   #define NVMET_MIN_MPOOL_OBJ		16
>   
> +void nvmet_file_set_limits(struct file *file, struct nvme_id_ns *id)
> +{
> +	/*
> +	 * Since the file backend uses buffered I/O and since the NVMe spec
> +	 * does not allow to report that atomic commands are not supported by
> +	 * a namespace, do not set any of the namespace specific atomic
> +	 * properties. This will cause the initiator to rely on the atomic
> +	 * properties reported by the controller. To do: discuss whether or
> +	 * not the file backend should use direct I/O instead of buffered I/O.
> +	 */
> +}
> +
>   void nvmet_file_ns_disable(struct nvmet_ns *ns)
>   {
>   	if (ns->file) {
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index dc270944bb25..552af334fe1d 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -365,6 +365,8 @@ u16 nvmet_set_feat_async_event(struct nvmet_req *req, u32 mask);
>   void nvmet_execute_async_event(struct nvmet_req *req);
>   
>   u16 nvmet_parse_connect_cmd(struct nvmet_req *req);
> +void nvmet_bdev_set_limits(struct block_device *bdev, struct nvme_id_ns *id);
> +void nvmet_file_set_limits(struct file *file, struct nvme_id_ns *id);
>   u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req);
>   u16 nvmet_file_parse_io_cmd(struct nvmet_req *req);
>   u16 nvmet_parse_admin_cmd(struct nvmet_req *req);
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/3] nvme: Introduce NVMe 1.4 Identify Namespace fields in struct nvme_id_ns
  2019-06-27 14:32 ` [PATCH v4 1/3] nvme: Introduce NVMe 1.4 Identify Namespace fields in struct nvme_id_ns Bart Van Assche
@ 2019-06-28  7:00   ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2019-06-28  7:00 UTC (permalink / raw)


On Thu, Jun 27, 2019@07:32:13AM -0700, Bart Van Assche wrote:
> Several new fields have been introduced in version 1.4 of the NVMe spec
> at offsets that were defined as reserved in version 1.3d of the NVMe
> spec. Update the definition of the nvme_id_ns data structure such that
> it is in sync with version 1.4 of the NVMe spec. This change preserves
> backwards compatibility.
> 
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
> Reviewed-by: Martin K. Petersen <martin.petersen at oracle.com>
> Reviewed-by: Keith Busch <kbusch at kernel.org>
> Cc: Sagi Grimberg <sagi at grimberg.me>
> Cc: Hannes Reinecke <hare at suse.com>
> Signed-off-by: Bart Van Assche <bvanassche at acm.org>

Looks good:

Reviewed-by: Christoph Hellwig <hch at lst.de>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 2/3] nvmet: Export NVMe namespace attributes
  2019-06-27 14:32 ` [PATCH v4 2/3] nvmet: Export NVMe namespace attributes Bart Van Assche
  2019-06-27 17:11   ` Chaitanya Kulkarni
@ 2019-06-28  7:02   ` Christoph Hellwig
  1 sibling, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2019-06-28  7:02 UTC (permalink / raw)


On Thu, Jun 27, 2019@07:32:14AM -0700, Bart Van Assche wrote:
> +/* Convert a 32-bit number to a 16-bit 0's based number */
> +static __le16 to0based(u32 a)
> +{
> +	return cpu_to_le16(max(1U, min(1U << 16, a)) - 1);
> +}

Maybe lift this to drivers/nvme/target/nvmet.h as an inline as it
might be generally useful for more thing in the future?

> +	/*
> +	 * For NVMe 1.4 and later, bit 4 indicates that the fields NPWG,

I'd drop the NVMe 1.4 as it is a bit pointless.  Also in NVMe you
can generally implement the TPs on older compliance levels (which is in
fact what we do here anyway).

> +void nvmet_file_set_limits(struct file *file, struct nvme_id_ns *id)
> +{
> +	/*
> +	 * Since the file backend uses buffered I/O and since the NVMe spec
> +	 * does not allow to report that atomic commands are not supported by
> +	 * a namespace, do not set any of the namespace specific atomic
> +	 * properties. This will cause the initiator to rely on the atomic
> +	 * properties reported by the controller. To do: discuss whether or
> +	 * not the file backend should use direct I/O instead of buffered I/O.
> +	 */
> +}

This function doesn't seem overly helpful to me.  Can we just skip
that whole function and the comment?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 3/3] nvme: Set physical block size and optimal I/O size according to NVMe 1.4
  2019-06-27 14:32 ` [PATCH v4 3/3] nvme: Set physical block size and optimal I/O size according to NVMe 1.4 Bart Van Assche
@ 2019-06-28  7:06   ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2019-06-28  7:06 UTC (permalink / raw)


Hi Bart,

> +	/*
> +	 * Bit 1 indicates whether NAWUPF is defined for this namespace
> +	 * and whether it should be used instead of AWUPF. If NAWUPF == 0
> +	 * then AWUPF must be used instead.
> +	 */
> +	if (id->nsfeat & (1 << 1) && id->nawupf)
> +		atomic_bs = (1 + id->nawupf) * bs;

I think we also need to take the NABO value into account here.  Probably
just in that we only set a physical sector size = LBA size, as dealing
with the offsets would be a royal pain (and I've never actually seen
an NVMe device setting the offset either).

Otherwise this looks great to me.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-06-28  7:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-27 14:32 [PATCH v4 0/3] NVMe 1.4 Identify Namespace Support Bart Van Assche
2019-06-27 14:32 ` [PATCH v4 1/3] nvme: Introduce NVMe 1.4 Identify Namespace fields in struct nvme_id_ns Bart Van Assche
2019-06-28  7:00   ` Christoph Hellwig
2019-06-27 14:32 ` [PATCH v4 2/3] nvmet: Export NVMe namespace attributes Bart Van Assche
2019-06-27 17:11   ` Chaitanya Kulkarni
2019-06-28  7:02   ` Christoph Hellwig
2019-06-27 14:32 ` [PATCH v4 3/3] nvme: Set physical block size and optimal I/O size according to NVMe 1.4 Bart Van Assche
2019-06-28  7:06   ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.