[PATCH v4] nvme: fix interpretation of dmrsl

[Tom Yan]

Hi Keith,

IIRC I was once told that 0xffffffff should be set as the queue's max
discard sectors, regardless of logical block size, to signify that the
drive does not pose any limit. Besides I don't think it's a good idea
to put a value that is not a count of sectors into a variable which
expects that (as it would be confusing).

I also kept the overflow check. Let me know if there's a good reason not to.

Regards,
Tom

On Fri, 29 Apr 2022 at 12:53, Tom Yan <tom.ty89 at gmail.com> wrote:
>
> dmrsl is in the unit of logical block, while max_discard_sectors is
> in the unit of "linux sector".
>
> Signed-off-by: Tom Yan <tom.ty89 at gmail.com>
> ---
> v2: ignore dmrsl if its sector-equivalent is larger than UINT_MAX
> v3: put the lts_shift declaration before code
> v4: make use of nvme_sect_to_lba() and nvme_lba_to_sect()
> ---
>  drivers/nvme/host/core.c | 6 ++++--
>  drivers/nvme/host/nvme.h | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index efb85c6d8e2d..8e06a6d675ef 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1622,6 +1622,9 @@ static void nvme_config_discard(struct gendisk *disk, struct nvme_ns *ns)
>         if (blk_queue_flag_test_and_set(QUEUE_FLAG_DISCARD, queue))
>                 return;
>
> +       if (ctrl->dmrsl && ctrl->dmrsl <= nvme_sect_to_lba(ns, UINT_MAX))
> +               ctrl->max_discard_sectors = nvme_lba_to_sect(ns, ctrl->dmrsl);
> +
>         blk_queue_max_discard_sectors(queue, ctrl->max_discard_sectors);
>         blk_queue_max_discard_segments(queue, ctrl->max_discard_segments);
>
> @@ -2881,8 +2884,7 @@ static int nvme_init_non_mdts_limits(struct nvme_ctrl *ctrl)
>
>         if (id->dmrl)
>                 ctrl->max_discard_segments = id->dmrl;
> -       if (id->dmrsl)
> -               ctrl->max_discard_sectors = le32_to_cpu(id->dmrsl);
> +       ctrl->dmrsl = le32_to_cpu(id->dmrsl);
>         if (id->wzsl)
>                 ctrl->max_zeroes_sectors = nvme_mps_to_sectors(ctrl, id->wzsl);
>
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 1393bbf82d71..1c85edcf7dbb 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -279,6 +279,7 @@ struct nvme_ctrl {
>  #endif
>         u16 crdt[3];
>         u16 oncs;
> +       u32 dmrsl;
>         u16 oacs;
>         u16 sqsize;
>         u32 max_namespaces;
> --
> 2.36.0
>