* [PATCH 01/10] nvme: unlink head after removing last namespace
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
@ 2020-04-09 16:08 ` Keith Busch
2020-04-10 7:12 ` Sagi Grimberg
2020-04-09 16:09 ` [PATCH 02/10] nvme: release namespace head reference on error Keith Busch
` (9 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:08 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
The driver had been unlinking the namespace head from the subsystem's
list only after the last reference was released, and outside of the
list's subsys->lock protection.
There is no reason to track an empty head, so unlink the entry from the
subsystem's list when the last namespace using that head is removed and
with the mutex lock protecting the list update. The next namespace to
attach reusing the previous NSID will allocate a new head rather than
find the old head with mismatched identifiers.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 5de3b993525b..ba621f9229e5 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -433,7 +433,6 @@ static void nvme_free_ns_head(struct kref *ref)
nvme_mpath_remove_disk(head);
ida_simple_remove(&head->subsys->ns_ida, head->instance);
- list_del_init(&head->entry);
cleanup_srcu_struct(&head->srcu);
nvme_put_subsystem(head->subsys);
kfree(head);
@@ -3414,7 +3413,6 @@ static int __nvme_check_ids(struct nvme_subsystem *subsys,
list_for_each_entry(h, &subsys->nsheads, entry) {
if (nvme_ns_ids_valid(&new->ids) &&
- !list_empty(&h->list) &&
nvme_ns_ids_equal(&new->ids, &h->ids))
return -EINVAL;
}
@@ -3658,6 +3656,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
out_unlink_ns:
mutex_lock(&ctrl->subsys->lock);
list_del_rcu(&ns->siblings);
+ if (list_empty(&ns->head->list))
+ list_del_init(&ns->head->entry);
mutex_unlock(&ctrl->subsys->lock);
nvme_put_ns_head(ns->head);
out_free_id:
@@ -3677,7 +3677,10 @@ static void nvme_ns_remove(struct nvme_ns *ns)
mutex_lock(&ns->ctrl->subsys->lock);
list_del_rcu(&ns->siblings);
+ if (list_empty(&ns->head->list))
+ list_del_init(&ns->head->entry);
mutex_unlock(&ns->ctrl->subsys->lock);
+
synchronize_rcu(); /* guarantee not available in head->list */
nvme_mpath_clear_current_path(ns);
synchronize_srcu(&ns->head->srcu); /* wait for concurrent submissions */
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 02/10] nvme: release namespace head reference on error
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
2020-04-09 16:08 ` [PATCH 01/10] nvme: unlink head after removing last namespace Keith Busch
@ 2020-04-09 16:09 ` Keith Busch
2020-04-10 7:13 ` Sagi Grimberg
2020-04-09 16:09 ` [PATCH 03/10] nvme: always search for namespace head Keith Busch
` (8 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:09 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
If a namespace identification does not match the subsystem's head for
that NSID, release the reference that was taken when the matching head
was initially found.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ba621f9229e5..253ea57855db 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3503,6 +3503,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
"IDs don't match for shared namespace %d\n",
nsid);
ret = -EINVAL;
+ nvme_put_ns_head(head);
goto out_unlock;
}
}
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 03/10] nvme: always search for namespace head
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
2020-04-09 16:08 ` [PATCH 01/10] nvme: unlink head after removing last namespace Keith Busch
2020-04-09 16:09 ` [PATCH 02/10] nvme: release namespace head reference on error Keith Busch
@ 2020-04-09 16:09 ` Keith Busch
2020-04-10 7:31 ` Sagi Grimberg
2020-04-09 16:09 ` [PATCH 04/10] nvme: check namespace head shared property Keith Busch
` (7 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:09 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
Even if a namespace reports it is not capable of sharing, search the
subsystem for a matching namespace head. If found, the driver should
reject that namespace since it's coming from an invalid configuration.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 253ea57855db..be2d856923e8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3489,8 +3489,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
goto out;
mutex_lock(&ctrl->subsys->lock);
- if (is_shared)
- head = nvme_find_ns_head(ctrl->subsys, nsid);
+ head = nvme_find_ns_head(ctrl->subsys, nsid);
if (!head) {
head = nvme_alloc_ns_head(ctrl, nsid, &ids);
if (IS_ERR(head)) {
@@ -3498,6 +3497,14 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
goto out_unlock;
}
} else {
+ if (!is_shared) {
+ dev_err(ctrl->device,
+ "Duplicate unshared namespace %d\n",
+ nsid);
+ ret = -EINVAL;
+ nvme_put_ns_head(head);
+ goto out_unlock;
+ }
if (!nvme_ns_ids_equal(&head->ids, &ids)) {
dev_err(ctrl->device,
"IDs don't match for shared namespace %d\n",
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 03/10] nvme: always search for namespace head
2020-04-09 16:09 ` [PATCH 03/10] nvme: always search for namespace head Keith Busch
@ 2020-04-10 7:31 ` Sagi Grimberg
0 siblings, 0 replies; 22+ messages in thread
From: Sagi Grimberg @ 2020-04-10 7:31 UTC (permalink / raw)
To: Keith Busch, linux-nvme, hch
> Even if a namespace reports it is not capable of sharing, search the
> subsystem for a matching namespace head. If found, the driver should
> reject that namespace since it's coming from an invalid configuration.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> drivers/nvme/host/core.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 253ea57855db..be2d856923e8 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3489,8 +3489,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
> goto out;
>
> mutex_lock(&ctrl->subsys->lock);
> - if (is_shared)
> - head = nvme_find_ns_head(ctrl->subsys, nsid);
> + head = nvme_find_ns_head(ctrl->subsys, nsid);
> if (!head) {
> head = nvme_alloc_ns_head(ctrl, nsid, &ids);
> if (IS_ERR(head)) {
> @@ -3498,6 +3497,14 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
> goto out_unlock;
> }
> } else {
> + if (!is_shared) {
> + dev_err(ctrl->device,
> + "Duplicate unshared namespace %d\n",
> + nsid);
> + ret = -EINVAL;
> + nvme_put_ns_head(head);
> + goto out_unlock;
> + }
Given that you have multiple places that you put the ns head ref,
maybe it will be more maintainable if you add an out_put_ns_head tag to
goto and that will be like:
--
out_unlock:
mutex_unlock(&ctrl->subsys->lock);
out:
if (ret > 0)
ret = blk_status_to_errno(nvme_error_status(ret));
return ret;
out_put_ns_head:
nvme_put_ns_head(head);
goto out_unlock;
--
Not a must, but to me seems better than puting the ref explicitly
every time...
otherwise,
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 04/10] nvme: check namespace head shared property
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
` (2 preceding siblings ...)
2020-04-09 16:09 ` [PATCH 03/10] nvme: always search for namespace head Keith Busch
@ 2020-04-09 16:09 ` Keith Busch
2020-04-10 7:35 ` Sagi Grimberg
2020-04-09 16:09 ` [PATCH 05/10] nvme: don't directly update multipath queue settings Keith Busch
` (6 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:09 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
Reject a new shared namespace if a duplicate unshared namespace exists.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 3 ++-
drivers/nvme/host/nvme.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index be2d856923e8..09ffb2d85854 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3496,8 +3496,9 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
ret = PTR_ERR(head);
goto out_unlock;
}
+ head->shared = is_shared;
} else {
- if (!is_shared) {
+ if (!is_shared || !head->shared) {
dev_err(ctrl->device,
"Duplicate unshared namespace %d\n",
nsid);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 36f44b79bb3b..6222439a0776 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -352,6 +352,7 @@ struct nvme_ns_head {
struct nvme_ns_ids ids;
struct list_head entry;
struct kref ref;
+ bool shared;
int instance;
#ifdef CONFIG_NVME_MULTIPATH
struct gendisk *disk;
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 04/10] nvme: check namespace head shared property
2020-04-09 16:09 ` [PATCH 04/10] nvme: check namespace head shared property Keith Busch
@ 2020-04-10 7:35 ` Sagi Grimberg
0 siblings, 0 replies; 22+ messages in thread
From: Sagi Grimberg @ 2020-04-10 7:35 UTC (permalink / raw)
To: Keith Busch, linux-nvme, hch
> Reject a new shared namespace if a duplicate unshared namespace exists.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> drivers/nvme/host/core.c | 3 ++-
> drivers/nvme/host/nvme.h | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index be2d856923e8..09ffb2d85854 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3496,8 +3496,9 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
> ret = PTR_ERR(head);
> goto out_unlock;
> }
> + head->shared = is_shared;
IIRC max added features bitmask on a namespace for pi? Maybe use that
and Max can rebase?
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 05/10] nvme: don't directly update multipath queue settings
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
` (3 preceding siblings ...)
2020-04-09 16:09 ` [PATCH 04/10] nvme: check namespace head shared property Keith Busch
@ 2020-04-09 16:09 ` Keith Busch
2020-04-10 7:45 ` Sagi Grimberg
2020-04-09 16:09 ` [PATCH 06/10] nvme-multipath: set bdi capabilities once Keith Busch
` (5 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:09 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
Use only the stacking limits to update the multipath disk. Setting these
directly overwrites the stacked settings of other paths.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 09ffb2d85854..7e5d28ed7b0c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1907,7 +1907,6 @@ static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
nvme_update_disk_info(disk, ns, id);
#ifdef CONFIG_NVME_MULTIPATH
if (ns->head->disk) {
- nvme_update_disk_info(ns->head->disk, ns, id);
blk_queue_stack_limits(ns->head->disk->queue, ns->queue);
if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) {
struct backing_dev_info *info =
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 05/10] nvme: don't directly update multipath queue settings
2020-04-09 16:09 ` [PATCH 05/10] nvme: don't directly update multipath queue settings Keith Busch
@ 2020-04-10 7:45 ` Sagi Grimberg
2020-04-22 16:51 ` Keith Busch
0 siblings, 1 reply; 22+ messages in thread
From: Sagi Grimberg @ 2020-04-10 7:45 UTC (permalink / raw)
To: Keith Busch, linux-nvme, hch
> Use only the stacking limits to update the multipath disk. Setting these
> directly overwrites the stacked settings of other paths.
Um, but update_disk_info does more than blk_queue_stack_limits.
Can you explain more accurately what breaks here?
Who will register integtrity on the mpath disk? who will set capacity?
Maybe I'm missing something...
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> drivers/nvme/host/core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 09ffb2d85854..7e5d28ed7b0c 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1907,7 +1907,6 @@ static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
> nvme_update_disk_info(disk, ns, id);
> #ifdef CONFIG_NVME_MULTIPATH
> if (ns->head->disk) {
> - nvme_update_disk_info(ns->head->disk, ns, id);
> blk_queue_stack_limits(ns->head->disk->queue, ns->queue);
> if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) {
> struct backing_dev_info *info =
>
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 05/10] nvme: don't directly update multipath queue settings
2020-04-10 7:45 ` Sagi Grimberg
@ 2020-04-22 16:51 ` Keith Busch
0 siblings, 0 replies; 22+ messages in thread
From: Keith Busch @ 2020-04-22 16:51 UTC (permalink / raw)
To: Sagi Grimberg; +Cc: hch, linux-nvme
On Fri, Apr 10, 2020 at 12:45:57AM -0700, Sagi Grimberg wrote:
>
> > Use only the stacking limits to update the multipath disk. Setting these
> > directly overwrites the stacked settings of other paths.
>
> Um, but update_disk_info does more than blk_queue_stack_limits.
>
> Can you explain more accurately what breaks here?
> Who will register integtrity on the mpath disk? who will set capacity?
>
> Maybe I'm missing something...
Ah, you're right. It's only the limits like max transfer size, discard
granularity, etc... that need to be properly stacked rather than overwrite
them with the last path to validate. This patch needs a bit more work to
split those parts out.
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 06/10] nvme-multipath: set bdi capabilities once
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
` (4 preceding siblings ...)
2020-04-09 16:09 ` [PATCH 05/10] nvme: don't directly update multipath queue settings Keith Busch
@ 2020-04-09 16:09 ` Keith Busch
2020-04-10 7:46 ` Sagi Grimberg
2020-04-09 16:09 ` [PATCH 07/10] nvme: revalidate after verifying identifiers Keith Busch
` (4 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:09 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
The queues' backing device info capabilities don't change with each
namespace revalidation. Set it only when each path's request_queue
is initially added to a multipath queue.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 7 -------
drivers/nvme/host/multipath.c | 8 ++++++++
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 7e5d28ed7b0c..2a27dcb7fac7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1908,13 +1908,6 @@ static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
#ifdef CONFIG_NVME_MULTIPATH
if (ns->head->disk) {
blk_queue_stack_limits(ns->head->disk->queue, ns->queue);
- if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) {
- struct backing_dev_info *info =
- ns->head->disk->queue->backing_dev_info;
-
- info->capabilities |= BDI_CAP_STABLE_WRITES;
- }
-
revalidate_disk(ns->head->disk);
}
#endif
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 54603bd3e02d..9f2844935fdf 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -3,6 +3,7 @@
* Copyright (c) 2017-2018 Christoph Hellwig.
*/
+#include <linux/backing-dev.h>
#include <linux/moduleparam.h>
#include <trace/events/block.h>
#include "nvme.h"
@@ -666,6 +667,13 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id)
nvme_mpath_set_live(ns);
mutex_unlock(&ns->head->lock);
}
+
+ if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) {
+ struct backing_dev_info *info =
+ ns->head->disk->queue->backing_dev_info;
+
+ info->capabilities |= BDI_CAP_STABLE_WRITES;
+ }
}
void nvme_mpath_remove_disk(struct nvme_ns_head *head)
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 07/10] nvme: revalidate after verifying identifiers
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
` (5 preceding siblings ...)
2020-04-09 16:09 ` [PATCH 06/10] nvme-multipath: set bdi capabilities once Keith Busch
@ 2020-04-09 16:09 ` Keith Busch
2020-04-10 7:47 ` Sagi Grimberg
2020-04-09 16:09 ` [PATCH 08/10] nvme: consolidate chunk_sectors settings Keith Busch
` (3 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:09 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
If the namespace identifiers have changed, skip updating the disk
information, as that will register parameters from a mismatched
namespace.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2a27dcb7fac7..081c9d47fb41 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1935,7 +1935,6 @@ static int nvme_revalidate_disk(struct gendisk *disk)
goto free_id;
}
- __nvme_revalidate_disk(disk, id);
ret = nvme_report_ns_ids(ctrl, ns->head->ns_id, id, &ids);
if (ret)
goto free_id;
@@ -1944,8 +1943,10 @@ static int nvme_revalidate_disk(struct gendisk *disk)
dev_err(ctrl->device,
"identifiers changed for nsid %d\n", ns->head->ns_id);
ret = -ENODEV;
+ goto free_id;
}
+ __nvme_revalidate_disk(disk, id);
free_id:
kfree(id);
out:
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 08/10] nvme: consolidate chunk_sectors settings
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
` (6 preceding siblings ...)
2020-04-09 16:09 ` [PATCH 07/10] nvme: revalidate after verifying identifiers Keith Busch
@ 2020-04-09 16:09 ` Keith Busch
2020-04-10 7:49 ` Sagi Grimberg
2020-04-09 16:09 ` [PATCH 09/10] nvme: revalidate namespace stream parameters Keith Busch
` (2 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:09 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
Move the quirked chunk_sectors setting to the same location as noiob so
one place registers this setting. And since the noiob value is only used
locally, remove the member from struct nvme_ns.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 22 ++++++++++------------
drivers/nvme/host/nvme.h | 1 -
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 081c9d47fb41..f80d65bb4308 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1725,12 +1725,6 @@ static void nvme_init_integrity(struct gendisk *disk, u16 ms, u8 pi_type)
}
#endif /* CONFIG_BLK_DEV_INTEGRITY */
-static void nvme_set_chunk_size(struct nvme_ns *ns)
-{
- u32 chunk_size = nvme_lba_to_sect(ns, ns->noiob);
- blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(chunk_size));
-}
-
static void nvme_config_discard(struct gendisk *disk, struct nvme_ns *ns)
{
struct nvme_ctrl *ctrl = ns->ctrl;
@@ -1885,6 +1879,7 @@ static void nvme_update_disk_info(struct gendisk *disk,
static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
{
struct nvme_ns *ns = disk->private_data;
+ u32 iob;
/*
* If identify namespace failed, use default 512 byte block size so
@@ -1893,7 +1888,13 @@ static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
ns->lba_shift = id->lbaf[id->flbas & NVME_NS_FLBAS_LBA_MASK].ds;
if (ns->lba_shift == 0)
ns->lba_shift = 9;
- ns->noiob = le16_to_cpu(id->noiob);
+
+ if ((ns->ctrl->quirks & NVME_QUIRK_STRIPE_SIZE) &&
+ is_power_of_2(ns->ctrl->max_hw_sectors))
+ iob = ns->ctrl->max_hw_sectors;
+ else
+ iob = nvme_lba_to_sect(ns, le16_to_cpu(id->noiob));
+
ns->ms = le16_to_cpu(id->lbaf[id->flbas & NVME_NS_FLBAS_LBA_MASK].ms);
ns->ext = ns->ms && (id->flbas & NVME_NS_FLBAS_META_EXT);
/* the PI implementation requires metadata equal t10 pi tuple size */
@@ -1902,8 +1903,8 @@ static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
else
ns->pi_type = 0;
- if (ns->noiob)
- nvme_set_chunk_size(ns);
+ if (iob)
+ blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
nvme_update_disk_info(disk, ns, id);
#ifdef CONFIG_NVME_MULTIPATH
if (ns->head->disk) {
@@ -2254,9 +2255,6 @@ static void nvme_set_queue_limits(struct nvme_ctrl *ctrl,
blk_queue_max_hw_sectors(q, ctrl->max_hw_sectors);
blk_queue_max_segments(q, min_t(u32, max_segments, USHRT_MAX));
}
- if ((ctrl->quirks & NVME_QUIRK_STRIPE_SIZE) &&
- is_power_of_2(ctrl->max_hw_sectors))
- blk_queue_chunk_sectors(q, ctrl->max_hw_sectors);
blk_queue_virt_boundary(q, ctrl->page_size - 1);
if (ctrl->vwc & NVME_CTRL_VWC_PRESENT)
vwc = true;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 6222439a0776..f3ab17778349 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -389,7 +389,6 @@ struct nvme_ns {
#define NVME_NS_REMOVING 0
#define NVME_NS_DEAD 1
#define NVME_NS_ANA_PENDING 2
- u16 noiob;
struct nvme_fault_inject fault_inject;
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 09/10] nvme: revalidate namespace stream parameters
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
` (7 preceding siblings ...)
2020-04-09 16:09 ` [PATCH 08/10] nvme: consolidate chunk_sectors settings Keith Busch
@ 2020-04-09 16:09 ` Keith Busch
2020-04-10 7:50 ` Sagi Grimberg
2020-04-09 16:09 ` [PATCH 10/10] nvme: consolodate io settings Keith Busch
2020-04-22 7:56 ` [PATCH 00/10] namespace settings updates Christoph Hellwig
10 siblings, 1 reply; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:09 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
The stream parameters are based on the currently formatted logical block
size. Recheck these parameters on namespace revalidation so the
registered constraints will be accurate.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 54 ++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f80d65bb4308..e9f71a4c6b89 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1810,6 +1810,32 @@ static bool nvme_ns_ids_equal(struct nvme_ns_ids *a, struct nvme_ns_ids *b)
memcmp(&a->eui64, &b->eui64, sizeof(a->eui64)) == 0;
}
+static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns)
+{
+ struct streams_directive_params s;
+ int ret;
+
+ if (!ctrl->nr_streams)
+ return 0;
+
+ ret = nvme_get_stream_params(ctrl, &s, ns->head->ns_id);
+ if (ret)
+ return ret;
+
+ ns->sws = le32_to_cpu(s.sws);
+ ns->sgs = le16_to_cpu(s.sgs);
+
+ if (ns->sws) {
+ unsigned int bs = 1 << ns->lba_shift;
+
+ blk_queue_io_min(ns->queue, bs * ns->sws);
+ if (ns->sgs)
+ blk_queue_io_opt(ns->queue, bs * ns->sws * ns->sgs);
+ }
+
+ return 0;
+}
+
static void nvme_update_disk_info(struct gendisk *disk,
struct nvme_ns *ns, struct nvme_id_ns *id)
{
@@ -1824,6 +1850,7 @@ static void nvme_update_disk_info(struct gendisk *disk,
blk_mq_freeze_queue(disk->queue);
blk_integrity_unregister(disk);
+ nvme_setup_streams_ns(ns->ctrl, ns);
if (id->nabo == 0) {
/*
* Bit 1 indicates whether NAWUPF is defined for this namespace
@@ -3545,32 +3572,6 @@ static struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
return ret;
}
-static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns)
-{
- struct streams_directive_params s;
- int ret;
-
- if (!ctrl->nr_streams)
- return 0;
-
- ret = nvme_get_stream_params(ctrl, &s, ns->head->ns_id);
- if (ret)
- return ret;
-
- ns->sws = le32_to_cpu(s.sws);
- ns->sgs = le16_to_cpu(s.sgs);
-
- if (ns->sws) {
- unsigned int bs = 1 << ns->lba_shift;
-
- blk_queue_io_min(ns->queue, bs * ns->sws);
- if (ns->sgs)
- blk_queue_io_opt(ns->queue, bs * ns->sws * ns->sgs);
- }
-
- return 0;
-}
-
static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
{
struct nvme_ns *ns;
@@ -3614,7 +3615,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
ret = nvme_init_ns_head(ns, nsid, id);
if (ret)
goto out_free_id;
- nvme_setup_streams_ns(ctrl, ns);
nvme_set_disk_name(disk_name, ns, ctrl, &flags);
disk = alloc_disk_node(0, node);
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 10/10] nvme: consolodate io settings
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
` (8 preceding siblings ...)
2020-04-09 16:09 ` [PATCH 09/10] nvme: revalidate namespace stream parameters Keith Busch
@ 2020-04-09 16:09 ` Keith Busch
2020-04-22 7:56 ` [PATCH 00/10] namespace settings updates Christoph Hellwig
10 siblings, 0 replies; 22+ messages in thread
From: Keith Busch @ 2020-04-09 16:09 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Keith Busch
The stream parameters indicating optimal io settings were just getting
overwritten later. Rearrange the settings so the streams parameters can
be preserved if provided.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/nvme/host/core.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e9f71a4c6b89..9d521f198ce7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1810,7 +1810,8 @@ static bool nvme_ns_ids_equal(struct nvme_ns_ids *a, struct nvme_ns_ids *b)
memcmp(&a->eui64, &b->eui64, sizeof(a->eui64)) == 0;
}
-static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns)
+static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
+ u32 *phys_bs, u32 *io_opt)
{
struct streams_directive_params s;
int ret;
@@ -1826,11 +1827,9 @@ static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns)
ns->sgs = le16_to_cpu(s.sgs);
if (ns->sws) {
- unsigned int bs = 1 << ns->lba_shift;
-
- blk_queue_io_min(ns->queue, bs * ns->sws);
+ *phys_bs = ns->sws * (1 << ns->lba_shift);
if (ns->sgs)
- blk_queue_io_opt(ns->queue, bs * ns->sws * ns->sgs);
+ *io_opt = *phys_bs * ns->sgs;
}
return 0;
@@ -1850,7 +1849,8 @@ static void nvme_update_disk_info(struct gendisk *disk,
blk_mq_freeze_queue(disk->queue);
blk_integrity_unregister(disk);
- nvme_setup_streams_ns(ns->ctrl, ns);
+ atomic_bs = phys_bs = io_opt = bs;
+ nvme_setup_streams_ns(ns->ctrl, ns, &phys_bs, &io_opt);
if (id->nabo == 0) {
/*
* Bit 1 indicates whether NAWUPF is defined for this namespace
@@ -1861,16 +1861,13 @@ static void nvme_update_disk_info(struct gendisk *disk,
atomic_bs = (1 + le16_to_cpu(id->nawupf)) * bs;
else
atomic_bs = (1 + ns->ctrl->subsys->awupf) * bs;
- } else {
- atomic_bs = 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);
+ phys_bs = bs * (1 + le16_to_cpu(id->npwg));
/* NOWS = Namespace Optimal Write Size */
- io_opt *= 1 + le16_to_cpu(id->nows);
+ io_opt = bs * (1 + le16_to_cpu(id->nows));
}
blk_queue_logical_block_size(disk->queue, bs);
--
2.24.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 00/10] namespace settings updates
2020-04-09 16:08 [PATCH 00/10] namespace settings updates Keith Busch
` (9 preceding siblings ...)
2020-04-09 16:09 ` [PATCH 10/10] nvme: consolodate io settings Keith Busch
@ 2020-04-22 7:56 ` Christoph Hellwig
10 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2020-04-22 7:56 UTC (permalink / raw)
To: Keith Busch; +Cc: hch, linux-nvme, sagi
Thanks,
I've applied all but patch 5, for which Sagi had a question.
I'll send a separate patch to address the cleanups Sagi suggested for
patch 3.
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 22+ messages in thread