From: "Michael S. Tsirkin" <mst@redhat.com> To: Asias He <asias@redhat.com> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Rusty Russell <rusty@rustcorp.com.au>, Christoph Hellwig <hch@lst.de>, Minchan Kim <minchan.kim@gmail.com> Subject: Re: [PATCH 3/3] virtio-blk: Add bio-based IO path for virtio-blk Date: Mon, 18 Jun 2012 13:21:08 +0300 [thread overview] Message-ID: <20120618102108.GD23134@redhat.com> (raw) In-Reply-To: <1340002390-3950-4-git-send-email-asias@redhat.com> On Mon, Jun 18, 2012 at 02:53:10PM +0800, Asias He wrote: > +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? > + virtblk_add_buf_wait(vblk, vbr, out, in); > + } else { > + virtqueue_kick(vblk->vq); Why special case the first call? task state manipulation so expensive? > + spin_unlock_irq(vblk->disk->queue->queue_lock); > + } > +} > +
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com> To: Asias He <asias@redhat.com> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Christoph Hellwig <hch@lst.de> Subject: Re: [PATCH 3/3] virtio-blk: Add bio-based IO path for virtio-blk Date: Mon, 18 Jun 2012 13:21:08 +0300 [thread overview] Message-ID: <20120618102108.GD23134@redhat.com> (raw) In-Reply-To: <1340002390-3950-4-git-send-email-asias@redhat.com> On Mon, Jun 18, 2012 at 02:53:10PM +0800, Asias He wrote: > +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? > + virtblk_add_buf_wait(vblk, vbr, out, in); > + } else { > + virtqueue_kick(vblk->vq); Why special case the first call? task state manipulation so expensive? > + spin_unlock_irq(vblk->disk->queue->queue_lock); > + } > +} > +
next prev parent reply other threads:[~2012-06-18 10:21 UTC|newest] Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-06-18 6:53 [PATCH v2 0/3] Improve virtio-blk performance Asias He 2012-06-18 6:53 ` Asias He 2012-06-18 6:53 ` [PATCH 1/3] block: Introduce __blk_segment_map_sg() helper Asias He 2012-06-18 6:53 ` Asias He 2012-06-18 21:31 ` Tejun Heo 2012-06-18 21:31 ` Tejun Heo 2012-06-19 2:02 ` Asias He 2012-06-19 2:02 ` Asias He 2012-06-19 18:00 ` Tejun Heo 2012-06-19 18:00 ` Tejun Heo 2012-06-18 6:53 ` [PATCH v2 2/3] block: Add blk_bio_map_sg() helper Asias He 2012-06-18 6:53 ` Asias He 2012-06-18 6:53 ` [PATCH 3/3] virtio-blk: Add bio-based IO path for virtio-blk Asias He 2012-06-18 6:53 ` Asias He 2012-06-18 7:46 ` Rusty Russell 2012-06-18 7:46 ` Rusty Russell 2012-06-18 8:03 ` Asias He 2012-06-18 8:03 ` Asias He 2012-06-18 10:05 ` Rusty Russell 2012-06-18 10:05 ` Rusty Russell 2012-06-18 11:14 ` Dor Laor 2012-06-18 11:14 ` Dor Laor 2012-06-18 11:39 ` Sasha Levin 2012-06-18 11:39 ` Sasha Levin 2012-06-19 2:51 ` Asias He 2012-06-19 2:51 ` Asias He 2012-06-19 6:21 ` Dor Laor 2012-06-19 6:21 ` Dor Laor 2012-06-20 4:46 ` Asias He 2012-06-20 4:46 ` Asias He 2012-06-21 9:49 ` Dor Laor 2012-06-21 9:49 ` Dor Laor 2012-07-01 23:54 ` Rusty Russell 2012-07-02 2:45 ` Asias He 2012-07-02 2:45 ` Asias He 2012-07-02 6:41 ` Rusty Russell 2012-07-02 6:41 ` Rusty Russell 2012-07-03 0:39 ` Asias He 2012-07-03 0:39 ` Asias He 2012-07-04 2:40 ` Rusty Russell 2012-07-04 2:40 ` Rusty Russell 2012-07-06 1:03 ` Asias He 2012-07-06 1:03 ` Asias He 2012-07-03 13:31 ` Paolo Bonzini 2012-07-03 13:31 ` Paolo Bonzini 2012-07-03 14:02 ` Asias He 2012-07-03 14:02 ` Asias He 2012-07-01 23:54 ` Rusty Russell 2012-07-03 14:22 ` Ronen Hod 2012-07-03 14:22 ` Ronen Hod 2012-07-03 14:28 ` Dor Laor 2012-07-03 14:28 ` Dor Laor 2012-07-04 14:10 ` Paolo Bonzini 2012-07-04 14:10 ` Paolo Bonzini 2012-06-18 21:28 ` Tejun Heo 2012-06-18 21:28 ` Tejun Heo 2012-06-19 2:39 ` Asias He 2012-06-19 2:39 ` Asias He 2012-06-18 10:13 ` Michael S. Tsirkin 2012-06-18 10:13 ` Michael S. Tsirkin 2012-06-19 2:21 ` Asias He 2012-06-19 2:21 ` Asias He 2012-06-18 9:37 ` Stefan Hajnoczi 2012-06-18 9:37 ` Stefan Hajnoczi 2012-06-18 10:21 ` Michael S. Tsirkin [this message] 2012-06-18 10:21 ` Michael S. Tsirkin 2012-06-18 10:45 ` Stefan Hajnoczi 2012-06-18 10:45 ` Stefan Hajnoczi 2012-06-19 2:30 ` Asias He 2012-06-19 2:30 ` Asias He 2012-06-18 9:14 ` [PATCH v2 0/3] Improve virtio-blk performance Stefan Hajnoczi 2012-06-18 9:14 ` Stefan Hajnoczi 2012-06-18 9:39 ` Asias He 2012-06-18 9:39 ` Asias He 2012-06-18 10:58 ` Stefan Hajnoczi 2012-06-18 10:58 ` Stefan Hajnoczi 2012-06-19 4:24 ` Asias He 2012-06-19 4:24 ` Asias He 2012-06-19 10:14 ` Stefan Hajnoczi 2012-06-19 10:14 ` Stefan Hajnoczi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20120618102108.GD23134@redhat.com \ --to=mst@redhat.com \ --cc=asias@redhat.com \ --cc=hch@lst.de \ --cc=kvm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=minchan.kim@gmail.com \ --cc=rusty@rustcorp.com.au \ --cc=virtualization@lists.linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.