search for: virtio_balloon_stat

agl at linux.vnet.ibm.com wrote: > Here are the corresponding changes to the Linux virtio driver... > > virtio: Add memory statistics reporting to the balloon driver > > When using ballooning to manage overcommitted memory on a host, a system for > guests to communicate their memory usage to the host can provide information > that will minimize the impact

agl at linux.vnet.ibm.com wrote: > Here are the corresponding changes to the Linux virtio driver... > > virtio: Add memory statistics reporting to the balloon driver > > When using ballooning to manage overcommitted memory on a host, a system for > guests to communicate their memory usage to the host can provide information > that will minimize the impact

...t don't follow this example. s/this/the existing statistics structure/ > + * > + * Do something like the below instead: If you want to implement a similar structure, do... Just that nobody gets the idea that they are supposed to implement new balloon statistics ;) > + * struct virtio_balloon_stat { > + * __virtio16 tag; > + * __u8 reserved[6]; > + * __virtio64 val; > + * }; (...) > @@ -225,13 +219,8 @@ static inline void update_stat(struct virtio_balloon *vb, int idx, > u16 tag, u64 val) > { > BUG_ON(idx >= VIRTIO_BALLOO...

...t don't follow this example. s/this/the existing statistics structure/ > + * > + * Do something like the below instead: If you want to implement a similar structure, do... Just that nobody gets the idea that they are supposed to implement new balloon statistics ;) > + * struct virtio_balloon_stat { > + * __virtio16 tag; > + * __u8 reserved[6]; > + * __virtio64 val; > + * }; (...) > @@ -225,13 +219,8 @@ static inline void update_stat(struct virtio_balloon *vb, int idx, > u16 tag, u64 val) > { > BUG_ON(idx >= VIRTIO_BALLOO...

...dant. What else? non-existing? > > + * > > + * Do something like the below instead: > > If you want to implement a similar structure, do... > > Just that nobody gets the idea that they are supposed to implement new > balloon statistics ;) > > > + * struct virtio_balloon_stat { > > + * __virtio16 tag; > > + * __u8 reserved[6]; > > + * __virtio64 val; > > + * }; > > (...) > > > @@ -225,13 +219,8 @@ static inline void update_stat(struct virtio_balloon *vb, int idx, > > u16 tag, u64 val)...

...le this padding. This also often causes compiler to + * generate suboptimal code. + * + * We maintain this statistics structure format for backwards compatibility, + * but don't follow this example. + * + * If implementing a similar structure, do something like the below instead: + * struct virtio_balloon_stat { + * __virtio16 tag; + * __u8 reserved[6]; + * __virtio64 val; + * }; + * + * In other words, add explicit reserved fields to align field and + * structure boundaries at field size, avoiding compiler padding + * without the packed attribute. + */ struct virtio_balloon_...

...le this padding. This also often causes compiler to + * generate suboptimal code. + * + * We maintain this statistics structure format for backwards compatibility, + * but don't follow this example. + * + * If implementing a similar structure, do something like the below instead: + * struct virtio_balloon_stat { + * __virtio16 tag; + * __u8 reserved[6]; + * __virtio64 val; + * }; + * + * In other words, add explicit reserved fields to align field and + * structure boundaries at field size, avoiding compiler padding + * without the packed attribute. + */ struct virtio_balloon_...

...this historically: we require all parts (spec,qemu,linux) to be available, but don't create specific order between them. In particular, I'd strongly prefer not waiting until 4.2, that would interfere with putting virtio 1 out to use in the field. Since both Rusty and Cornelia are against virtio_balloon_stat_modern, I accept this as the majority decision. And switching over to __virtio tags found a bug, so I'm convinced now. ---> virtio_balloon: drop virtio_balloon_stat_modern Looks like we are better off sticking with the misaligned stat struct, to reduce the amount of virtio 1 specific code...

On Tue, 14 Apr 2015 10:42:56 +0930 Rusty Russell <rusty at rustcorp.com.au> wrote: > "Michael S. Tsirkin" <mst at redhat.com> writes: > > On Wed, Apr 01, 2015 at 02:57:35PM +0200, Michael S. Tsirkin wrote: > >> Virtio 1.0 doesn't include a modern balloon device. At some point we'll likely > >> define an incompatible interface with a

On Tue, 14 Apr 2015 10:42:56 +0930 Rusty Russell <rusty at rustcorp.com.au> wrote: > "Michael S. Tsirkin" <mst at redhat.com> writes: > > On Wed, Apr 01, 2015 at 02:57:35PM +0200, Michael S. Tsirkin wrote: > >> Virtio 1.0 doesn't include a modern balloon device. At some point we'll likely > >> define an incompatible interface with a

...> > + * >> > + * Do something like the below instead: >> >> If you want to implement a similar structure, do... >> >> Just that nobody gets the idea that they are supposed to implement new >> balloon statistics ;) >> >> > + * struct virtio_balloon_stat { >> > + * __virtio16 tag; >> > + * __u8 reserved[6]; >> > + * __virtio64 val; >> > + * }; >> >> (...) >> >> > @@ -225,13 +219,8 @@ static inline void update_stat(struct virtio_balloon *vb, int idx, >&gt...

...> > + * >> > + * Do something like the below instead: >> >> If you want to implement a similar structure, do... >> >> Just that nobody gets the idea that they are supposed to implement new >> balloon statistics ;) >> >> > + * struct virtio_balloon_stat { >> > + * __virtio16 tag; >> > + * __u8 reserved[6]; >> > + * __virtio64 val; >> > + * }; >> >> (...) >> >> > @@ -225,13 +219,8 @@ static inline void update_stat(struct virtio_balloon *vb, int idx, >&gt...

...> VirtIOBalloon *dev = VIRTIO_BALLOON(vdev); > f |= dev->host_features; > virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ); > + virtio_add_feature(&f, VIRTIO_BALLOON_F_GET_FREE_PAGES); > return f; > } > > @@ -372,6 +410,45 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) > VIRTIO_BALLOON_PFN_SHIFT); > } > > +static int virtio_balloon_free_pages(void *opaque, > + unsigned long *free_pages_bitmap, > +...

...e_vq, *deflate_vq, *stats_vq; /* Where the ballooning thread waits for config to change. */ wait_queue_head_t config_change; @@ -50,6 +50,9 @@ struct virtio_balloon /* The array of pfns we tell the Host about. */ unsigned int num_pfns; u32 pfns[256]; + + /* Memory statistics */ + struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR]; }; static struct virtio_device_id id_table[] = { @@ -155,6 +158,57 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num) } } +static inline void update_stat(struct virtio_balloon *vb, int idx, + __le16 tag, __le64 val) +{ + BUG_ON(idx >= VIRTI...

...e_vq, *deflate_vq, *stats_vq; /* Where the ballooning thread waits for config to change. */ wait_queue_head_t config_change; @@ -50,6 +50,9 @@ struct virtio_balloon /* The array of pfns we tell the Host about. */ unsigned int num_pfns; u32 pfns[256]; + + /* Memory statistics */ + struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR]; }; static struct virtio_device_id id_table[] = { @@ -155,6 +158,57 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num) } } +static inline void update_stat(struct virtio_balloon *vb, int idx, + __le16 tag, __le64 val) +{ + BUG_ON(idx >= VIRTI...

...LOON_S_SWAP_IN 0 /* Amount of memory swapped in */ @@ -51,9 +51,16 @@ struct virtio_balloon_config { #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */ #define VIRTIO_BALLOON_S_NR 6 +/* Legacy stat structure. We keep it around to avoid breaking old userspace. */ struct virtio_balloon_stat { __u16 tag; __u64 val; } __attribute__((packed)); +struct virtio_balloon_stat_modern { + __u8 reserved[6]; + __virtio16 tag; + __virtio64 val; +}; + #endif /* _LINUX_VIRTIO_BALLOON_H */ diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 6a356e3..574267f 10...

...LOON_S_SWAP_IN 0 /* Amount of memory swapped in */ @@ -51,9 +51,16 @@ struct virtio_balloon_config { #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */ #define VIRTIO_BALLOON_S_NR 6 +/* Legacy stat structure. We keep it around to avoid breaking old userspace. */ struct virtio_balloon_stat { __u16 tag; __u64 val; } __attribute__((packed)); +struct virtio_balloon_stat_modern { + __u8 reserved[6]; + __virtio16 tag; + __virtio64 val; +}; + #endif /* _LINUX_VIRTIO_BALLOON_H */ diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 6a356e3..574267f 10...

...n_config { /* Number of pages host wants Guest to give up. */ - __le32 num_pages; + __u32 num_pages; /* Number of pages we've actually got in balloon. */ - __le32 actual; + __u32 actual; }; #define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */ @@ -56,4 +57,10 @@ struct virtio_balloon_stat { __u64 val; } __attribute__((packed)); +struct virtio_balloon_stat_modern { + __le16 tag; + __u8 reserved[6]; + __le64 val; +}; + #endif /* _LINUX_VIRTIO_BALLOON_H */ diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 6a356e3..0583720 100644 --- a/drivers/vi...

...eatures(VirtIODevice *vdev, uint64_t f, VirtIOBalloon *dev = VIRTIO_BALLOON(vdev); f |= dev->host_features; virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ); + virtio_add_feature(&f, VIRTIO_BALLOON_F_GET_FREE_PAGES); return f; } @@ -372,6 +410,45 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) VIRTIO_BALLOON_PFN_SHIFT); } +static int virtio_balloon_free_pages(void *opaque, + unsigned long *free_pages_bitmap, + unsigned long *free_pages_...

...n_config { /* Number of pages host wants Guest to give up. */ - __le32 num_pages; + __u32 num_pages; /* Number of pages we've actually got in balloon. */ - __le32 actual; + __u32 actual; }; #define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */ @@ -56,4 +57,10 @@ struct virtio_balloon_stat { __u64 val; } __attribute__((packed)); +struct virtio_balloon_stat_modern { + __le16 tag; + __u8 reserved[6]; + __le64 val; +}; + #endif /* _LINUX_VIRTIO_BALLOON_H */ diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 6a356e3..0583720 100644 --- a/drivers/vi...