Re: [PATCH 4/6] virtio block driver

On Thu, 2007-09-20 at 15:05 +0200, Jens Axboe wrote:

On Thu, Sep 20 2007, Rusty Russell wrote:

quoted

+static void end_dequeued_request(struct request *req,
+				 struct request_queue *q, int uptodate)
+{
+	/* And so the insanity of the block layer infects us here. */
+	int nsectors = req->hard_nr_sectors;
+
+	if (blk_pc_request(req)) {
+		nsectors = (req->data_len + 511) >> 9;
+		if (!nsectors)
+			nsectors = 1;
+	}
+	if (end_that_request_first(req, uptodate, nsectors))
+		BUG();
+	add_disk_randomness(req->rq_disk);
+	end_that_request_last(req, uptodate);
+}

We have end_queued_request(), lets add end_dequeued_request(). Below.

OK, thanks, I'll throw that in the mix and test...

quoted

+	vblk->sg[0].page = virt_to_page(&vbr->out_hdr);
+	vblk->sg[0].offset = offset_in_page(&vbr->out_hdr);
+	vblk->sg[0].length = sizeof(vbr->out_hdr);
+	num = blk_rq_map_sg(q, vbr->req, vblk->sg+1);

This wont work for chained sglists, but I gather (I'm so funny) that it
wont be an issue for you. How large are your sglists?

Hmm, potentially extremely large.  What do I need to do for chained
sglists?

quoted

+		if (!do_req(q, vblk, req)) {
+			/* Queue full?  Wait. */
+			blk_stop_queue(q);
+			break;
+		}

Personally I think this bool stuff is foul. You return false/true, but
still use ! to test. That is just more confusing than the canonical 0/1
good/bad return imho.

Except "0/1" is not canonical in the kernel.  Arguably, "-errno/0" is
canonical.  OTOH, bool is clear.

if do_req() fails, we assume the queue is full.  I shall change the
comment to that effect.

Thanks!
Rusty.