[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/3] hw/block/nvme: Check zone state before checking boundari
From: |
Klaus Jensen |
Subject: |
Re: [PATCH 2/3] hw/block/nvme: Check zone state before checking boundaries |
Date: |
Tue, 26 Jan 2021 08:54:17 +0100 |
On Jan 26 14:02, Dmitry Fomichev wrote:
> The code in nvme_check_zone_write() first checks for zone boundary
> condition violation and only after that it proceeds to verify that the
> zone state is suitable the write to happen. This is not consistent with
> nvme_check_zone_read() flow - in that function, zone state is checked
> before making any boundary checks. Besides, checking that ZSLBA + NLB
> does not cross the write boundary is now redundant for Zone Append and
> only needs to be done for writes.
>
> Move the check in the code to be performed for Write and Write Zeroes
> commands only and to occur after zone state checks.
>
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> ---
> hw/block/nvme.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 67538010ef..b712634c27 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1138,13 +1138,8 @@ static uint16_t nvme_check_zone_write(NvmeCtrl *n,
> NvmeNamespace *ns,
> uint64_t bndry = nvme_zone_wr_boundary(zone);
> uint16_t status;
>
> - if (unlikely(slba + nlb > bndry)) {
> - status = NVME_ZONE_BOUNDARY_ERROR;
> - } else {
> - status = nvme_check_zone_state_for_write(zone);
> - }
Alright. The double check on boundary that I pointed out in the previous
patch is fixed here.
> -
> - if (status != NVME_SUCCESS) {
> + status = nvme_check_zone_state_for_write(zone);
> + if (status) {
> trace_pci_nvme_err_zone_write_not_ok(slba, nlb, status);
> } else {
> assert(nvme_wp_is_valid(zone));
> @@ -1158,10 +1153,14 @@ static uint16_t nvme_check_zone_write(NvmeCtrl *n,
> NvmeNamespace *ns,
> trace_pci_nvme_err_append_too_large(slba, nlb, n->zasl);
> status = NVME_INVALID_FIELD;
> }
> - } else if (unlikely(slba != zone->w_ptr)) {
> - trace_pci_nvme_err_write_not_at_wp(slba, zone->d.zslba,
> - zone->w_ptr);
> - status = NVME_ZONE_INVALID_WRITE;
> + } else {
> + if (unlikely(slba != zone->w_ptr)) {
> + trace_pci_nvme_err_write_not_at_wp(slba, zone->d.zslba,
> + zone->w_ptr);
> + status = NVME_ZONE_INVALID_WRITE;
> + } else if (unlikely(slba + nlb > bndry)) {
> + status = NVME_ZONE_BOUNDARY_ERROR;
> + }
> }
> }
>
> --
> 2.28.0
>
>
--
One of us - No more doubt, silence or taboo about mental illness.
signature.asc
Description: PGP signature