On Tue, 05 Aug 2008 02:45:16 +0100 David Woodhouse <dwmw2@xxxxxxxxxxxxx> wrote: > On Sat, 2008-07-26 at 13:02 -0700, Andrew Morton wrote: > > I seem to be hearing a lot of silence over support for SSD devices. I > > have this vague worry that there will be a large rollout of SSD > > hardware and Linux will be found to have pants-around-ankles. > > > > For example, support for the T13 Trim command. There will be others, > > but I surely don't know what they are. > > Here's a first attempt at that. It implements basic support for > 'discard' requests in the block layer, implements that in FTL (the flash > translation layer used on PCMCIA flash cards), and in FAT. Looks nice and simple. > > ... > > +int blkdev_issue_discard(struct block_device *bdev, sector_t sector, > + unsigned nr_sects, sector_t *error_sector) > +{ > + DECLARE_COMPLETION_ONSTACK(wait); > + struct request_queue *q; > + struct bio *bio; > + int ret; > + > + if (bdev->bd_disk == NULL) > + return -ENXIO; > + > + q = bdev_get_queue(bdev); > + if (!q) > + return -ENXIO; > + > + bio = bio_alloc(GFP_KERNEL, 0); > + if (!bio) > + return -ENOMEM; > + > + bio->bi_end_io = bio_end_empty_barrier; > + bio->bi_private = &wait; > + bio->bi_bdev = bdev; > + bio->bi_sector = sector; > + bio->bi_size = nr_sects << 9; > + submit_bio(1 << BIO_RW_DISCARD, bio); > + > + wait_for_completion(&wait); The synchronous nature might get pretty painful, even after merging is working? err, actually, it'll have to become async to be able to merge, won't it? > + /* > + * The driver must store the error location in ->bi_sector, if > + * it supports it. For non-stacked drivers, this should be copied > + * from rq->sector. > + */ > + if (error_sector) > + *error_sector = bio->bi_sector; > + > + ret = 0; > + if (bio_flagged(bio, BIO_EOPNOTSUPP)) > + ret = -EOPNOTSUPP; > + else if (!bio_flagged(bio, BIO_UPTODATE)) > + ret = -EIO; > + > + bio_put(bio); > + return ret; > +} > +EXPORT_SYMBOL(blkdev_issue_discard); > > ... > > @@ -1411,6 +1421,10 @@ end_io: > err = -EOPNOTSUPP; > goto end_io; > } > + if (bio_discard(bio) && !q->discard_fn) { > + err = -EOPNOTSUPP; > + goto end_io; > + } > > ret = q->make_request_fn(q, bio); > } while (ret); > @@ -1488,7 +1502,7 @@ void submit_bio(int rw, struct bio *bio) > * If it's a regular read/write or a barrier with data attached, > * go through the normal accounting stuff before submission. > */ > - if (!bio_empty_barrier(bio)) { > + if (!bio_empty_barrier(bio) && !bio_discard(bio)) { Perhaps we should have a new bio_doesnt_have_any_data_in_it() helper, rather than opn-coding it here? Assuming that new non-data-containing bios will turn up in the future. > > BIO_BUG_ON(!bio->bi_size); > BIO_BUG_ON(!bio->bi_io_vec); > @@ -1562,13 +1576,12 @@ static int __end_that_request_first(struct request *req, int error, > int nbytes; > > /* > - * For an empty barrier request, the low level driver must > - * store a potential error location in ->sector. We pass > - * that back up in ->bi_sector. > + * For an empty barrier request or sector discard request, the > + * low level driver must store a potential error location in > + * ->sector. We pass that back up in ->bi_sector. > */ > - if (blk_empty_barrier(req)) > + if (blk_empty_barrier(req) || blk_discard_rq(req)) Maybe ditto. > bio->bi_sector = req->sector; > - > if (nr_bytes >= bio->bi_size) { > req->bio = bio->bi_next; > nbytes = bio->bi_size; > > ... > > > +static int ftl_discardsect(struct mtd_blktrans_dev *dev, > + unsigned long sector, unsigned nr_sects) > +{ > + partition_t *part = (void *)dev; wuh? We're casting a `struct mtd_blktrans_ops *' into a partition_t? -- 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