|
| From: | Avi Kivity |
| Subject: | Re: [Qemu-devel] [RFC 1/1] pci-dma-api-v2 |
| Date: | Tue, 02 Dec 2008 11:45:25 +0200 |
| User-agent: | Thunderbird 2.0.0.18 (X11/20081119) |
Anthony Liguori wrote:
What should be fixed? Are these emulated functions wrong?There's a lack of symmetry here. We should have a bdrv_readv and bdrv_aio_readv. bdrv_read and bdrv_aio_read should disappear. We can maintain wrappers that create a compatible interface for older code but just adding a new API is wrong.It was understood a real aio readv/writev was being worked on, so the emulation could be a temporary step.Yes, but that's orthogonal to what I'm saying here. I'm saying that instead of adding an optional ->aio_readv() member, we should eliminate the ->aio_read() member and replace it with ->aio_readv(). There are only three or four places in the code that implement aio so it's not a big change. It avoids introducing a new API too.
The api replacement would make a lot more sense when there's an implementation backing it up. I have no concrete objection to this though, maybe Andrea can whip up a follow on patch to kill aio_read.
I also think we should have complimentary synchronous vector functions although I'd like to see the synchronous API disappear completely.
I don't see a user for the synchronous ops. I think the sync ops are used only by the implementation of block formats, not by disk controller emulation, and these only use them to access metadata. Haven't checked this though.
struct iovec does not exist on Windows. I also don't think you've got the abstraction right. Reading and writing may require actions to happen. I don't think you can just introduce something as simple as an iovec here. I think we need something more active.Can you elaborate? Actions happen in the completion callback. This is just an straightforward extension of the block API, I don't think a dma api should materially change the block api.If we're not going to try and fold in the IOMMU/PCI BUS API at this pass, then this is less important. But to implement a proper PCI bus API, I think there has to be function pointers associated with each element of the scatter/gather list that control how data is copied in and out of each element.
Certainly not with each element, that would increase overhead for the common zero copy case. The way to do transformation is before/after bouncing, in the slow path.
The way I see it is:- transformations that only manipulate addresses, maintaining alignment requirements, can be done at the sg->iovec transform phase without any need for further manipulation - if there are alignment problems, we bounce, but again no need for further manipulation - transformations that manipulate data are bounced and manipulated on the bounce buffer
It can all be done within the dma api, no need to involve the block layer with callbacks. That's not its job.
-- error compiling committee.c: too many arguments to function
| [Prev in Thread] | Current Thread | [Next in Thread] |