* [PATCH V3] nvme: enable ro namespace for ZNS without append
@ 2020-11-10 21:07 ` javier
0 siblings, 0 replies; 18+ messages in thread
From: javier @ 2020-11-10 21:07 UTC (permalink / raw)
To: linux-nvme
Cc: linux-block, hch, kbusch, sagi, axboe, joshi.k, k.jensen,
Niklas.Cassel, Javier González
From: Javier González <javier.gonz@samsung.com>
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 V2:
- Fix small conflict with a queued patch from Sagi (from Christoph)
- Fix indentation (from Niklas)
- Refresh effects log page to account for FW changes (from Keith)
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 | 5 ++---
drivers/nvme/host/nvme.h | 2 ++
drivers/nvme/host/zns.c | 19 ++++++++++++++-----
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index fff90200497c..8a224a6f2473 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2083,7 +2083,7 @@ 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);
}
@@ -2951,8 +2951,7 @@ int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi,
return nvme_submit_sync_cmd(ctrl->admin_q, &c, log, size);
}
-static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,
- struct nvme_effects_log **log)
+int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi, struct nvme_effects_log **log)
{
struct nvme_cel *cel = xa_load(&ctrl->cels, csi);
int ret;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 83fb30e317e0..857fca95f016 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -449,6 +449,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;
@@ -638,6 +639,7 @@ int nvme_delete_ctrl(struct nvme_ctrl *ctrl);
int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi,
void *log, size_t size, u64 offset);
+int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi, struct nvme_effects_log **log);
struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk,
struct nvme_ns_head **head, int *srcu_idx);
void nvme_put_ns_from_disk(struct nvme_ns_head *head, int idx);
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 67e87e9f306f..47679a90795c 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -54,13 +54,22 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
struct nvme_id_ns_zns *id;
int status;
+ /* Refresh effects log page to check for changes on append support */
+ status = nvme_get_effects_log(ns->ctrl, ns->head->ids.csi, &ns->head->effects);
+ if (status)
+ return status;
+
/* Driver requires zone append support */
- if (!(le32_to_cpu(log->iocs[nvme_cmd_zone_append]) &
- NVME_CMD_EFFECTS_CSUPP)) {
+ if ((le32_to_cpu(log->iocs[nvme_cmd_zone_append]) & NVME_CMD_EFFECTS_CSUPP)) {
+ if (test_and_clear_bit(NVME_NS_FORCE_RO, &ns->flags))
+ dev_warn(ns->ctrl->device,
+ "append supported for zoned namespace:%d. Remove read-only mode\n",
+ ns->head->ns_id);
+ } else {
+ set_bit(NVME_NS_FORCE_RO, &ns->flags);
dev_warn(ns->ctrl->device,
- "append not supported for zoned namespace:%d\n",
- ns->head->ns_id);
- return -EINVAL;
+ "append not supported for zoned namespace:%d. Forcing to read-only mode\n",
+ ns->head->ns_id);
}
/* Lazily query controller append limit for the first zoned namespace */
--
2.17.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH V3] nvme: enable ro namespace for ZNS without append
@ 2020-11-10 21:07 ` javier
0 siblings, 0 replies; 18+ messages in thread
From: javier @ 2020-11-10 21:07 UTC (permalink / raw)
To: linux-nvme
Cc: axboe, Niklas.Cassel, sagi, joshi.k, k.jensen, linux-block,
kbusch, Javier González, hch
From: Javier González <javier.gonz@samsung.com>
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 V2:
- Fix small conflict with a queued patch from Sagi (from Christoph)
- Fix indentation (from Niklas)
- Refresh effects log page to account for FW changes (from Keith)
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 | 5 ++---
drivers/nvme/host/nvme.h | 2 ++
drivers/nvme/host/zns.c | 19 ++++++++++++++-----
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index fff90200497c..8a224a6f2473 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2083,7 +2083,7 @@ 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);
}
@@ -2951,8 +2951,7 @@ int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi,
return nvme_submit_sync_cmd(ctrl->admin_q, &c, log, size);
}
-static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,
- struct nvme_effects_log **log)
+int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi, struct nvme_effects_log **log)
{
struct nvme_cel *cel = xa_load(&ctrl->cels, csi);
int ret;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 83fb30e317e0..857fca95f016 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -449,6 +449,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;
@@ -638,6 +639,7 @@ int nvme_delete_ctrl(struct nvme_ctrl *ctrl);
int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi,
void *log, size_t size, u64 offset);
+int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi, struct nvme_effects_log **log);
struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk,
struct nvme_ns_head **head, int *srcu_idx);
void nvme_put_ns_from_disk(struct nvme_ns_head *head, int idx);
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 67e87e9f306f..47679a90795c 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -54,13 +54,22 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
struct nvme_id_ns_zns *id;
int status;
+ /* Refresh effects log page to check for changes on append support */
+ status = nvme_get_effects_log(ns->ctrl, ns->head->ids.csi, &ns->head->effects);
+ if (status)
+ return status;
+
/* Driver requires zone append support */
- if (!(le32_to_cpu(log->iocs[nvme_cmd_zone_append]) &
- NVME_CMD_EFFECTS_CSUPP)) {
+ if ((le32_to_cpu(log->iocs[nvme_cmd_zone_append]) & NVME_CMD_EFFECTS_CSUPP)) {
+ if (test_and_clear_bit(NVME_NS_FORCE_RO, &ns->flags))
+ dev_warn(ns->ctrl->device,
+ "append supported for zoned namespace:%d. Remove read-only mode\n",
+ ns->head->ns_id);
+ } else {
+ set_bit(NVME_NS_FORCE_RO, &ns->flags);
dev_warn(ns->ctrl->device,
- "append not supported for zoned namespace:%d\n",
- ns->head->ns_id);
- return -EINVAL;
+ "append not supported for zoned namespace:%d. Forcing to read-only mode\n",
+ ns->head->ns_id);
}
/* 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 [flat|nested] 18+ messages in thread
* Re: [PATCH V3] nvme: enable ro namespace for ZNS without append
2020-11-10 21:07 ` javier
@ 2020-11-10 22:09 ` Keith Busch
-1 siblings, 0 replies; 18+ messages in thread
From: Keith Busch @ 2020-11-10 22:09 UTC (permalink / raw)
To: javier
Cc: linux-nvme, linux-block, hch, sagi, axboe, joshi.k, k.jensen,
Niklas.Cassel, Javier González
On Tue, Nov 10, 2020 at 10:07:08PM +0100, javier@javigon.com 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.
> + /* Refresh effects log page to check for changes on append support */
> + status = nvme_get_effects_log(ns->ctrl, ns->head->ids.csi, &ns->head->effects);
That function just returns the cached log; no refresh occurs there.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V3] nvme: enable ro namespace for ZNS without append
@ 2020-11-10 22:09 ` Keith Busch
0 siblings, 0 replies; 18+ messages in thread
From: Keith Busch @ 2020-11-10 22:09 UTC (permalink / raw)
To: javier
Cc: axboe, Niklas.Cassel, sagi, joshi.k, k.jensen, linux-nvme,
linux-block, Javier González, hch
On Tue, Nov 10, 2020 at 10:07:08PM +0100, javier@javigon.com 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.
> + /* Refresh effects log page to check for changes on append support */
> + status = nvme_get_effects_log(ns->ctrl, ns->head->ids.csi, &ns->head->effects);
That function just returns the cached log; no refresh occurs there.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V3] nvme: enable ro namespace for ZNS without append
2020-11-10 22:09 ` Keith Busch
@ 2020-11-11 1:41 ` Sagi Grimberg
-1 siblings, 0 replies; 18+ messages in thread
From: Sagi Grimberg @ 2020-11-11 1:41 UTC (permalink / raw)
To: Keith Busch, javier
Cc: linux-nvme, linux-block, hch, 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);
>
> 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...
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V3] nvme: enable ro namespace for ZNS without append
@ 2020-11-11 1:41 ` Sagi Grimberg
0 siblings, 0 replies; 18+ messages in thread
From: Sagi Grimberg @ 2020-11-11 1:41 UTC (permalink / raw)
To: Keith Busch, javier
Cc: axboe, Niklas.Cassel, joshi.k, k.jensen, linux-nvme, linux-block,
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);
>
> 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...
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V3] nvme: enable ro namespace for ZNS without append
2020-11-11 1:41 ` Sagi Grimberg
@ 2020-11-11 8:15 ` Christoph Hellwig
-1 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2020-11-11 8:15 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Keith Busch, javier, linux-nvme, linux-block, hch, axboe,
joshi.k, k.jensen, Niklas.Cassel, Javier González
On Tue, Nov 10, 2020 at 05:41:15PM -0800, 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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V3] nvme: enable ro namespace for ZNS without append
@ 2020-11-11 8:15 ` Christoph Hellwig
0 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2020-11-11 8:15 UTC (permalink / raw)
To: Sagi Grimberg
Cc: axboe, Niklas.Cassel, javier, joshi.k, k.jensen, linux-nvme,
linux-block, Keith Busch, Javier González, hch
On Tue, Nov 10, 2020 at 05:41:15PM -0800, 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.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V3] nvme: enable ro namespace for ZNS without append
2020-11-11 8:15 ` Christoph Hellwig
@ 2020-11-11 8:36 ` Sagi Grimberg
-1 siblings, 0 replies; 18+ messages in thread
From: Sagi Grimberg @ 2020-11-11 8:36 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Keith Busch, javier, linux-nvme, linux-block, 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);
>>>
>>> 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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH V3] nvme: enable ro namespace for ZNS without append
@ 2020-11-11 8:36 ` Sagi Grimberg
0 siblings, 0 replies; 18+ messages in thread
From: Sagi Grimberg @ 2020-11-11 8:36 UTC (permalink / raw)
To: Christoph Hellwig
Cc: axboe, Niklas.Cassel, javier, joshi.k, k.jensen, linux-nvme,
linux-block, Keith Busch, 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);
>>>
>>> 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.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 18+ 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
-1 siblings, 0 replies; 18+ 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] 18+ messages in thread
* Re: nvme: enable ro namespace for ZNS without append
@ 2020-11-11 9:21 ` Javier González
0 siblings, 0 replies; 18+ 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] 18+ messages in thread
* Re: nvme: enable ro namespace for ZNS without append
2020-11-10 11:25 ` [PATCH V2] " Niklas Cassel
@ 2020-11-10 19:55 ` Javier González
0 siblings, 0 replies; 18+ 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] 18+ messages in thread
* Re: nvme: enable ro namespace for ZNS without append
@ 2020-11-10 19:55 ` Javier González
0 siblings, 0 replies; 18+ 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] 18+ messages in thread
* Re: nvme: enable ro namespace for ZNS without append
2020-11-10 14:16 ` [PATCH V2] " Keith Busch
@ 2020-11-10 15:53 ` Javier González
0 siblings, 0 replies; 18+ 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] 18+ messages in thread
* Re: nvme: enable ro namespace for ZNS without append
@ 2020-11-10 15:53 ` Javier González
0 siblings, 0 replies; 18+ 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] 18+ 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
2020-11-10 11:25 ` [PATCH V2] " Niklas Cassel
1 sibling, 0 replies; 18+ 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] 18+ messages in thread
* Re: nvme: enable ro namespace for ZNS without append
@ 2020-11-10 10:15 ` Javier González
0 siblings, 0 replies; 18+ 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] 18+ messages in thread
end of thread, other threads:[~2020-11-11 9:22 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 21:07 [PATCH V3] nvme: enable ro namespace for ZNS without append javier
2020-11-10 21:07 ` javier
2020-11-10 22:09 ` Keith Busch
2020-11-10 22:09 ` Keith Busch
2020-11-11 1:41 ` Sagi Grimberg
2020-11-11 1:41 ` Sagi Grimberg
2020-11-11 8:15 ` Christoph Hellwig
2020-11-11 8:15 ` Christoph Hellwig
2020-11-11 8:36 ` Sagi Grimberg
2020-11-11 8:36 ` Sagi Grimberg
2020-11-11 9:21 ` Javier González
2020-11-11 9:21 ` Javier González
-- strict thread matches above, loose matches on Subject: below --
2020-11-10 9:39 [PATCH V2] " Javier González
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 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 15:53 ` Javier González
2020-11-10 15:53 ` 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.