All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] nvme: enable ro namespace for ZNS without append
@ 2020-11-10  9:39 ` Javier González
  0 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-10  9:39 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-block, hch, kbusch, sagi, axboe, joshi.k, k.jensen,
	Niklas.Cassel, Javier González

Allow ZNS NVMe SSDs to present a read-only namespace when append is not
supported, instead of rejecting the namespace directly.

This allows (i) the namespace to be used in read-only mode, which is not
a problem as the append command only affects the write path, and (ii) to
use standard management tools such as nvme-cli to choose a different
format or firmware slot that is compatible with the Linux zoned block
device.

This patch includes comments from Christoph, Niklas and Keith that
applied to a different approach setting capacity to 0
  https://www.spinics.net/lists/linux-block/msg60747.html

The reminder of the original patch will be submitted separately.

Changes since V1:
  - Change logic to use NVME_NS_ATTR_RO (from Christoph)
  - Set max_zone_append egen in RO. This allows the device to be properly
    revalidated and enables user-space tools such as blkzone to be used when
    interacting with this zoned device.

Signed-off-by: Javier González <javier.gonz@samsung.com>
---
 drivers/nvme/host/core.c | 3 ++-
 drivers/nvme/host/nvme.h | 1 +
 drivers/nvme/host/zns.c  | 4 ++--
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 40ca71b29bb9..4fcaec0b330b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2058,7 +2058,8 @@ static void nvme_update_disk_info(struct gendisk *disk,
 	nvme_config_discard(disk, ns);
 	nvme_config_write_zeroes(disk, ns);
 
-	if (id->nsattr & NVME_NS_ATTR_RO)
+	if (id->nsattr & NVME_NS_ATTR_RO ||
+			test_bit(NVME_NS_FORCE_RO, &ns->flags))
 		set_disk_ro(disk, true);
 	else
 		set_disk_ro(disk, false);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index bc330bf0d3bd..3a985e98ca27 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -448,6 +448,7 @@ struct nvme_ns {
 #define NVME_NS_REMOVING	0
 #define NVME_NS_DEAD     	1
 #define NVME_NS_ANA_PENDING	2
+#define NVME_NS_FORCE_RO	3
 
 	struct nvme_fault_inject fault_inject;
 
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 67e87e9f306f..8f3632c4db05 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -57,10 +57,10 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
 	/* Driver requires zone append support */
 	if (!(le32_to_cpu(log->iocs[nvme_cmd_zone_append]) &
 			NVME_CMD_EFFECTS_CSUPP)) {
+		set_bit(NVME_NS_FORCE_RO, &ns->flags);
 		dev_warn(ns->ctrl->device,
-			"append not supported for zoned namespace:%d\n",
+			"append not supported for zoned namespace:%d. Forcing to read-only mode\n",
 			ns->head->ns_id);
-		return -EINVAL;
 	}
 
 	/* Lazily query controller append limit for the first zoned namespace */
-- 
2.17.1


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

* [PATCH V2] nvme: enable ro namespace for ZNS without append
@ 2020-11-10  9:39 ` Javier González
  0 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-10  9:39 UTC (permalink / raw)
  To: linux-nvme
  Cc: axboe, Niklas.Cassel, sagi, joshi.k, k.jensen, linux-block,
	kbusch, Javier González, hch

Allow ZNS NVMe SSDs to present a read-only namespace when append is not
supported, instead of rejecting the namespace directly.

This allows (i) the namespace to be used in read-only mode, which is not
a problem as the append command only affects the write path, and (ii) to
use standard management tools such as nvme-cli to choose a different
format or firmware slot that is compatible with the Linux zoned block
device.

This patch includes comments from Christoph, Niklas and Keith that
applied to a different approach setting capacity to 0
  https://www.spinics.net/lists/linux-block/msg60747.html

The reminder of the original patch will be submitted separately.

Changes since V1:
  - Change logic to use NVME_NS_ATTR_RO (from Christoph)
  - Set max_zone_append egen in RO. This allows the device to be properly
    revalidated and enables user-space tools such as blkzone to be used when
    interacting with this zoned device.

Signed-off-by: Javier González <javier.gonz@samsung.com>
---
 drivers/nvme/host/core.c | 3 ++-
 drivers/nvme/host/nvme.h | 1 +
 drivers/nvme/host/zns.c  | 4 ++--
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 40ca71b29bb9..4fcaec0b330b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2058,7 +2058,8 @@ static void nvme_update_disk_info(struct gendisk *disk,
 	nvme_config_discard(disk, ns);
 	nvme_config_write_zeroes(disk, ns);
 
-	if (id->nsattr & NVME_NS_ATTR_RO)
+	if (id->nsattr & NVME_NS_ATTR_RO ||
+			test_bit(NVME_NS_FORCE_RO, &ns->flags))
 		set_disk_ro(disk, true);
 	else
 		set_disk_ro(disk, false);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index bc330bf0d3bd..3a985e98ca27 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -448,6 +448,7 @@ struct nvme_ns {
 #define NVME_NS_REMOVING	0
 #define NVME_NS_DEAD     	1
 #define NVME_NS_ANA_PENDING	2
+#define NVME_NS_FORCE_RO	3
 
 	struct nvme_fault_inject fault_inject;
 
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 67e87e9f306f..8f3632c4db05 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -57,10 +57,10 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
 	/* Driver requires zone append support */
 	if (!(le32_to_cpu(log->iocs[nvme_cmd_zone_append]) &
 			NVME_CMD_EFFECTS_CSUPP)) {
+		set_bit(NVME_NS_FORCE_RO, &ns->flags);
 		dev_warn(ns->ctrl->device,
-			"append not supported for zoned namespace:%d\n",
+			"append not supported for zoned namespace:%d. Forcing to read-only mode\n",
 			ns->head->ns_id);
-		return -EINVAL;
 	}
 
 	/* Lazily query controller append limit for the first zoned namespace */
-- 
2.17.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: [PATCH V2] nvme: enable ro namespace for ZNS without append
  2020-11-10  9:39 ` Javier González
@ 2020-11-10  9:43   ` Christoph Hellwig
  -1 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2020-11-10  9:43 UTC (permalink / raw)
  To: Javier González
  Cc: linux-nvme, linux-block, hch, kbusch, sagi, axboe, joshi.k,
	k.jensen, Niklas.Cassel, Javier González

> -	if (id->nsattr & NVME_NS_ATTR_RO)
> +	if (id->nsattr & NVME_NS_ATTR_RO ||
> +			test_bit(NVME_NS_FORCE_RO, &ns->flags))
>  		set_disk_ro(disk, true);
>  	else
>  		set_disk_ro(disk, false);

This has a very minor conflict with the patch from Sagi to remove the
else side of the clause.  I'll merge your patch and will fix unless
someone else objects to the approach.

Note that as discussed we really need a hard read-only settings
instead of set_disk_ro for both NVME_NS_ATTR_RO and the zns with missing
features case, but I'll look into that once I've got a few other things
off my plate.

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

* Re: [PATCH V2] nvme: enable ro namespace for ZNS without append
@ 2020-11-10  9:43   ` Christoph Hellwig
  0 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2020-11-10  9:43 UTC (permalink / raw)
  To: Javier González
  Cc: axboe, Niklas.Cassel, sagi, joshi.k, k.jensen, linux-nvme,
	linux-block, kbusch, Javier González, hch

> -	if (id->nsattr & NVME_NS_ATTR_RO)
> +	if (id->nsattr & NVME_NS_ATTR_RO ||
> +			test_bit(NVME_NS_FORCE_RO, &ns->flags))
>  		set_disk_ro(disk, true);
>  	else
>  		set_disk_ro(disk, false);

This has a very minor conflict with the patch from Sagi to remove the
else side of the clause.  I'll merge your patch and will fix unless
someone else objects to the approach.

Note that as discussed we really need a hard read-only settings
instead of set_disk_ro for both NVME_NS_ATTR_RO and the zns with missing
features case, but I'll look into that once I've got a few other things
off my plate.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: nvme: enable ro namespace for ZNS without append
  2020-11-10  9:43   ` Christoph Hellwig
@ 2020-11-10 10:15     ` Javier González
  -1 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-10 10:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-nvme, linux-block, kbusch, sagi, axboe, joshi.k, k.jensen,
	Niklas.Cassel

On 10.11.2020 10:43, Christoph Hellwig wrote:
>> -	if (id->nsattr & NVME_NS_ATTR_RO)
>> +	if (id->nsattr & NVME_NS_ATTR_RO ||
>> +			test_bit(NVME_NS_FORCE_RO, &ns->flags))
>>  		set_disk_ro(disk, true);
>>  	else
>>  		set_disk_ro(disk, false);
>
>This has a very minor conflict with the patch from Sagi to remove the
>else side of the clause.  I'll merge your patch and will fix unless
>someone else objects to the approach.

Sounds good.
>
>Note that as discussed we really need a hard read-only settings
>instead of set_disk_ro for both NVME_NS_ATTR_RO and the zns with missing
>features case, but I'll look into that once I've got a few other things
>off my plate.

Thanks! Please, let me know if there is anything we can help with. I'm
looking into the char device now.

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

* Re: nvme: enable ro namespace for ZNS without append
@ 2020-11-10 10:15     ` Javier González
  0 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-10 10:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: axboe, Niklas.Cassel, sagi, joshi.k, k.jensen, linux-nvme,
	linux-block, kbusch

On 10.11.2020 10:43, Christoph Hellwig wrote:
>> -	if (id->nsattr & NVME_NS_ATTR_RO)
>> +	if (id->nsattr & NVME_NS_ATTR_RO ||
>> +			test_bit(NVME_NS_FORCE_RO, &ns->flags))
>>  		set_disk_ro(disk, true);
>>  	else
>>  		set_disk_ro(disk, false);
>
>This has a very minor conflict with the patch from Sagi to remove the
>else side of the clause.  I'll merge your patch and will fix unless
>someone else objects to the approach.

Sounds good.
>
>Note that as discussed we really need a hard read-only settings
>instead of set_disk_ro for both NVME_NS_ATTR_RO and the zns with missing
>features case, but I'll look into that once I've got a few other things
>off my plate.

Thanks! Please, let me know if there is anything we can help with. I'm
looking into the char device now.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: [PATCH V2] nvme: enable ro namespace for ZNS without append
  2020-11-10  9:43   ` Christoph Hellwig
@ 2020-11-10 11:25     ` Niklas Cassel
  -1 siblings, 0 replies; 16+ messages in thread
From: Niklas Cassel @ 2020-11-10 11:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Javier González, linux-nvme, linux-block, kbusch, sagi,
	axboe, joshi.k, k.jensen, Javier González

On Tue, Nov 10, 2020 at 10:43:54AM +0100, Christoph Hellwig wrote:
> > -	if (id->nsattr & NVME_NS_ATTR_RO)
> > +	if (id->nsattr & NVME_NS_ATTR_RO ||
> > +			test_bit(NVME_NS_FORCE_RO, &ns->flags))

Indentation for the test_bit() looks off.
I assume that Christoph can fixup that when applying.

$ ./scripts/checkpatch.pl --strict ~/javier.patch
CHECK: Alignment should match open parenthesis
#280: FILE: drivers/nvme/host/core.c:2062:
+       if (id->nsattr & NVME_NS_ATTR_RO ||
+                       test_bit(NVME_NS_FORCE_RO, &ns->flags))


For the record:

WARNING: From:/Signed-off-by: email address mismatch: 'From: "Javier González" <javier@javigon.com>' != 'Signed-off-by: Javier González <javier.gonz@samsung.com>'

If you want to use a SoB that is different from the email address which
you are sending from, then according to the The canonical patch format:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format

"""
The from line must be the very first line in the message body, and has the form:

    From: Patch Author <author@example.com>

The from line specifies who will be credited as the author of the patch in the permanent changelog.
If the from line is missing, then the From: line from the email header will be used to determine
the patch author in the changelog.

Note, the From: tag is optional when the From: author is also the person (and email) listed in the
From: line of the email header.
"""


That way, when the maintainers use git am, it will pick the author
from the "From:" in the message body, rather than from the email header.

Otherwise the Author: field in the git log will be different from your SoB.

There are several ways you can fix this, either by using the correct email
when you do the commit in the first place, then git format-patch will add
From: automatically, or by using the git config sendemail.from, or --from
option to git-send-email.


Kind regards,
Niklas

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

* Re: [PATCH V2] nvme: enable ro namespace for ZNS without append
@ 2020-11-10 11:25     ` Niklas Cassel
  0 siblings, 0 replies; 16+ messages in thread
From: Niklas Cassel @ 2020-11-10 11:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: axboe, Javier González, sagi, joshi.k, k.jensen, linux-nvme,
	linux-block, kbusch, Javier González

On Tue, Nov 10, 2020 at 10:43:54AM +0100, Christoph Hellwig wrote:
> > -	if (id->nsattr & NVME_NS_ATTR_RO)
> > +	if (id->nsattr & NVME_NS_ATTR_RO ||
> > +			test_bit(NVME_NS_FORCE_RO, &ns->flags))

Indentation for the test_bit() looks off.
I assume that Christoph can fixup that when applying.

$ ./scripts/checkpatch.pl --strict ~/javier.patch
CHECK: Alignment should match open parenthesis
#280: FILE: drivers/nvme/host/core.c:2062:
+       if (id->nsattr & NVME_NS_ATTR_RO ||
+                       test_bit(NVME_NS_FORCE_RO, &ns->flags))


For the record:

WARNING: From:/Signed-off-by: email address mismatch: 'From: "Javier González" <javier@javigon.com>' != 'Signed-off-by: Javier González <javier.gonz@samsung.com>'

If you want to use a SoB that is different from the email address which
you are sending from, then according to the The canonical patch format:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format

"""
The from line must be the very first line in the message body, and has the form:

    From: Patch Author <author@example.com>

The from line specifies who will be credited as the author of the patch in the permanent changelog.
If the from line is missing, then the From: line from the email header will be used to determine
the patch author in the changelog.

Note, the From: tag is optional when the From: author is also the person (and email) listed in the
From: line of the email header.
"""


That way, when the maintainers use git am, it will pick the author
from the "From:" in the message body, rather than from the email header.

Otherwise the Author: field in the git log will be different from your SoB.

There are several ways you can fix this, either by using the correct email
when you do the commit in the first place, then git format-patch will add
From: automatically, or by using the git config sendemail.from, or --from
option to git-send-email.


Kind regards,
Niklas
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: [PATCH V2] nvme: enable ro namespace for ZNS without append
  2020-11-10  9:39 ` Javier González
@ 2020-11-10 14:16   ` Keith Busch
  -1 siblings, 0 replies; 16+ messages in thread
From: Keith Busch @ 2020-11-10 14:16 UTC (permalink / raw)
  To: Javier González
  Cc: linux-nvme, linux-block, hch, sagi, axboe, joshi.k, k.jensen,
	Niklas.Cassel, Javier González

On Tue, Nov 10, 2020 at 10:39:38AM +0100, Javier González wrote:
>  	if (!(le32_to_cpu(log->iocs[nvme_cmd_zone_append]) &
>  			NVME_CMD_EFFECTS_CSUPP)) {
> +		set_bit(NVME_NS_FORCE_RO, &ns->flags);
>  		dev_warn(ns->ctrl->device,
> -			"append not supported for zoned namespace:%d\n",
> +			"append not supported for zoned namespace:%d. Forcing to read-only mode\n",
>  			ns->head->ns_id);
> -		return -EINVAL;
>  	}

In the unlikely event that a f/w upgrade adds append support, do we want
to bother clearing this flag? If so, we would need to refresh the
command effects log page.

If not, you'd have to rebind the driver to make it writable. I don't see
that as being a big deal, so I think the patch is probably fine as-is.

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

* Re: [PATCH V2] nvme: enable ro namespace for ZNS without append
@ 2020-11-10 14:16   ` Keith Busch
  0 siblings, 0 replies; 16+ messages in thread
From: Keith Busch @ 2020-11-10 14:16 UTC (permalink / raw)
  To: Javier González
  Cc: axboe, Niklas.Cassel, sagi, joshi.k, k.jensen, linux-nvme,
	linux-block, Javier González, hch

On Tue, Nov 10, 2020 at 10:39:38AM +0100, Javier González wrote:
>  	if (!(le32_to_cpu(log->iocs[nvme_cmd_zone_append]) &
>  			NVME_CMD_EFFECTS_CSUPP)) {
> +		set_bit(NVME_NS_FORCE_RO, &ns->flags);
>  		dev_warn(ns->ctrl->device,
> -			"append not supported for zoned namespace:%d\n",
> +			"append not supported for zoned namespace:%d. Forcing to read-only mode\n",
>  			ns->head->ns_id);
> -		return -EINVAL;
>  	}

In the unlikely event that a f/w upgrade adds append support, do we want
to bother clearing this flag? If so, we would need to refresh the
command effects log page.

If not, you'd have to rebind the driver to make it writable. I don't see
that as being a big deal, so I think the patch is probably fine as-is.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: nvme: enable ro namespace for ZNS without append
  2020-11-10 14:16   ` Keith Busch
@ 2020-11-10 15:53     ` Javier González
  -1 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-10 15:53 UTC (permalink / raw)
  To: Keith Busch
  Cc: linux-nvme, linux-block, hch, sagi, axboe, joshi.k, k.jensen,
	Niklas.Cassel

On 10.11.2020 06:16, Keith Busch wrote:
>On Tue, Nov 10, 2020 at 10:39:38AM +0100, Javier González wrote:
>>  	if (!(le32_to_cpu(log->iocs[nvme_cmd_zone_append]) &
>>  			NVME_CMD_EFFECTS_CSUPP)) {
>> +		set_bit(NVME_NS_FORCE_RO, &ns->flags);
>>  		dev_warn(ns->ctrl->device,
>> -			"append not supported for zoned namespace:%d\n",
>> +			"append not supported for zoned namespace:%d. Forcing to read-only mode\n",
>>  			ns->head->ns_id);
>> -		return -EINVAL;
>>  	}
>
>In the unlikely event that a f/w upgrade adds append support, do we want
>to bother clearing this flag? If so, we would need to refresh the
>command effects log page.

Good point. Also useful when selecting a new FW slot. Would it make
sense to move the check to the revalidate path and eventually clear the
flag?

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

* Re: nvme: enable ro namespace for ZNS without append
@ 2020-11-10 15:53     ` Javier González
  0 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-10 15:53 UTC (permalink / raw)
  To: Keith Busch
  Cc: axboe, Niklas.Cassel, sagi, joshi.k, k.jensen, linux-nvme,
	linux-block, hch

On 10.11.2020 06:16, Keith Busch wrote:
>On Tue, Nov 10, 2020 at 10:39:38AM +0100, Javier González wrote:
>>  	if (!(le32_to_cpu(log->iocs[nvme_cmd_zone_append]) &
>>  			NVME_CMD_EFFECTS_CSUPP)) {
>> +		set_bit(NVME_NS_FORCE_RO, &ns->flags);
>>  		dev_warn(ns->ctrl->device,
>> -			"append not supported for zoned namespace:%d\n",
>> +			"append not supported for zoned namespace:%d. Forcing to read-only mode\n",
>>  			ns->head->ns_id);
>> -		return -EINVAL;
>>  	}
>
>In the unlikely event that a f/w upgrade adds append support, do we want
>to bother clearing this flag? If so, we would need to refresh the
>command effects log page.

Good point. Also useful when selecting a new FW slot. Would it make
sense to move the check to the revalidate path and eventually clear the
flag?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: nvme: enable ro namespace for ZNS without append
  2020-11-10 11:25     ` Niklas Cassel
@ 2020-11-10 19:55       ` Javier González
  -1 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-10 19:55 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Christoph Hellwig, linux-nvme, linux-block, kbusch, sagi, axboe,
	joshi.k, k.jensen

On 10.11.2020 11:25, Niklas Cassel wrote:
>On Tue, Nov 10, 2020 at 10:43:54AM +0100, Christoph Hellwig wrote:
>> > -	if (id->nsattr & NVME_NS_ATTR_RO)
>> > +	if (id->nsattr & NVME_NS_ATTR_RO ||
>> > +			test_bit(NVME_NS_FORCE_RO, &ns->flags))
>
>Indentation for the test_bit() looks off.
>I assume that Christoph can fixup that when applying.
>
>$ ./scripts/checkpatch.pl --strict ~/javier.patch
>CHECK: Alignment should match open parenthesis
>#280: FILE: drivers/nvme/host/core.c:2062:
>+       if (id->nsattr & NVME_NS_ATTR_RO ||
>+                       test_bit(NVME_NS_FORCE_RO, &ns->flags))
>
>
>For the record:
>
>WARNING: From:/Signed-off-by: email address mismatch: 'From: "Javier González" <javier@javigon.com>' != 'Signed-off-by: Javier González <javier.gonz@samsung.com>'
>
>If you want to use a SoB that is different from the email address which
>you are sending from, then according to the The canonical patch format:
>
>https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
>
>"""
>The from line must be the very first line in the message body, and has the form:
>
>    From: Patch Author <author@example.com>
>
>The from line specifies who will be credited as the author of the patch in the permanent changelog.
>If the from line is missing, then the From: line from the email header will be used to determine
>the patch author in the changelog.
>
>Note, the From: tag is optional when the From: author is also the person (and email) listed in the
>From: line of the email header.
>"""
>
>
>That way, when the maintainers use git am, it will pick the author
>from the "From:" in the message body, rather than from the email header.
>
>Otherwise the Author: field in the git log will be different from your SoB.
>
>There are several ways you can fix this, either by using the correct email
>when you do the commit in the first place, then git format-patch will add
>From: automatically, or by using the git config sendemail.from, or --from
>option to git-send-email.
>

Thanks for taking the time Niklas. I have done this intentionally for
quite some time - I use javier@javigon.com to submit and commit and then
sign-off with my current employer.

If this presents an inconvenience, I don't mind changing the committer
to my current Samsung email.

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

* Re: nvme: enable ro namespace for ZNS without append
@ 2020-11-10 19:55       ` Javier González
  0 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-10 19:55 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: axboe, sagi, joshi.k, k.jensen, linux-nvme, linux-block, kbusch,
	Christoph Hellwig

On 10.11.2020 11:25, Niklas Cassel wrote:
>On Tue, Nov 10, 2020 at 10:43:54AM +0100, Christoph Hellwig wrote:
>> > -	if (id->nsattr & NVME_NS_ATTR_RO)
>> > +	if (id->nsattr & NVME_NS_ATTR_RO ||
>> > +			test_bit(NVME_NS_FORCE_RO, &ns->flags))
>
>Indentation for the test_bit() looks off.
>I assume that Christoph can fixup that when applying.
>
>$ ./scripts/checkpatch.pl --strict ~/javier.patch
>CHECK: Alignment should match open parenthesis
>#280: FILE: drivers/nvme/host/core.c:2062:
>+       if (id->nsattr & NVME_NS_ATTR_RO ||
>+                       test_bit(NVME_NS_FORCE_RO, &ns->flags))
>
>
>For the record:
>
>WARNING: From:/Signed-off-by: email address mismatch: 'From: "Javier González" <javier@javigon.com>' != 'Signed-off-by: Javier González <javier.gonz@samsung.com>'
>
>If you want to use a SoB that is different from the email address which
>you are sending from, then according to the The canonical patch format:
>
>https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
>
>"""
>The from line must be the very first line in the message body, and has the form:
>
>    From: Patch Author <author@example.com>
>
>The from line specifies who will be credited as the author of the patch in the permanent changelog.
>If the from line is missing, then the From: line from the email header will be used to determine
>the patch author in the changelog.
>
>Note, the From: tag is optional when the From: author is also the person (and email) listed in the
>From: line of the email header.
>"""
>
>
>That way, when the maintainers use git am, it will pick the author
>from the "From:" in the message body, rather than from the email header.
>
>Otherwise the Author: field in the git log will be different from your SoB.
>
>There are several ways you can fix this, either by using the correct email
>when you do the commit in the first place, then git format-patch will add
>From: automatically, or by using the git config sendemail.from, or --from
>option to git-send-email.
>

Thanks for taking the time Niklas. I have done this intentionally for
quite some time - I use javier@javigon.com to submit and commit and then
sign-off with my current employer.

If this presents an inconvenience, I don't mind changing the committer
to my current Samsung email.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

* Re: nvme: enable ro namespace for ZNS without append
  2020-11-11  8:36       ` Sagi Grimberg
@ 2020-11-11  9:21           ` Javier González
  0 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-11  9:21 UTC (permalink / raw)
  To: Sagi Grimberg
  Cc: Christoph Hellwig, Keith Busch, linux-nvme, linux-block, axboe,
	joshi.k, k.jensen, Niklas.Cassel

On 11.11.2020 00:36, Sagi Grimberg wrote:
>
>>>>>-	if (id->nsattr & NVME_NS_ATTR_RO)
>>>>>+	if (id->nsattr & NVME_NS_ATTR_RO || test_bit(NVME_NS_FORCE_RO, &ns->flags))
>>>>>   		set_disk_ro(disk, true);
>>>>
>>>>If the FORCE_RO flag is set, the disk is set to read-only. If that flag
>>>>is later cleared, nothing clears the disk's read-only setting.
>>>
>>>Yea, that is true also for the non-force case, but before it broke
>>>BLKROSET so I reverted that. We can use this FORCE_RO for BLKROSET as
>>>well I think...
>>
>>Let me prioritize the hard r/o setting.  mkp actually has an older patch
>>that did just that which I'll resurrect.
>
>Sounds good.

Cool. I'll repost fixing the log page update. I can then rebase on the
patches you send for this - or you can put them on top if it is easier.

Thanks!

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

* Re: nvme: enable ro namespace for ZNS without append
@ 2020-11-11  9:21           ` Javier González
  0 siblings, 0 replies; 16+ messages in thread
From: Javier González @ 2020-11-11  9:21 UTC (permalink / raw)
  To: Sagi Grimberg
  Cc: axboe, Niklas.Cassel, joshi.k, k.jensen, linux-nvme, linux-block,
	Keith Busch, Christoph Hellwig

On 11.11.2020 00:36, Sagi Grimberg wrote:
>
>>>>>-	if (id->nsattr & NVME_NS_ATTR_RO)
>>>>>+	if (id->nsattr & NVME_NS_ATTR_RO || test_bit(NVME_NS_FORCE_RO, &ns->flags))
>>>>>   		set_disk_ro(disk, true);
>>>>
>>>>If the FORCE_RO flag is set, the disk is set to read-only. If that flag
>>>>is later cleared, nothing clears the disk's read-only setting.
>>>
>>>Yea, that is true also for the non-force case, but before it broke
>>>BLKROSET so I reverted that. We can use this FORCE_RO for BLKROSET as
>>>well I think...
>>
>>Let me prioritize the hard r/o setting.  mkp actually has an older patch
>>that did just that which I'll resurrect.
>
>Sounds good.

Cool. I'll repost fixing the log page update. I can then rebase on the
patches you send for this - or you can put them on top if it is easier.

Thanks!

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

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

end of thread, other threads:[~2020-11-11  9:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10  9:39 [PATCH V2] nvme: enable ro namespace for ZNS without append Javier González
2020-11-10  9:39 ` Javier González
2020-11-10  9:43 ` Christoph Hellwig
2020-11-10  9:43   ` Christoph Hellwig
2020-11-10 10:15   ` Javier González
2020-11-10 10:15     ` Javier González
2020-11-10 11:25   ` [PATCH V2] " Niklas Cassel
2020-11-10 11:25     ` Niklas Cassel
2020-11-10 19:55     ` Javier González
2020-11-10 19:55       ` Javier González
2020-11-10 14:16 ` [PATCH V2] " Keith Busch
2020-11-10 14:16   ` Keith Busch
2020-11-10 15:53   ` Javier González
2020-11-10 15:53     ` Javier González
2020-11-10 21:07 [PATCH V3] " javier
2020-11-10 22:09 ` Keith Busch
2020-11-11  1:41   ` Sagi Grimberg
2020-11-11  8:15     ` Christoph Hellwig
2020-11-11  8:36       ` Sagi Grimberg
2020-11-11  9:21         ` Javier González
2020-11-11  9:21           ` Javier González

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.