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

Dor Laor
2007-08-22T09:26:35Z

>> +struct desc_pages
>> +{
>> + /* Page of descriptors. */
>> + struct lguest_desc desc[NUM_DESCS];
>> +
>> + /* Next page: how we tell other side what buffers are available.
>*/
>> + unsigned int avail_idx;
>> + unsigned int available[NUM_DESCS];
>> + char pad[PAGE_SIZE - (NUM_DESCS+1) * sizeof(unsigned int)];
>> +
>> + /* Third page: how other side tells us what's used. */
>> + unsigned int used_idx;
>> + struct lguest_used used[NUM_DESCS];
>> +};
>
>Please consider to add this patch to make this data structure work on
64
>bit
>to make the second page, really page aligned. On 32 bit this should be
a
>no-op.
>
>Signed-Off-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
>
>---
> drivers/lguest/lguest_virtio.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>Index: lguest/drivers/lguest/lguest_virtio.c
>===================================================================
>--- lguest.orig/drivers/lguest/lguest_virtio.c
>+++ lguest/drivers/lguest/lguest_virtio.c
>@@ -33,11 +33,12 @@ struct desc_pages
> {
> /* Page of descriptors. */
> struct lguest_desc desc[NUM_DESCS];
>+ char pad0[PAGE_SIZE - NUM_DESCS * sizeof(struct lguest_desc)];
>
> /* Next page: how we tell other side what buffers are available.
>*/
> unsigned int avail_idx;
> unsigned int available[NUM_DESCS];
>- char pad[PAGE_SIZE - (NUM_DESCS+1) * sizeof(unsigned int)];
>+ char pad1[PAGE_SIZE - (NUM_DESCS+1) * sizeof(unsigned int)];
>
> /* Third page: how other side tells us what's used. */
> unsigned int used_idx;

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.

--Dor