Mostly a preparation for the next patch.

In principle this fixes an infinite loop if max_discard_sectors < granularity,
but that really shouldn't happen.

Cc: Jens Axboe <axboe@kernel.dk>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/blk-lib.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 2b461b4..16b06f6 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -44,6 +44,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
struct request_queue *q = bdev_get_queue(bdev);
int type = REQ_WRITE | REQ_DISCARD;
unsigned int max_discard_sectors;
+ unsigned int granularity;
struct bio_batch bb;
struct bio *bio;
int ret = 0;
@@ -54,18 +55,18 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
if (!blk_queue_discard(q))
return -EOPNOTSUPP;

+ /* Zero-sector (unknown) and one-sector granularities are the same. */
+ granularity = max(q->limits.discard_granularity >> 9, 1U);
+
/*
* Ensure that max_discard_sectors is of the proper
* granularity
*/
max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
+ max_discard_sectors = round_down(max_discard_sectors, granularity);
if (unlikely(!max_discard_sectors)) {
/* Avoid infinite loop below. Being cautious never hurts. */
return -EOPNOTSUPP;
- } else if (q->limits.discard_granularity) {
- unsigned int disc_sects = q->limits.discard_granularity >> 9;
-
- max_discard_sectors &= ~(disc_sects - 1);
}

if (flags & BLKDEV_DISCARD_SECURE) {
--
1.7.1


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Il 03/07/2012 04:49, Vivek Goyal ha scritto:
>> > + /* Zero-sector (unknown) and one-sector granularities are the same. */
>> > + granularity = max(q->limits.discard_granularity >> 9, 1U);
>> > +
>> > /*
>> > * Ensure that max_discard_sectors is of the proper
>> > * granularity
>> > */
>> > max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
>> > + max_discard_sectors = round_down(max_discard_sectors, granularity);
>> > if (unlikely(!max_discard_sectors)) {
>> > /* Avoid infinite loop below. Being cautious never hurts. */
>> > return -EOPNOTSUPP;
>> > - } else if (q->limits.discard_granularity) {
>> > - unsigned int disc_sects = q->limits.discard_granularity >> 9;
>> > -
>> > - max_discard_sectors &= ~(disc_sects - 1);
> This is kind of odd. If discard_granularity is zero, we assume that
> discards are supported and granularity is 1. But if max_discard_sectors
> is zero, we assume discards are disabled. Not sure if we should treat
> max_discard_sectors and discard_granularity in same way or not.

Yes, this keeps the same behavior as before. It is also the one that is
consistent with drivers/scsi/sd.c. sd_config_discard always sets
limits.discard_granularity and then uses limits.max_discard_sectors to
disable discards.

Paolo

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Mostly a preparation for the next patch.

In principle this fixes an infinite loop if max_discard_sectors < granularity,
but that really shouldn't happen.

Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/blk-lib.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 2b461b4..16b06f6 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -44,6 +44,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
struct request_queue *q = bdev_get_queue(bdev);
int type = REQ_WRITE | REQ_DISCARD;
unsigned int max_discard_sectors;
+ unsigned int granularity;
struct bio_batch bb;
struct bio *bio;
int ret = 0;
@@ -54,18 +55,18 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
if (!blk_queue_discard(q))
return -EOPNOTSUPP;

+ /* Zero-sector (unknown) and one-sector granularities are the same. */
+ granularity = max(q->limits.discard_granularity >> 9, 1U);
+
/*
* Ensure that max_discard_sectors is of the proper
* granularity
*/
max_discard_sectors = min(q->limits.max_discard_sectors, UINT_MAX >> 9);
+ max_discard_sectors = round_down(max_discard_sectors, granularity);
if (unlikely(!max_discard_sectors)) {
/* Avoid infinite loop below. Being cautious never hurts. */
return -EOPNOTSUPP;
- } else if (q->limits.discard_granularity) {
- unsigned int disc_sects = q->limits.discard_granularity >> 9;
-
- max_discard_sectors &= ~(disc_sects - 1);
}

if (flags & BLKDEV_DISCARD_SECURE) {
--
1.7.1


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel