Re: [PATCH 3/3] virtio-blk: Add bio-based IO path for virtio-blk

On Mon, Jun 18, 2012 at 11:21 AM, Michael S. Tsirkin [off-list ref] wrote:

On Mon, Jun 18, 2012 at 02:53:10PM +0800, Asias He wrote:

quoted

+static void virtblk_make_request(struct request_queue *q, struct bio *bio)
+{
+     struct virtio_blk *vblk = q->queuedata;
+     unsigned int num, out = 0, in = 0;
+     struct virtblk_req *vbr;
+
+     BUG_ON(bio->bi_phys_segments + 2 > vblk->sg_elems);
+     BUG_ON(bio->bi_rw & (REQ_FLUSH | REQ_FUA));
+
+     vbr = virtblk_alloc_req(vblk, GFP_NOIO);
+     if (!vbr) {
+             bio_endio(bio, -ENOMEM);
+             return;
+     }
+
+     vbr->bio = bio;
+     vbr->req = NULL;
+     vbr->out_hdr.type = 0;
+     vbr->out_hdr.sector = bio->bi_sector;
+     vbr->out_hdr.ioprio = bio_prio(bio);
+
+     sg_set_buf(&vbr->sg[out++], &vbr->out_hdr, sizeof(vbr->out_hdr));
+
+     num = blk_bio_map_sg(q, bio, vbr->sg + out);
+
+     sg_set_buf(&vbr->sg[num + out + in++], &vbr->status,
+                sizeof(vbr->status));
+
+     if (num) {
+             if (bio->bi_rw & REQ_WRITE) {
+                     vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
+                     out += num;
+             } else {
+                     vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
+                     in += num;
+             }
+     }
+
+     spin_lock_irq(vblk->disk->queue->queue_lock);
+     if (virtqueue_add_buf(vblk->vq, vbr->sg, out, in, vbr,
+                           GFP_ATOMIC) < 0) {
+             spin_unlock_irq(vblk->disk->queue->queue_lock);

Any implications of dropping lock like that?
E.g. for suspend. like we are still discussing with
unlocked kick?

Since we aquired the lock in this function there should be no problem.
 Whatever protects against vblk or vblk->disk disappearing upon
entering this function also protects after unlocking queue_lock.
Otherwise all .make_request_fn() functions would be broken.

I'd still like to understand the details though.

Stefan