Re: [PATCH 2/4] nvme: also skip discard granularity updates in nvme_config_discard

Max Gurtovoy
2023-12-26T23:18:39Z

On 26/12/2023 10:58, Christoph Hellwig wrote:
> Don't just skip the discard sectors and segments but also the granularity
> if a value was already set before.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/nvme/host/core.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 56107cfc97b7bc..6c52b0ab382c85 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1727,7 +1727,6 @@ static void nvme_config_discard(struct nvme_ctrl *ctrl, struct gendisk *disk,
> struct nvme_ns_head *head)
> {
> struct request_queue *queue = disk->queue;
> - u32 size = queue_logical_block_size(queue);
>
> if (ctrl->dmrsl && ctrl->dmrsl <= nvme_sect_to_lba(head, UINT_MAX))
> ctrl->max_discard_sectors =
> @@ -1741,8 +1740,6 @@ static void nvme_config_discard(struct nvme_ctrl *ctrl, struct gendisk *disk,
> BUILD_BUG_ON(PAGE_SIZE / sizeof(struct nvme_dsm_range) <
> NVME_DSM_MAX_RANGES);
>
> - queue->limits.discard_granularity = size;
> -
> /*
> * If discard is already enabled, don't reset queue limits.
> *
> @@ -1755,6 +1752,7 @@ static void nvme_config_discard(struct nvme_ctrl *ctrl, struct gendisk *disk,
>
> blk_queue_max_discard_sectors(queue, ctrl->max_discard_sectors);
> blk_queue_max_discard_segments(queue, ctrl->max_discard_segments);
> + queue->limits.discard_granularity = queue_logical_block_size(queue);
>
> if (ctrl->quirks & NVME_QUIRK_DEALLOCATE_ZEROES)
> blk_queue_max_write_zeroes_sectors(queue, UINT_MAX);

maybe worth to add a small helper like we have for other discard limits:

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 0046b447268f..b7eee29ca8ec 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -369,6 +369,17 @@ void blk_queue_zone_write_granularity(struct
request_queue *q,
}
EXPORT_SYMBOL_GPL(blk_queue_zone_write_granularity);

+/**
+ * blk_queue_discard_granularity - set discard granularity for the queue
+ * @q: the request queue for the device
+ * @size: the discard granularity size, in bytes
+ */
+void blk_queue_discard_granularity(struct request_queue *q, unsigned
int size)
+{
+ q->limits.discard_granularity = size;
+}
+EXPORT_SYMBOL_GPL(blk_queue_discard_granularity);
+
/**
* blk_queue_alignment_offset - set physical block alignment offset
* @q: the request queue for the device

otherwise looks good,
Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>