All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvme: disable namespaces with unsupported metadata
@ 2021-11-23 17:45 Keith Busch
  2021-11-23 19:59 ` Sagi Grimberg
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Keith Busch @ 2021-11-23 17:45 UTC (permalink / raw)
  To: linux-nvme, hch; +Cc: Keith Busch, Max Gurtovoy, Sagi Grimberg

The only fabrics target that supports metadata is RDMA and usable only
if it is 8B per block, and formatted for protection information. If an
rdma target were to export a namespace with a different format (ex:
4k+64B), the driver will not be able to submit a valid command for that
format.

Suppress exporting these namespaces as read/writeable to the block
stack.

Cc: Max Gurtovoy <mgurtovoy@nvidia.com>
Cc: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/core.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 4b5de8f5435a..3afbf73639c3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1740,7 +1740,16 @@ static int nvme_configure_metadata(struct nvme_ns *ns, struct nvme_id_ns *id)
 		 */
 		if (WARN_ON_ONCE(!(id->flbas & NVME_NS_FLBAS_META_EXT)))
 			return -EINVAL;
-		if (ctrl->max_integrity_segments)
+
+		/*
+		 * Currently rdma is the only fabrics nvme host that supports
+		 * metadata integrity segments, but will not use the metadata
+		 * payload if it doesn't satisfy 'nvme_ns_has_pi()'. Since the
+		 * initiator is not able to form requests that satisify the
+		 * format, do not support the namespace if the metadata does
+		 * not satisfy that requirement.
+		 */
+		if (ctrl->max_integrity_segments && nvme_ns_has_pi(ns))
 			ns->features |=
 				(NVME_NS_METADATA_SUPPORTED | NVME_NS_EXT_LBAS);
 	} else {
-- 
2.25.4



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: disable namespaces with unsupported metadata
  2021-11-23 17:45 [PATCH] nvme: disable namespaces with unsupported metadata Keith Busch
@ 2021-11-23 19:59 ` Sagi Grimberg
  2021-11-23 21:10 ` Keith Busch
  2021-11-24 18:47 ` Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2021-11-23 19:59 UTC (permalink / raw)
  To: Keith Busch, linux-nvme, hch; +Cc: Max Gurtovoy

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: disable namespaces with unsupported metadata
  2021-11-23 17:45 [PATCH] nvme: disable namespaces with unsupported metadata Keith Busch
  2021-11-23 19:59 ` Sagi Grimberg
@ 2021-11-23 21:10 ` Keith Busch
  2021-11-24  9:29   ` Max Gurtovoy
  2021-11-24 18:47 ` Christoph Hellwig
  2 siblings, 1 reply; 6+ messages in thread
From: Keith Busch @ 2021-11-23 21:10 UTC (permalink / raw)
  To: linux-nvme, hch; +Cc: Max Gurtovoy, Sagi Grimberg

On Tue, Nov 23, 2021 at 09:45:54AM -0800, Keith Busch wrote:
> The only fabrics target that supports metadata is RDMA and usable only

Sorry, the above should have said:

  "The only fabrics host driver that currently supports metadata is ..."

I believe the spec allows all transports to support any arbitrary
metadata format. The drivers just currently aren't able to, and we can
update this path as they become more capable.

> if it is 8B per block, and formatted for protection information. If an
> rdma target were to export a namespace with a different format (ex:
> 4k+64B), the driver will not be able to submit a valid command for that
> format.
> 
> Suppress exporting these namespaces as read/writeable to the block
> stack.


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: disable namespaces with unsupported metadata
  2021-11-23 21:10 ` Keith Busch
@ 2021-11-24  9:29   ` Max Gurtovoy
  0 siblings, 0 replies; 6+ messages in thread
From: Max Gurtovoy @ 2021-11-24  9:29 UTC (permalink / raw)
  To: Keith Busch, linux-nvme, hch; +Cc: Sagi Grimberg


On 11/23/2021 11:10 PM, Keith Busch wrote:
> On Tue, Nov 23, 2021 at 09:45:54AM -0800, Keith Busch wrote:
>> The only fabrics target that supports metadata is RDMA and usable only
> Sorry, the above should have said:
>
>    "The only fabrics host driver that currently supports metadata is ..."
>
> I believe the spec allows all transports to support any arbitrary
> metadata format. The drivers just currently aren't able to, and we can
> update this path as they become more capable.
>
>> if it is 8B per block, and formatted for protection information. If an
>> rdma target were to export a namespace with a different format (ex:
>> 4k+64B), the driver will not be able to submit a valid command for that
>> format.
>>
>> Suppress exporting these namespaces as read/writeable to the block
>> stack.

Looks good,

Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>



^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: disable namespaces with unsupported metadata
  2021-11-23 17:45 [PATCH] nvme: disable namespaces with unsupported metadata Keith Busch
  2021-11-23 19:59 ` Sagi Grimberg
  2021-11-23 21:10 ` Keith Busch
@ 2021-11-24 18:47 ` Christoph Hellwig
  2021-11-24 19:57   ` Keith Busch
  2 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2021-11-24 18:47 UTC (permalink / raw)
  To: Keith Busch; +Cc: linux-nvme, hch, Max Gurtovoy, Sagi Grimberg

On Tue, Nov 23, 2021 at 09:45:54AM -0800, Keith Busch wrote:
> The only fabrics target that supports metadata is RDMA and usable only
> if it is 8B per block, and formatted for protection information. If an
> rdma target were to export a namespace with a different format (ex:
> 4k+64B), the driver will not be able to submit a valid command for that
> format.
> 
> Suppress exporting these namespaces as read/writeable to the block
> stack.

Where do we disable the namepsace?  We only do not set the metadata
supported flag here, don't we?

> +		/*
> +		 * Currently rdma is the only fabrics nvme host that supports
> +		 * metadata integrity segments, but will not use the metadata
> +		 * payload if it doesn't satisfy 'nvme_ns_has_pi()'.

s/fabric nvme host/transport/

> Since the
> +		 * initiator is not able to form requests that satisify the
> +		 * format, do not support the namespace if the metadata does
> +		 * not satisfy that requirement.
> +		 */

I can't really parse this sentence.  Also initiator doesn't make much
sense in a NVMe context.


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: disable namespaces with unsupported metadata
  2021-11-24 18:47 ` Christoph Hellwig
@ 2021-11-24 19:57   ` Keith Busch
  0 siblings, 0 replies; 6+ messages in thread
From: Keith Busch @ 2021-11-24 19:57 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, Max Gurtovoy, Sagi Grimberg

On Wed, Nov 24, 2021 at 07:47:57PM +0100, Christoph Hellwig wrote:
> On Tue, Nov 23, 2021 at 09:45:54AM -0800, Keith Busch wrote:
> > The only fabrics target that supports metadata is RDMA and usable only
> > if it is 8B per block, and formatted for protection information. If an
> > rdma target were to export a namespace with a different format (ex:
> > 4k+64B), the driver will not be able to submit a valid command for that
> > format.
> > 
> > Suppress exporting these namespaces as read/writeable to the block
> > stack.
> 
> Where do we disable the namepsace?  We only do not set the metadata
> supported flag here, don't we?

Without the flag set, a namespace with metadata gets to be 0 capacity.
That will make it unusable through the block stack r/w path, at least.
 
> > +		/*
> > +		 * Currently rdma is the only fabrics nvme host that supports
> > +		 * metadata integrity segments, but will not use the metadata
> > +		 * payload if it doesn't satisfy 'nvme_ns_has_pi()'.
> 
> s/fabric nvme host/transport/
> 
> > Since the
> > +		 * initiator is not able to form requests that satisify the
> > +		 * format, do not support the namespace if the metadata does
> > +		 * not satisfy that requirement.
> > +		 */
> 
> I can't really parse this sentence.  Also initiator doesn't make much
> sense in a NVMe context.

I'll reword this.


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-24 19:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-23 17:45 [PATCH] nvme: disable namespaces with unsupported metadata Keith Busch
2021-11-23 19:59 ` Sagi Grimberg
2021-11-23 21:10 ` Keith Busch
2021-11-24  9:29   ` Max Gurtovoy
2021-11-24 18:47 ` Christoph Hellwig
2021-11-24 19:57   ` Keith Busch

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.