From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753980AbbDNJvL (ORCPT ); Tue, 14 Apr 2015 05:51:11 -0400 Received: from e06smtp12.uk.ibm.com ([195.75.94.108]:56010 "EHLO e06smtp12.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752329AbbDNJvA (ORCPT ); Tue, 14 Apr 2015 05:51:00 -0400 Date: Tue, 14 Apr 2015 11:50:53 +0200 From: Cornelia Huck To: "Michael S. Tsirkin" Cc: Rusty Russell , linux-kernel@vger.kernel.org, Pawel Moll , virtio-dev@lists.oasis-open.org, virtualization@lists.linux-foundation.org Subject: Re: [PATCH v3 0/6] virtio_balloon: virtio 1 support Message-ID: <20150414115053.78189c71.cornelia.huck@de.ibm.com> In-Reply-To: <20150414103036-mutt-send-email-mst@redhat.com> References: <1427884468-23930-1-git-send-email-mst@redhat.com> <20150412170141-mutt-send-email-mst@redhat.com> <87h9sjtsvb.fsf@rustcorp.com.au> <20150414102438.11d12347.cornelia.huck@de.ibm.com> <20150414103036-mutt-send-email-mst@redhat.com> Organization: IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz =?UTF-8?B?R2VzY2jDpGZ0c2bDvGhydW5nOg==?= Dirk Wittkopp Sitz der Gesellschaft: =?UTF-8?B?QsO2Ymxpbmdlbg==?= Registergericht: Amtsgericht Stuttgart, HRB 243294 X-Mailer: Claws Mail 3.8.0 (GTK+ 2.24.10; i686-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15041409-0009-0000-0000-000003CE5D63 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 14 Apr 2015 11:21:11 +0200 "Michael S. Tsirkin" wrote: > diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h > index f81b220..164e0c2 100644 > --- a/include/uapi/linux/virtio_balloon.h > +++ b/include/uapi/linux/virtio_balloon.h > @@ -52,15 +52,31 @@ struct virtio_balloon_config { > #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */ > #define VIRTIO_BALLOON_S_NR 6 > > +/* > + * Memory statistics structure. > + * Driver fills an array of these structures and passes to device. > + * > + * NOTE: fields are laid out in a way that would make compiler add padding > + * between and after fields, so we have to use compiler-specific attributes to > + * pack it, to disable this padding. This also often causes compiler to > + * generate suboptimal code. > + * > + * We maintain this for backwards compatibility, but 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_BALLOON_S_NR); > - if (virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1)) { > - vb->stats[idx].tag = cpu_to_le32(tag); > - vb->stats[idx].val = cpu_to_le64(val); > - } else { > - vb->legacy_stats[idx].tag = tag; > - vb->legacy_stats[idx].val = val; > - } > + vb->stats[idx].tag = cpu_to_virtio16(vb->vdev, tag); Seems that nobody seemed to care much about statistics... > + vb->stats[idx].val = cpu_to_virtio64(vb->vdev, val); > } > > #define pages_to_bytes(x) ((u64)(x) << PAGE_SHIFT) > With these changes merged in: Acked-by: Cornelia Huck