[PATCH] nvme: set discard_alignment to zero

[PATCH] nvme: set discard_alignment to zero

[David Disseldorp]

Similar to 7c084289795b ("rbd: set discard_alignment to zero"), NVMe
devices are currently incorrectly initialised with the block queue
discard_alignment set to the NVMe stream alignment.

As per Documentation/ABI/testing/sysfs-block:
  The discard_alignment parameter indicates how many bytes the beginning
  of the device is offset from the internal allocation unit's natural
  alignment.

Correcting the discard_alignment parameter to zero has no effect on how
discard requests are propagated through the block layer - @alignment in
__blkdev_issue_discard() remains zero. However, it does fix other
consumers, such as LIO's Block Limits VPD response.

Signed-off-by: David Disseldorp <ddiss at suse.de>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 25da74d310d1..2e224ab5c340 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1287,7 +1287,7 @@ static void nvme_config_discard(struct nvme_ctrl *ctrl,
 	BUILD_BUG_ON(PAGE_SIZE / sizeof(struct nvme_dsm_range) <
 			NVME_DSM_MAX_RANGES);
 
-	queue->limits.discard_alignment = size;
+	queue->limits.discard_alignment = 0;
 	queue->limits.discard_granularity = size;
 
 	blk_queue_max_discard_sectors(queue, UINT_MAX);
-- 
2.13.6

[PATCH] nvme: set discard_alignment to zero

[Christoph Hellwig]

Adding Jens, who added this code, but it looks fine to me.

On Fri, Nov 24, 2017@04:30:53PM +0100, David Disseldorp wrote:
> Similar to 7c084289795b ("rbd: set discard_alignment to zero"), NVMe
> devices are currently incorrectly initialised with the block queue
> discard_alignment set to the NVMe stream alignment.
> 
> As per Documentation/ABI/testing/sysfs-block:
>   The discard_alignment parameter indicates how many bytes the beginning
>   of the device is offset from the internal allocation unit's natural
>   alignment.
> 
> Correcting the discard_alignment parameter to zero has no effect on how
> discard requests are propagated through the block layer - @alignment in
> __blkdev_issue_discard() remains zero. However, it does fix other
> consumers, such as LIO's Block Limits VPD response.
> 
> Signed-off-by: David Disseldorp <ddiss at suse.de>
> ---
>  drivers/nvme/host/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 25da74d310d1..2e224ab5c340 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1287,7 +1287,7 @@ static void nvme_config_discard(struct nvme_ctrl *ctrl,
>  	BUILD_BUG_ON(PAGE_SIZE / sizeof(struct nvme_dsm_range) <
>  			NVME_DSM_MAX_RANGES);
>  
> -	queue->limits.discard_alignment = size;
> +	queue->limits.discard_alignment = 0;
>  	queue->limits.discard_granularity = size;
>  
>  	blk_queue_max_discard_sectors(queue, UINT_MAX);
> -- 
> 2.13.6
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
---end quoted text---

[PATCH] nvme: set discard_alignment to zero

[Martin K. Petersen]

David,

> Correcting the discard_alignment parameter to zero has no effect on how
> discard requests are propagated through the block layer - @alignment in
> __blkdev_issue_discard() remains zero. However, it does fix other
> consumers, such as LIO's Block Limits VPD response.
>
> Signed-off-by: David Disseldorp <ddiss at suse.de>
> ---
>  drivers/nvme/host/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 25da74d310d1..2e224ab5c340 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1287,7 +1287,7 @@ static void nvme_config_discard(struct nvme_ctrl *ctrl,
>  	BUILD_BUG_ON(PAGE_SIZE / sizeof(struct nvme_dsm_range) <
>  			NVME_DSM_MAX_RANGES);
>  
> -	queue->limits.discard_alignment = size;
> +	queue->limits.discard_alignment = 0;
>  	queue->limits.discard_granularity = size;

This looks good to me.

The alignment parameter was really only there to handle early drives
which were deliberately misaligned internally to avoid performance
penalties on operating systems that started the first partition on LBA
63.

Reviewed-by: Martin K. Petersen <martin.petersen at oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH] nvme: set discard_alignment to zero

[Jens Axboe]

On 11/28/2017 05:38 AM, Christoph Hellwig wrote:
> Adding Jens, who added this code, but it looks fine to me.
> 
> On Fri, Nov 24, 2017@04:30:53PM +0100, David Disseldorp wrote:
>> Similar to 7c084289795b ("rbd: set discard_alignment to zero"), NVMe
>> devices are currently incorrectly initialised with the block queue
>> discard_alignment set to the NVMe stream alignment.
>>
>> As per Documentation/ABI/testing/sysfs-block:
>>   The discard_alignment parameter indicates how many bytes the beginning
>>   of the device is offset from the internal allocation unit's natural
>>   alignment.
>>
>> Correcting the discard_alignment parameter to zero has no effect on how
>> discard requests are propagated through the block layer - @alignment in
>> __blkdev_issue_discard() remains zero. However, it does fix other
>> consumers, such as LIO's Block Limits VPD response.
>>
>> Signed-off-by: David Disseldorp <ddiss at suse.de>
>> ---
>>  drivers/nvme/host/core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 25da74d310d1..2e224ab5c340 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -1287,7 +1287,7 @@ static void nvme_config_discard(struct nvme_ctrl *ctrl,
>>  	BUILD_BUG_ON(PAGE_SIZE / sizeof(struct nvme_dsm_range) <
>>  			NVME_DSM_MAX_RANGES);
>>  
>> -	queue->limits.discard_alignment = size;
>> +	queue->limits.discard_alignment = 0;
>>  	queue->limits.discard_granularity = size;
>>  
>>  	blk_queue_max_discard_sectors(queue, UINT_MAX);

Looks good to me.

-- 
Jens Axboe

[PATCH] nvme: set discard_alignment to zero

[Christoph Hellwig]

Thanks,

applied to nvme-4.15.