Re: [PATCH v10 09/12] hw/block/nvme: Introduce max active and open zone limits
On Nov 7 08:43, Dmitry Fomichev wrote:
> Add two module properties, "zoned.max_active" and "zoned.max_open"
> to control the maximum number of zones that can be active or open.
> Once these variables are set to non-default values, these limits are
> checked during I/O and Too Many Active or Too Many Open command status
> is returned if they are exceeded.
>
> Signed-off-by: Hans Holmberg
> Signed-off-by: Dmitry Fomichev
> Reviewed-by: Niklas Cassel
> ---
For Zone Management Send, if the action is Open Zone and Select All is
set, it must be checked that there are enough resources for all Closed
zones to transition to Explicitly Opened - otherwise no state
transitions "shall" occur.
> hw/block/nvme-ns.h| 41 +++
> hw/block/nvme-ns.c| 30 +-
> hw/block/nvme.c | 94 +++
> hw/block/trace-events | 2 +
> 4 files changed, 165 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
> index d2631ff5a3..421bab0a57 100644
> --- a/hw/block/nvme-ns.h
> +++ b/hw/block/nvme-ns.h
> @@ -33,6 +33,8 @@ typedef struct NvmeNamespaceParams {
> bool cross_zone_read;
> uint64_t zone_size_bs;
> uint64_t zone_cap_bs;
> +uint32_t max_active_zones;
> +uint32_t max_open_zones;
> } NvmeNamespaceParams;
>
> typedef struct NvmeNamespace {
> @@ -56,6 +58,8 @@ typedef struct NvmeNamespace {
> uint64_tzone_capacity;
> uint64_tzone_array_size;
> uint32_tzone_size_log2;
> +int32_t nr_open_zones;
> +int32_t nr_active_zones;
>
> NvmeNamespaceParams params;
> } NvmeNamespace;
> @@ -123,6 +127,43 @@ static inline bool nvme_wp_is_valid(NvmeZone *zone)
> st != NVME_ZONE_STATE_OFFLINE;
> }
>
> +static inline void nvme_aor_inc_open(NvmeNamespace *ns)
> +{
> +assert(ns->nr_open_zones >= 0);
> +if (ns->params.max_open_zones) {
> +ns->nr_open_zones++;
> +assert(ns->nr_open_zones <= ns->params.max_open_zones);
> +}
> +}
> +
> +static inline void nvme_aor_dec_open(NvmeNamespace *ns)
> +{
> +if (ns->params.max_open_zones) {
> +assert(ns->nr_open_zones > 0);
> +ns->nr_open_zones--;
> +}
> +assert(ns->nr_open_zones >= 0);
> +}
> +
> +static inline void nvme_aor_inc_active(NvmeNamespace *ns)
> +{
> +assert(ns->nr_active_zones >= 0);
> +if (ns->params.max_active_zones) {
> +ns->nr_active_zones++;
> +assert(ns->nr_active_zones <= ns->params.max_active_zones);
> +}
> +}
> +
> +static inline void nvme_aor_dec_active(NvmeNamespace *ns)
> +{
> +if (ns->params.max_active_zones) {
> +assert(ns->nr_active_zones > 0);
> +ns->nr_active_zones--;
> +assert(ns->nr_active_zones >= ns->nr_open_zones);
> +}
> +assert(ns->nr_active_zones >= 0);
> +}
> +
> int nvme_ns_setup(NvmeCtrl *n, NvmeNamespace *ns, Error **errp);
> void nvme_ns_drain(NvmeNamespace *ns);
> void nvme_ns_flush(NvmeNamespace *ns);
> diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
> index e6db7f7d3b..2e45838c15 100644
> --- a/hw/block/nvme-ns.c
> +++ b/hw/block/nvme-ns.c
> @@ -119,6 +119,20 @@ static int nvme_calc_zone_geometry(NvmeNamespace *ns,
> Error **errp)
> ns->zone_size_log2 = 63 - clz64(ns->zone_size);
> }
>
> +/* Make sure that the values of all ZNS properties are sane */
> +if (ns->params.max_open_zones > nz) {
> +error_setg(errp,
> + "max_open_zones value %u exceeds the number of zones %u",
> + ns->params.max_open_zones, nz);
> +return -1;
> +}
> +if (ns->params.max_active_zones > nz) {
> +error_setg(errp,
> + "max_active_zones value %u exceeds the number of zones
> %u",
> + ns->params.max_active_zones, nz);
> +return -1;
> +}
> +
> return 0;
> }
>
> @@ -166,8 +180,8 @@ static int nvme_zoned_init_ns(NvmeCtrl *n, NvmeNamespace
> *ns, int lba_index,
> id_ns_z = g_malloc0(sizeof(NvmeIdNsZoned));
>
> /* MAR/MOR are zeroes-based, 0x means no limit */
> -id_ns_z->mar = 0x;
> -id_ns_z->mor = 0x;
> +id_ns_z->mar = cpu_to_le32(ns->params.max_active_zones - 1);
> +id_ns_z->mor = cpu_to_le32(ns->params.max_open_zones - 1);
> id_ns_z->zoc = 0;
> id_ns_z->ozcs = ns->params.cross_zone_read ? 0x01 : 0x00;
>
> @@ -195,6 +209,7 @@ static void nvme_clear_zone(NvmeNamespace *ns, NvmeZone
> *zone)
> trace_pci_nvme_clear_ns_close(state, zone->d.zslba);
> nvme_set_zone_state(zone, NVME_ZONE_STATE_CLOSED);
> }
> +nvme_aor_inc_active(ns);
> QTAILQ_INSERT_HEAD(>closed_zones, zone, entry);
> } else {
> trace_pci_nvme_clear_ns_reset(state, zone->d.zslba);
> @@ -211,16 +226,23 @@ static void nvme_zoned_ns_shutdown(NvmeNamespace *ns)
>
> QTAILQ_FOREACH_SAFE(zone, >closed_zones,
[PATCH v10 09/12] hw/block/nvme: Introduce max active and open zone limits
Add two module properties, "zoned.max_active" and "zoned.max_open"
to control the maximum number of zones that can be active or open.
Once these variables are set to non-default values, these limits are
checked during I/O and Too Many Active or Too Many Open command status
is returned if they are exceeded.
Signed-off-by: Hans Holmberg
Signed-off-by: Dmitry Fomichev
Reviewed-by: Niklas Cassel
---
hw/block/nvme-ns.h| 41 +++
hw/block/nvme-ns.c| 30 +-
hw/block/nvme.c | 94 +++
hw/block/trace-events | 2 +
4 files changed, 165 insertions(+), 2 deletions(-)
diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
index d2631ff5a3..421bab0a57 100644
--- a/hw/block/nvme-ns.h
+++ b/hw/block/nvme-ns.h
@@ -33,6 +33,8 @@ typedef struct NvmeNamespaceParams {
bool cross_zone_read;
uint64_t zone_size_bs;
uint64_t zone_cap_bs;
+uint32_t max_active_zones;
+uint32_t max_open_zones;
} NvmeNamespaceParams;
typedef struct NvmeNamespace {
@@ -56,6 +58,8 @@ typedef struct NvmeNamespace {
uint64_tzone_capacity;
uint64_tzone_array_size;
uint32_tzone_size_log2;
+int32_t nr_open_zones;
+int32_t nr_active_zones;
NvmeNamespaceParams params;
} NvmeNamespace;
@@ -123,6 +127,43 @@ static inline bool nvme_wp_is_valid(NvmeZone *zone)
st != NVME_ZONE_STATE_OFFLINE;
}
+static inline void nvme_aor_inc_open(NvmeNamespace *ns)
+{
+assert(ns->nr_open_zones >= 0);
+if (ns->params.max_open_zones) {
+ns->nr_open_zones++;
+assert(ns->nr_open_zones <= ns->params.max_open_zones);
+}
+}
+
+static inline void nvme_aor_dec_open(NvmeNamespace *ns)
+{
+if (ns->params.max_open_zones) {
+assert(ns->nr_open_zones > 0);
+ns->nr_open_zones--;
+}
+assert(ns->nr_open_zones >= 0);
+}
+
+static inline void nvme_aor_inc_active(NvmeNamespace *ns)
+{
+assert(ns->nr_active_zones >= 0);
+if (ns->params.max_active_zones) {
+ns->nr_active_zones++;
+assert(ns->nr_active_zones <= ns->params.max_active_zones);
+}
+}
+
+static inline void nvme_aor_dec_active(NvmeNamespace *ns)
+{
+if (ns->params.max_active_zones) {
+assert(ns->nr_active_zones > 0);
+ns->nr_active_zones--;
+assert(ns->nr_active_zones >= ns->nr_open_zones);
+}
+assert(ns->nr_active_zones >= 0);
+}
+
int nvme_ns_setup(NvmeCtrl *n, NvmeNamespace *ns, Error **errp);
void nvme_ns_drain(NvmeNamespace *ns);
void nvme_ns_flush(NvmeNamespace *ns);
diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index e6db7f7d3b..2e45838c15 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -119,6 +119,20 @@ static int nvme_calc_zone_geometry(NvmeNamespace *ns,
Error **errp)
ns->zone_size_log2 = 63 - clz64(ns->zone_size);
}
+/* Make sure that the values of all ZNS properties are sane */
+if (ns->params.max_open_zones > nz) {
+error_setg(errp,
+ "max_open_zones value %u exceeds the number of zones %u",
+ ns->params.max_open_zones, nz);
+return -1;
+}
+if (ns->params.max_active_zones > nz) {
+error_setg(errp,
+ "max_active_zones value %u exceeds the number of zones %u",
+ ns->params.max_active_zones, nz);
+return -1;
+}
+
return 0;
}
@@ -166,8 +180,8 @@ static int nvme_zoned_init_ns(NvmeCtrl *n, NvmeNamespace
*ns, int lba_index,
id_ns_z = g_malloc0(sizeof(NvmeIdNsZoned));
/* MAR/MOR are zeroes-based, 0x means no limit */
-id_ns_z->mar = 0x;
-id_ns_z->mor = 0x;
+id_ns_z->mar = cpu_to_le32(ns->params.max_active_zones - 1);
+id_ns_z->mor = cpu_to_le32(ns->params.max_open_zones - 1);
id_ns_z->zoc = 0;
id_ns_z->ozcs = ns->params.cross_zone_read ? 0x01 : 0x00;
@@ -195,6 +209,7 @@ static void nvme_clear_zone(NvmeNamespace *ns, NvmeZone
*zone)
trace_pci_nvme_clear_ns_close(state, zone->d.zslba);
nvme_set_zone_state(zone, NVME_ZONE_STATE_CLOSED);
}
+nvme_aor_inc_active(ns);
QTAILQ_INSERT_HEAD(>closed_zones, zone, entry);
} else {
trace_pci_nvme_clear_ns_reset(state, zone->d.zslba);
@@ -211,16 +226,23 @@ static void nvme_zoned_ns_shutdown(NvmeNamespace *ns)
QTAILQ_FOREACH_SAFE(zone, >closed_zones, entry, next) {
QTAILQ_REMOVE(>closed_zones, zone, entry);
+nvme_aor_dec_active(ns);
nvme_clear_zone(ns, zone);
}
QTAILQ_FOREACH_SAFE(zone, >imp_open_zones, entry, next) {
QTAILQ_REMOVE(>imp_open_zones, zone, entry);
+nvme_aor_dec_open(ns);
+nvme_aor_dec_active(ns);
nvme_clear_zone(ns, zone);
}
QTAILQ_FOREACH_SAFE(zone, >exp_open_zones, entry, next) {
QTAILQ_REMOVE(>exp_open_zones, zone, entry);
+nvme_aor_dec_open(ns);
+
