* [PATCH] rbd: set discard alignment to zero
@ 2017-11-01 12:24 David Disseldorp
2017-11-01 17:14 ` Ilya Dryomov
0 siblings, 1 reply; 5+ messages in thread
From: David Disseldorp @ 2017-11-01 12:24 UTC (permalink / raw)
To: ceph-devel; +Cc: Mike Christie, Ilya Dryomov, David Disseldorp
RBD devices are currently incorrectly initialised with the block queue
discard_alignment set to the underlying RADOS object size.
As per Documentation/ABI/testing/sysfs-block:
The discard_alignment parameter indicates how many bytes the beginning
of the partition is offset from the internal allocation unit's natural
alignment.
Correcting the discard_alignment parameter from the RADOS object size 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 the UNMAP granularity alignment value advertised
to SCSI initiators via the Block Limits VPD.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
drivers/block/rbd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index b640ad8a6d20..cd4c65843f77 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -4423,7 +4423,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
/* enable the discard support */
queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
q->limits.discard_granularity = segment_size;
- q->limits.discard_alignment = segment_size;
+ q->limits.discard_alignment = 0;
blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE);
blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE);
--
2.13.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] rbd: set discard alignment to zero
2017-11-01 12:24 [PATCH] rbd: set discard alignment to zero David Disseldorp
@ 2017-11-01 17:14 ` Ilya Dryomov
2017-11-01 23:43 ` David Disseldorp
0 siblings, 1 reply; 5+ messages in thread
From: Ilya Dryomov @ 2017-11-01 17:14 UTC (permalink / raw)
To: David Disseldorp; +Cc: Ceph Development, Mike Christie
On Wed, Nov 1, 2017 at 1:24 PM, David Disseldorp <ddiss@suse.de> wrote:
> RBD devices are currently incorrectly initialised with the block queue
> discard_alignment set to the underlying RADOS object size.
>
> As per Documentation/ABI/testing/sysfs-block:
> The discard_alignment parameter indicates how many bytes the beginning
> of the partition is offset from the internal allocation unit's natural
> alignment.
>
> Correcting the discard_alignment parameter from the RADOS object size 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 the UNMAP granularity alignment value advertised
> to SCSI initiators via the Block Limits VPD.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> ---
> drivers/block/rbd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index b640ad8a6d20..cd4c65843f77 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -4423,7 +4423,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
> /* enable the discard support */
> queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
> q->limits.discard_granularity = segment_size;
> - q->limits.discard_alignment = segment_size;
> + q->limits.discard_alignment = 0;
> blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE);
> blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE);
Hi David,
Yeah, discard_alignment == discard_granularity seems bogus. The name
is misleading too, but that probably comes from SBC, which talks about
OPTIMAL UNMAP GRANULARITY and UNMAP GRANULARITY ALIGNMENT.
0 is the default -- let's just drop discard_alignment assignment?
Also, you quoted <disk>/<partition>/discard_alignment description, but
this is about <disk>/discard_alignment. This had me confused for
a moment.
Thanks,
Ilya
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] rbd: set discard alignment to zero
2017-11-01 17:14 ` Ilya Dryomov
@ 2017-11-01 23:43 ` David Disseldorp
2017-11-02 0:05 ` [PATCH v2] " David Disseldorp
0 siblings, 1 reply; 5+ messages in thread
From: David Disseldorp @ 2017-11-01 23:43 UTC (permalink / raw)
To: Ilya Dryomov; +Cc: Ceph Development, Mike Christie
Thanks for the feedback, Ilya.
On Wed, 1 Nov 2017 18:14:04 +0100, Ilya Dryomov wrote:
> Yeah, discard_alignment == discard_granularity seems bogus. The name
> is misleading too, but that probably comes from SBC, which talks about
> OPTIMAL UNMAP GRANULARITY and UNMAP GRANULARITY ALIGNMENT.
>
> 0 is the default -- let's just drop discard_alignment assignment?
Works for me, if you'd prefer. I'll send through a v2 with this.
> Also, you quoted <disk>/<partition>/discard_alignment description, but
> this is about <disk>/discard_alignment. This had me confused for
> a moment.
Oops, my bad. Will use the <disk> only description in the next round.
Cheers, David
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] rbd: set discard alignment to zero
2017-11-01 23:43 ` David Disseldorp
@ 2017-11-02 0:05 ` David Disseldorp
2017-11-02 8:53 ` Ilya Dryomov
0 siblings, 1 reply; 5+ messages in thread
From: David Disseldorp @ 2017-11-02 0:05 UTC (permalink / raw)
To: ceph-devel; +Cc: Mike Christie, Ilya Dryomov, David Disseldorp
RBD devices are currently incorrectly initialised with the block queue
discard_alignment set to the underlying RADOS object size.
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 from the RADOS object size to
zero (the blk_set_default_limits() default) has no effect on how discard
requests are propagated through the block layer - @alignment in
__blkdev_issue_discard() remains zero. However, it does fix the UNMAP
granularity alignment value advertised to SCSI initiators via the Block
Limits VPD.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
drivers/block/rbd.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index b640ad8a6d20..016542c75e1f 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -4423,7 +4423,6 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
/* enable the discard support */
queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
q->limits.discard_granularity = segment_size;
- q->limits.discard_alignment = segment_size;
blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE);
blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE);
--
2.13.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] rbd: set discard alignment to zero
2017-11-02 0:05 ` [PATCH v2] " David Disseldorp
@ 2017-11-02 8:53 ` Ilya Dryomov
0 siblings, 0 replies; 5+ messages in thread
From: Ilya Dryomov @ 2017-11-02 8:53 UTC (permalink / raw)
To: David Disseldorp; +Cc: Ceph Development, Mike Christie
On Thu, Nov 2, 2017 at 1:05 AM, David Disseldorp <ddiss@suse.de> wrote:
> RBD devices are currently incorrectly initialised with the block queue
> discard_alignment set to the underlying RADOS object size.
>
> 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 from the RADOS object size to
> zero (the blk_set_default_limits() default) has no effect on how discard
> requests are propagated through the block layer - @alignment in
> __blkdev_issue_discard() remains zero. However, it does fix the UNMAP
> granularity alignment value advertised to SCSI initiators via the Block
> Limits VPD.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> ---
> drivers/block/rbd.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index b640ad8a6d20..016542c75e1f 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -4423,7 +4423,6 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
> /* enable the discard support */
> queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
> q->limits.discard_granularity = segment_size;
> - q->limits.discard_alignment = segment_size;
> blk_queue_max_discard_sectors(q, segment_size / SECTOR_SIZE);
> blk_queue_max_write_zeroes_sectors(q, segment_size / SECTOR_SIZE);
LGTM, applied.
Thanks,
Ilya
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-02 8:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-01 12:24 [PATCH] rbd: set discard alignment to zero David Disseldorp
2017-11-01 17:14 ` Ilya Dryomov
2017-11-01 23:43 ` David Disseldorp
2017-11-02 0:05 ` [PATCH v2] " David Disseldorp
2017-11-02 8:53 ` Ilya Dryomov
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.