Re: [PATCH 00/10] PV-IO v3

[Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>]

On Wed, 2007-08-22 at 02:26 -0700, Dor Laor wrote:
> Actually while playing with virtio for kvm Avi saw that and recommended
> to do the following:
> struct desc_pages
> {
> 	/* Page of descriptors. */
> 	union {
> 		struct virtio_desc desc[NUM_DESCS];
> 		char pad1[PAGE_SIZE];
> 	};
> 
> 	/* Next page: how we tell other side what buffers are available. */
> 	union {
> 		struct {
> 			unsigned int avail_idx;
> 			unsigned int available[NUM_DESCS];
> 		};
> 		char pad2[PAGE_SIZE];
> 	};
> 
> 	/* Third page: how other side tells us what's used. */
> 	union {
> 		struct {
> 			unsigned int used_idx;
> 			struct virtio_used used[NUM_DESCS];
> 		};
> 		char pad3[PAGE_SIZE];
> 	};
> };
> 
> It saves useless pointer arithmetic.

Hi Dor!

Please consider moving the "used" field into the descriptor (maybe as a
ptr for cache reasons, 'cept I'd really like to trim descriptor size).
That makes the avail and used rings identical, plus the current model
means we can't trust the used length if we don't trust the other side
(this is one of my FIXMEs).

Of course, we could go further and break the fixed structure: there's no
reason for the first and second part to be on separate pages, nor for
the third to be consecutive.  But there is no need until be have an
untrusted demonstration...

Cheers,
Rusty.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/