On Sat, Aug 16 2008, Pierre Ossman wrote:
> I've cooked up some patches that maps David's new discard requests to
> erase operations on MMC and SD cards. I'm not entirely sure these are
> something to keep though as I've been unable to see any performance
> increase in keeping blocks erased. Do we have any other reason to keep
> it?
>
> During development and test I noticed one issue though; the discard
> requests are chopped up into smaller pieces. As the erase commands have
> a granularity, sometimes this can mean that parts of the flash are
> missed because the discard request isn't split in alignment with the
> erase boundaries.
>
> For this reason, I propose that discard requests do not respect the
> regular queue restrictions. Those are generally for expressing
> limitations in the devices' DMA engines. Since the discard request
> carries no data, the DMA engine will never get involved.
I agree with that, the thought did cross my mind earlier as well. I
committed something like the below (in two patches). Do you want me to
queue up your mmc patches as well?
diff --git a/block/blk-core.c b/block/blk-core.c
index d785466..52824c0 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1381,7 +1381,8 @@ end_io:
break;
}
- if (unlikely(nr_sectors > q->max_hw_sectors)) {
+ if (unlikely(bio_has_data(bio) &&
+ nr_sectors > q->max_hw_sectors)) {
printk(KERN_ERR "bio too big device %s (%u > %u)\n",
bdevname(bio->bi_bdev, b),
bio_sectors(bio),
diff --git a/block/ioctl.c b/block/ioctl.c
index 375c579..ec6fa5d 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -125,52 +125,41 @@ static int blk_ioctl_discard(struct block_device *bdev, uint64_t start,
uint64_t len)
{
struct request_queue *q = bdev_get_queue(bdev);
+ DECLARE_COMPLETION_ONSTACK(wait);
+ struct bio *bio;
int ret = 0;
if (start & 511)
return -EINVAL;
if (len & 511)
return -EINVAL;
- start >>= 9;
- len >>= 9;
- if (start + len > (bdev->bd_inode->i_size >> 9))
+ if (start + len > bdev->bd_inode->i_size)
return -EINVAL;
if (!q->prepare_discard_fn)
return -EOPNOTSUPP;
- while (len && !ret) {
- DECLARE_COMPLETION_ONSTACK(wait);
- struct bio *bio;
-
- bio = bio_alloc(GFP_KERNEL, 0);
- if (!bio)
- return -ENOMEM;
-
- bio->bi_end_io = blk_ioc_discard_endio;
- bio->bi_bdev = bdev;
- bio->bi_private = &wait;
- bio->bi_sector = start;
-
- if (len > q->max_hw_sectors) {
- bio->bi_size = q->max_hw_sectors << 9;
- len -= q->max_hw_sectors;
- start += q->max_hw_sectors;
- } else {
- bio->bi_size = len << 9;
- len = 0;
- }
- submit_bio(DISCARD_NOBARRIER, bio);
-
- wait_for_completion(&wait);
-
- if (bio_flagged(bio, BIO_EOPNOTSUPP))
- ret = -EOPNOTSUPP;
- else if (!bio_flagged(bio, BIO_UPTODATE))
- ret = -EIO;
- bio_put(bio);
- }
+ bio = bio_alloc(GFP_KERNEL, 0);
+ if (!bio)
+ return -ENOMEM;
+
+ bio->bi_end_io = blk_ioc_discard_endio;
+ bio->bi_bdev = bdev;
+ bio->bi_private = &wait;
+ bio->bi_sector = start >> 9;
+ bio->bi_size = len;
+
+ submit_bio(DISCARD_NOBARRIER, bio);
+
+ wait_for_completion(&wait);
+
+ if (bio_flagged(bio, BIO_EOPNOTSUPP))
+ ret = -EOPNOTSUPP;
+ else if (!bio_flagged(bio, BIO_UPTODATE))
+ ret = -EIO;
+
+ bio_put(bio);
return ret;
}
--
Jens Axboe
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
