rbd: clean up rbd_rq_fn()

Some refactoring to improve readability. -Alex

[PATCH 1/2] rbd: encapsulate handling for a single request [PATCH 2/2] rbd: a little more cleanup of rbd_rq_fn()

In rbd_rq_fn(), requests are fetched from the block layer and each request is processed, looping through the request's list of bio's until they've all been consumed.

Separate the handling for a single request into its own function to make it a bit easier to see what's going on.

---
 drivers/block/rbd.c |  119
+++++++++++++++++++++++++++------------------------
 1 file changed, 63 insertions(+), 56 deletions(-)

index be18b5f..6aed59b 100644
@@ -1585,6 +1585,64 @@ static struct rbd_req_coll *rbd_alloc_coll(int

num_reqs)
return coll;
}

+static int rbd_dev_do_request(struct request *rq,
+ struct rbd_device *rbd_dev,
+ struct ceph_snap_context *snapc,
+ u64 ofs, unsigned int size,
+ struct bio *bio_chain)
+{
+ int num_segs;
+ struct rbd_req_coll *coll;
+ unsigned int bio_offset;
+ int cur_seg = 0;
+
+ dout("%s 0x%x bytes at 0x%llx\n",
+ rq_data_dir(rq) == WRITE ? "write" : "read",
+ size, (unsigned long long) blk_rq_pos(rq) * SECTOR_SIZE);
+
+ num_segs = rbd_get_num_segments(&rbd_dev->header, ofs, size);
+ if (num_segs <= 0)
+ return num_segs;
+
+ coll = rbd_alloc_coll(num_segs);
+ if (!coll)
+ return -ENOMEM;
+
+ bio_offset = 0;
+ do {
+ u64 limit = rbd_segment_length(rbd_dev, ofs, size);
+ unsigned int clone_size;
+ struct bio *bio_clone;
+
+ BUG_ON(limit > (u64) UINT_MAX);
+ clone_size = (unsigned int) limit;
+ dout("bio_chain->bi_vcnt=%hu\n", bio_chain->bi_vcnt);
+
+ kref_get(&coll->kref);
+
+ /* Pass a cloned bio chain via an osd request */
+
+ bio_clone = bio_chain_clone_range(&bio_chain,
+ &bio_offset, clone_size,
+ GFP_ATOMIC);
+ if (bio_clone)
+ (void) rbd_do_op(rq, rbd_dev, snapc,
+ ofs, clone_size,
+ bio_clone, coll, cur_seg);
+ else
+ rbd_coll_end_req_index(rq, coll, cur_seg,
+ (s32) -ENOMEM,
+ clone_size);
+ size -= clone_size;
+ ofs += clone_size;
+
+ cur_seg++;
+ } while (size > 0);
+ kref_put(&coll->kref, rbd_coll_release);
+
+ return 0;
+}
+
/*
* block device queue callback
*/
@@ -1598,10 +1656,8 @@ static void rbd_rq_fn(struct request_queue *q)
bool do_write;
unsigned int size;
u64 ofs;
- int num_segs, cur_seg = 0;
- struct rbd_req_coll *coll;
struct ceph_snap_context *snapc;
- unsigned int bio_offset;
+ int result;

dout("fetched request\n");

@@ -1639,60 +1695,11 @@ static void rbd_rq_fn(struct request_queue *q)
ofs = blk_rq_pos(rq) * SECTOR_SIZE;
bio = rq->bio;

- dout("%s 0x%x bytes at 0x%llx\n",
- do_write ? "write" : "read",
- size, (unsigned long long) blk_rq_pos(rq) * SECTOR_SIZE);
-
- num_segs = rbd_get_num_segments(&rbd_dev->header, ofs, size);
- if (num_segs <= 0) {
- spin_lock_irq(q->queue_lock);
- __blk_end_request_all(rq, num_segs);
- ceph_put_snap_context(snapc);
- continue;
- }
- coll = rbd_alloc_coll(num_segs);
- if (!coll) {
- spin_lock_irq(q->queue_lock);
- __blk_end_request_all(rq, -ENOMEM);
- ceph_put_snap_context(snapc);
- continue;
- }
-
- bio_offset = 0;
- do {
- u64 limit = rbd_segment_length(rbd_dev, ofs, size);
- unsigned int chain_size;
- struct bio *bio_chain;
-
- BUG_ON(limit > (u64) UINT_MAX);
- chain_size = (unsigned int) limit;
- dout("rq->bio->bi_vcnt=%hu\n", rq->bio->bi_vcnt);
-
- kref_get(&coll->kref);
-
- /* Pass a cloned bio chain via an osd request */
-
- bio_chain = bio_chain_clone_range(&bio,
- &bio_offset, chain_size,
- GFP_ATOMIC);
- if (bio_chain)
- (void) rbd_do_op(rq, rbd_dev, snapc,
- ofs, chain_size,
- bio_chain, coll, cur_seg);
- else
- rbd_coll_end_req_index(rq, coll, cur_seg,
- (s32) -ENOMEM,
- chain_size);
- size -= chain_size;
- ofs += chain_size;
-
- cur_seg++;
- } while (size > 0);
- kref_put(&coll->kref, rbd_coll_release);
-
- spin_lock_irq(q->queue_lock);
-
+ result = rbd_dev_do_request(rq, rbd_dev, snapc, ofs, size, bio);
ceph_put_snap_context(snapc);
+ spin_lock_irq(q->queue_lock);
+ if (result < 0)
+ __blk_end_request_all(rq, result);
}
}

1.7.9.5

Now that a big hunk in the middle of rbd_rq_fn() has been moved into its own routine we can simplify it a little more.

---
 drivers/block/rbd.c |   50
+++++++++++++++++++++++---------------------------
 1 file changed, 23 insertions(+), 27 deletions(-)

index 6aed59b..fb727c0 100644
@@ -1649,53 +1649,49 @@ static int rbd_dev_do_request(struct request *rq,
 static void rbd_rq_fn(struct request_queue *q)
 {
 	struct rbd_device *rbd_dev = q->queuedata;
+	bool read_only = rbd_dev->mapping.read_only;
 	struct request *rq;
 
 	while ((rq = blk_fetch_request(q))) {
-		struct bio *bio;
-		bool do_write;
-		unsigned int size;
-		u64 ofs;
-		struct ceph_snap_context *snapc;
+		struct ceph_snap_context *snapc = NULL;
 		int result;
 
 		dout("fetched request\n");
 
-		/* filter out block requests we don't understand */
+		/* Filter out block requests we don't understand */
+
 		if ((rq->cmd_type != REQ_TYPE_FS)) {
 			__blk_end_request_all(rq, 0);
 			continue;
 		}
+		spin_unlock_irq(q->queue_lock);
 
-		/* deduce our operation (read, write) */
-		do_write = (rq_data_dir(rq) == WRITE);
-		if (do_write && rbd_dev->mapping.read_only) {
-			__blk_end_request_all(rq, -EROFS);
-			continue;
-		}
+		/* Stop writes to a read-only device */
 
-		spin_unlock_irq(q->queue_lock);
+		result = -EROFS;
+		if (read_only && rq_data_dir(rq) == WRITE)
+			goto out_end_request;
+
+		/* Grab a reference to the snapshot context */
 
 		down_read(&rbd_dev->header_rwsem);
+		if (rbd_dev->exists) {
+			snapc = ceph_get_snap_context(rbd_dev->header.snapc);
+			rbd_assert(snapc != NULL);
+		}
+		up_read(&rbd_dev->header_rwsem);
 
-		if (!rbd_dev->exists) {
+		if (!snapc) {
 			rbd_assert(rbd_dev->spec->snap_id != CEPH_NOSNAP);
-			up_read(&rbd_dev->header_rwsem);
 			dout("request for non-existent snapshot");
-			spin_lock_irq(q->queue_lock);
-			__blk_end_request_all(rq, -ENXIO);
-			continue;
+			result = -ENXIO;
+			goto out_end_request;
 		}
 
-		snapc = ceph_get_snap_context(rbd_dev->header.snapc);
-
-		up_read(&rbd_dev->header_rwsem);
-
-		size = blk_rq_bytes(rq);
-		ofs = blk_rq_pos(rq) * SECTOR_SIZE;
-		bio = rq->bio;
-
-		result = rbd_dev_do_request(rq, rbd_dev, snapc, ofs, size, bio);
+		result = rbd_dev_do_request(rq, rbd_dev, snapc,
+				blk_rq_pos(rq) * SECTOR_SIZE,
+				blk_rq_bytes(rq), rq->bio);
+out_end_request:
 		ceph_put_snap_context(snapc);
 		spin_lock_irq(q->queue_lock);
 		if (result < 0)

1.7.9.5