Re: [PATCH 1/5] [BLOCK] Add 'discard' request handling

[Jens Axboe <jens.axboe@oracle.com>]

On Fri, Aug 08 2008, Jens Axboe wrote:
> On Fri, Aug 08 2008, David Woodhouse wrote:
> > On Thu, 2008-08-07 at 20:41 +0200, Jens Axboe wrote:
> > > I've queued this up for some testing, I'll add the merge support as
> > > well.
> > 
> > Thanks. Did you pull from my tree? If so I'll provide an incremental
> > patch. Otherwise I'll go back and recommit it with your suggested
> > changes.
> 
> I did not, the final version will be different from the 1st one, so no
> point in carrying that history. What I did this morning was add the
> bio_has_data() support bits, here:

This is what I came up with. You don't need the request discard flag,
since you are using a specific request type for this. Additionally, we
can move the request init where it is done for fs requests
(init_req_from_bio) and get it set there. I also added the small bits
for merging. Nothing tested, so it may blow up :-)

diff --git a/block/blk-barrier.c b/block/blk-barrier.c
index a09ead1..1f58063 100644
--- a/block/blk-barrier.c
+++ b/block/blk-barrier.c
@@ -315,3 +315,56 @@ int blkdev_issue_flush(struct block_device *bdev, sector_t *error_sector)
 	return ret;
 }
 EXPORT_SYMBOL(blkdev_issue_flush);
+
+static void bio_discard_end_io(struct bio *bio, int err)
+{
+	bio_put(bio);
+}
+
+/**
+ * blkdev_issue_discard - queue a discard
+ * @bdev:	blockdev to issue discard for
+ * @sector:	start sector
+ * @nr_sects:	number of sectors to discard
+ * @end_io:	end_io function (or %NULL)
+ *
+ * Description:
+ *    Issue a discard request for the sectors in question. Caller can pass an
+ *    end_io function (which must call bio_put()), if they really want to see
+ *    the outcome. Most callers probably won't, and can just pass %NULL.
+ */
+int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
+			 unsigned nr_sects, bio_end_io_t end_io)
+{
+	struct request_queue *q;
+	struct bio *bio;
+
+	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;
+
+	/*
+	 * Many callers won't care at all about the outcome. After all,
+	 * this is just a hint to the underlying device. They'll just
+	 * ignore errors completely. So the end_io function can be just
+	 * a call to bio_put()
+	 */
+	if (end_io)
+		bio->bi_end_io = end_io;
+	else
+		bio->bi_end_io = bio_discard_end_io;
+
+	bio->bi_bdev = bdev;
+	bio->bi_sector = sector;
+	bio->bi_size = nr_sects << 9;
+	submit_bio(1 << BIO_RW_DISCARD, bio);
+	return 0;
+}
+EXPORT_SYMBOL(blkdev_issue_discard);
diff --git a/block/blk-core.c b/block/blk-core.c
index ff7ec49..f1344ea 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1068,7 +1068,10 @@ EXPORT_SYMBOL(blk_put_request);
 
 void init_request_from_bio(struct request *req, struct bio *bio)
 {
-	req->cmd_type = REQ_TYPE_FS;
+	if (bio_discard(bio))
+		req->cmd_type = REQ_TYPE_DISCARD;
+	else
+		req->cmd_type = REQ_TYPE_FS;
 
 	/*
 	 * inherit FAILFAST from bio (for read-ahead, and explicit FAILFAST)
@@ -2013,12 +2016,15 @@ void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
 	/* first two bits are identical in rq->cmd_flags and bio->bi_rw */
 	rq->cmd_flags |= (bio->bi_rw & 3);
 
-	rq->nr_phys_segments = bio_phys_segments(q, bio);
-	rq->nr_hw_segments = bio_hw_segments(q, bio);
+	if (bio_has_data(bio)) {
+		rq->nr_phys_segments = bio_phys_segments(q, bio);
+		rq->nr_hw_segments = bio_hw_segments(q, bio);
+		rq->buffer = bio_data(bio);
+	}
+
 	rq->current_nr_sectors = bio_cur_sectors(bio);
 	rq->hard_cur_sectors = rq->current_nr_sectors;
 	rq->hard_nr_sectors = rq->nr_sectors = bio_sectors(bio);
-	rq->buffer = bio_data(bio);
 	rq->data_len = bio->bi_size;
 
 	rq->bio = rq->biotail = bio;
diff --git a/block/elevator.c b/block/elevator.c
index ed6f8f3..e704086 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -75,6 +75,13 @@ int elv_rq_merge_ok(struct request *rq, struct bio *bio)
 		return 0;
 
 	/*
+	 * Don't merge file system and non-file system requests
+	 */
+	if ((!bio_has_data(bio) && bio_has_data(rq->bio)) ||
+	    (bio_has_data(bio) && !bio_has_data(rq->bio)))
+		return 0;
+
+	/*
 	 * different data direction or already started, don't merge
 	 */
 	if (bio_data_dir(bio) != rq_data_dir(rq))
@@ -607,7 +614,6 @@ void elv_insert(struct request_queue *q, struct request *rq, int where)
 		break;
 
 	case ELEVATOR_INSERT_SORT:
-		BUG_ON(!blk_fs_request(rq));
 		rq->cmd_flags |= REQ_SORTED;
 		q->nr_sorted++;
 		if (rq_mergeable(rq)) {
diff --git a/include/linux/bio.h b/include/linux/bio.h
index dbeb66f..8dbc6c5 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -149,6 +149,8 @@ struct bio {
  * bit 2 -- barrier
  * bit 3 -- fail fast, don't want low level driver retries
  * bit 4 -- synchronous I/O hint: the block layer will unplug immediately
+ * bit 5 -- metadata request
+ * bit 6 -- discard sectors request
  */
 #define BIO_RW		0
 #define BIO_RW_AHEAD	1
@@ -156,6 +158,7 @@ struct bio {
 #define BIO_RW_FAILFAST	3
 #define BIO_RW_SYNC	4
 #define BIO_RW_META	5
+#define BIO_RW_DISCARD	6
 
 /*
  * upper 16 bits of bi_rw define the io priority of this bio
@@ -186,6 +189,7 @@ struct bio {
 #define bio_rw_ahead(bio)	((bio)->bi_rw & (1 << BIO_RW_AHEAD))
 #define bio_rw_meta(bio)	((bio)->bi_rw & (1 << BIO_RW_META))
 #define bio_empty_barrier(bio)	(bio_barrier(bio) && !bio_has_data(bio))
+#define bio_discard(bio)	((bio)->bi_rw & (1 << BIO_RW_DISCARD))
 
 static inline unsigned int bio_cur_sectors(struct bio *bio)
 {
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e61f22b..73cd4da 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -56,6 +56,7 @@ enum rq_cmd_type_bits {
 	REQ_TYPE_PM_SHUTDOWN,		/* shutdown request */
 	REQ_TYPE_FLUSH,			/* flush request */
 	REQ_TYPE_SPECIAL,		/* driver defined type */
+	REQ_TYPE_DISCARD,		/* discard sectors request */
 	REQ_TYPE_LINUX_BLOCK,		/* generic block layer message */
 	/*
 	 * for ATA/ATAPI devices. this really doesn't belong here, ide should
@@ -522,6 +523,7 @@ enum {
 #define blk_pc_request(rq)	((rq)->cmd_type == REQ_TYPE_BLOCK_PC)
 #define blk_special_request(rq)	((rq)->cmd_type == REQ_TYPE_SPECIAL)
 #define blk_sense_request(rq)	((rq)->cmd_type == REQ_TYPE_SENSE)
+#define blk_discard_request(rq)	((rq)->cmd_type == REQ_TYPE_DISCARD)
 
 #define blk_noretry_request(rq)	((rq)->cmd_flags & REQ_FAILFAST)
 #define blk_rq_started(rq)	((rq)->cmd_flags & REQ_STARTED)
@@ -829,6 +831,8 @@ static inline struct request *blk_map_queue_find_tag(struct blk_queue_tag *bqt,
 }
 
 extern int blkdev_issue_flush(struct block_device *, sector_t *);
+extern int blkdev_issue_discard(struct block_device *, sector_t,
+				unsigned int, bio_end_io_t *);
 
 /*
 * command filter functions

-- 
Jens Axboe