Re: [PATCH RFC 3/3] Virtio draft III: example block driver

Rusty Russell
2007-06-19T06:27:46Z

On Mon, 2007-06-18 at 12:09 +0300, Avi Kivity wrote:
> Rusty Russell wrote:
> > On Sun, 2007-06-17 at 17:25 +0300, Avi Kivity wrote:
> >
> >> Rusty Russell wrote:
> >>
> >>> + /* Set up for reply. */
> >>> + vblk->sg[0].page = virt_to_page(&vbr->in_hdr);
> >>> + vblk->sg[0].offset = offset_in_page(&vbr->in_hdr);
> >>> + vblk->sg[0].length = sizeof(vbr->in_hdr);
> >>> + num = blk_rq_map_sg(q, vbr->req, vblk->sg+1);
> >>> + vbr->out_hdr.id = vblk->vdev->ops->add_inbuf(vblk->vdev, vblk->sg,
> >>> + 1+num, vbr);
> >>> + if (IS_ERR_VALUE(vbr->out_hdr.id))
> >>> + goto full;
> >>> +
> >>> + 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);
> >>> +
> >>> + vbr->out_id = vblk->vdev->ops->add_outbuf(vblk->vdev, vblk->sg, 1,
> >>> + vbr);
> >>>
> >> This strikes me as wasteful. Why not set up a single descriptor which
> >> contains both placement and the data itself?
> >>
> >
> > We could actually do this for write, but not for read (where the length
> > & sector need to be sent to other end, and the data comes from other
> > end).
>
> So you map the first sg entry for output, and the rest for input.
>
> Less pretty, but more efficient.

At the moment there are two kinds of sgs: input and output. You're
proposing instead that we have a single kind, and a flag on each segment
which indicates direction. That may make block a little efficient,
depending on particular implementation of virtio.

Yet network really wants in and out separate. I'm not even sure how
we'd implement a mixed-sg scheme efficiently for that case. At the
moment it's very straightforward.

Of course, I'm carrying two implementations and two drivers, so it's
natural for me to try to resist changes 8)

Cheers,
Rusty.