* [PATCH v2] nvme: fix interpretation of dmrsl
@ 2022-04-21 8:48 Tom Yan
2022-04-28 15:10 ` Christoph Hellwig
0 siblings, 1 reply; 7+ messages in thread
From: Tom Yan @ 2022-04-21 8:48 UTC (permalink / raw)
To: linux-nvme; +Cc: Tom Yan
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@gmail.com>
---
v2: ignore dmrsl if its sector-equivalent is larger than UINT_MAX
---
drivers/nvme/host/core.c | 8 ++++++--
drivers/nvme/host/nvme.h | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index efb85c6d8e2d..9d2f32739a15 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1622,6 +1622,11 @@ static void nvme_config_discard(struct gendisk *disk, struct nvme_ns *ns)
if (blk_queue_flag_test_and_set(QUEUE_FLAG_DISCARD, queue))
return;
+ int lts_shift = ilog2(size) - 9;
+
+ if (ctrl->dmrsl && ctrl->dmrsl <= UINT_MAX >> lts_shift)
+ ctrl->max_discard_sectors = ctrl->dmrsl << lts_shift;
+
blk_queue_max_discard_sectors(queue, ctrl->max_discard_sectors);
blk_queue_max_discard_segments(queue, ctrl->max_discard_segments);
@@ -2881,8 +2886,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.35.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] nvme: fix interpretation of dmrsl
2022-04-21 8:48 [PATCH v2] nvme: fix interpretation of dmrsl Tom Yan
@ 2022-04-28 15:10 ` Christoph Hellwig
2022-04-28 16:50 ` [PATCH v3] " Tom Yan
0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2022-04-28 15:10 UTC (permalink / raw)
To: Tom Yan; +Cc: linux-nvme
This doesn't even compile:
drivers/nvme/host/core.c: In function ‘nvme_config_discard’:
drivers/nvme/host/core.c:1638:2: error: ISO C90 forbids mixed
declarations and code [-Werror=declaration-after-statement]
1638 | int lts_shift = ilog2(size) - 9;
| ^~~
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3] nvme: fix interpretation of dmrsl
2022-04-28 15:10 ` Christoph Hellwig
@ 2022-04-28 16:50 ` Tom Yan
2022-04-28 17:18 ` Keith Busch
0 siblings, 1 reply; 7+ messages in thread
From: Tom Yan @ 2022-04-28 16:50 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +Cc: Tom Yan
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@gmail.com>
---
v2: ignore dmrsl if its sector-equivalent is larger than UINT_MAX
v3: put the lts_shift declaration before code
---
drivers/nvme/host/core.c | 7 +++++--
drivers/nvme/host/nvme.h | 1 +
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index efb85c6d8e2d..8bee0d139fd6 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1606,6 +1606,7 @@ static void nvme_config_discard(struct gendisk *disk, struct nvme_ns *ns)
struct nvme_ctrl *ctrl = ns->ctrl;
struct request_queue *queue = disk->queue;
u32 size = queue_logical_block_size(queue);
+ int lts_shift = ilog2(size) - 9;
if (ctrl->max_discard_sectors == 0) {
blk_queue_flag_clear(QUEUE_FLAG_DISCARD, queue);
@@ -1622,6 +1623,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 <= UINT_MAX >> lts_shift)
+ ctrl->max_discard_sectors = ctrl->dmrsl << lts_shift;
+
blk_queue_max_discard_sectors(queue, ctrl->max_discard_sectors);
blk_queue_max_discard_segments(queue, ctrl->max_discard_segments);
@@ -2881,8 +2885,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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] nvme: fix interpretation of dmrsl
2022-04-28 16:50 ` [PATCH v3] " Tom Yan
@ 2022-04-28 17:18 ` Keith Busch
2022-04-29 4:52 ` [PATCH v4] " Tom Yan
0 siblings, 1 reply; 7+ messages in thread
From: Keith Busch @ 2022-04-28 17:18 UTC (permalink / raw)
To: Tom Yan; +Cc: linux-nvme, hch, sagi
On Fri, Apr 29, 2022 at 12:50:51AM +0800, Tom Yan wrote:
> @@ -1606,6 +1606,7 @@ static void nvme_config_discard(struct gendisk *disk, struct nvme_ns *ns)
> struct nvme_ctrl *ctrl = ns->ctrl;
> struct request_queue *queue = disk->queue;
> u32 size = queue_logical_block_size(queue);
> + int lts_shift = ilog2(size) - 9;
>
> if (ctrl->max_discard_sectors == 0) {
> blk_queue_flag_clear(QUEUE_FLAG_DISCARD, queue);
> @@ -1622,6 +1623,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 <= UINT_MAX >> lts_shift)
> + ctrl->max_discard_sectors = ctrl->dmrsl << lts_shift;
> +
> blk_queue_max_discard_sectors(queue, ctrl->max_discard_sectors);
I think the only change you really need is just changing the above line to:
blk_queue_max_discard_sectors(queue, nvme_lba_to_sect(ns, ctrl->max_discard_sectors));
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4] nvme: fix interpretation of dmrsl
2022-04-28 17:18 ` Keith Busch
@ 2022-04-29 4:52 ` Tom Yan
2022-04-29 5:01 ` Tom Yan
2022-05-10 6:04 ` Christoph Hellwig
0 siblings, 2 replies; 7+ messages in thread
From: Tom Yan @ 2022-04-29 4:52 UTC (permalink / raw)
To: linux-nvme, kbusch, hch, sagi; +Cc: Tom Yan
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@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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4] nvme: fix interpretation of dmrsl
2022-04-29 4:52 ` [PATCH v4] " Tom Yan
@ 2022-04-29 5:01 ` Tom Yan
2022-05-10 6:04 ` Christoph Hellwig
1 sibling, 0 replies; 7+ messages in thread
From: Tom Yan @ 2022-04-29 5:01 UTC (permalink / raw)
To: linux-nvme, kbusch, Christoph Hellwig, sagi
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@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@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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] nvme: fix interpretation of dmrsl
2022-04-29 4:52 ` [PATCH v4] " Tom Yan
2022-04-29 5:01 ` Tom Yan
@ 2022-05-10 6:04 ` Christoph Hellwig
1 sibling, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2022-05-10 6:04 UTC (permalink / raw)
To: Tom Yan; +Cc: linux-nvme, kbusch, hch, sagi
Thanks,
applied to nvme-5.19.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-05-10 6:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-21 8:48 [PATCH v2] nvme: fix interpretation of dmrsl Tom Yan
2022-04-28 15:10 ` Christoph Hellwig
2022-04-28 16:50 ` [PATCH v3] " Tom Yan
2022-04-28 17:18 ` Keith Busch
2022-04-29 4:52 ` [PATCH v4] " Tom Yan
2022-04-29 5:01 ` Tom Yan
2022-05-10 6:04 ` Christoph Hellwig
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.