Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751406AbbDAEUu (ORCPT ); Wed, 1 Apr 2015 00:20:50 -0400 Received: from ozlabs.org ([103.22.144.67]:37233 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750848AbbDAEUs (ORCPT ); Wed, 1 Apr 2015 00:20:48 -0400 From: Rusty Russell To: "Michael S. Tsirkin" , linux-kernel@vger.kernel.org Cc: virtualization@lists.linux-foundation.org, linux-api@vger.kernel.org Subject: Re: [PATCH v2 1/6] virtio_balloon: transitional interface In-Reply-To: <1427831230-28044-2-git-send-email-mst@redhat.com> References: <1427831230-28044-1-git-send-email-mst@redhat.com> <1427831230-28044-2-git-send-email-mst@redhat.com> User-Agent: Notmuch/0.17 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) Date: Wed, 01 Apr 2015 14:17:23 +1030 Message-ID: <87mw2sy0fo.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2915 Lines: 85 "Michael S. Tsirkin" writes: > Virtio 1.0 doesn't include a modern balloon device. > But it's not a big change to support a transitional > balloon device: this has the advantage of supporting > existing drivers, transparently. You decided to fix the packed struct... > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c > index 6a356e3..574267f 100644 > --- a/drivers/virtio/virtio_balloon.c > +++ b/drivers/virtio/virtio_balloon.c > @@ -77,7 +77,7 @@ struct virtio_balloon { > > /* Memory statistics */ > int need_stats_update; > - struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR]; > + struct virtio_balloon_stat_modern stats[VIRTIO_BALLOON_S_NR]; > > /* To register callback in oom notifier call chain */ > struct notifier_block nb; > @@ -269,7 +269,11 @@ static void stats_handle_request(struct virtio_balloon *vb) > vq = vb->stats_vq; > if (!virtqueue_get_buf(vq, &len)) > return; > - sg_init_one(&sg, vb->stats, sizeof(vb->stats)); > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) > + sg_init_one(&sg, vb->stats, sizeof(vb->stats)); > + else > + sg_init_one(&sg, &vb->stats->tag, sizeof(vb->stats) - > + offsetof(typeof(*vb->stats, tag); This makes it compile, but definitely won't work. > virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL); > virtqueue_kick(vq); > } > @@ -283,21 +287,30 @@ static void virtballoon_changed(struct virtio_device *vdev) > > static inline s64 towards_target(struct virtio_balloon *vb) > { > - __le32 v; > s64 target; > + u32 num_pages; > > - virtio_cread(vb->vdev, struct virtio_balloon_config, num_pages, &v); > + virtio_cread(vb->vdev, struct virtio_balloon_config, > + num_pages, &num_pages); > > - target = le32_to_cpu(v); > + /* Legacy balloon config space is LE, unlike all other devices. */ > + if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1)) > + num_pages = le32_to_cpu((__force le32)num_pages); > + > + target = num_pages; > return target - vb->num_pages; > } > > static void update_balloon_size(struct virtio_balloon *vb) > { > - __le32 actual = cpu_to_le32(vb->num_pages); > + u32 actual = vb->num_pages; > + > + /* Legacy balloon config space is LE, unlike all other devices. */ > + if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1)) > + actual = (__force u32)cpu_to_le32(num_pages); > > - virtio_cwrite(vb->vdev, struct virtio_balloon_config, actual, > - &actual); > + virtio_cwrite(vb->vdev, struct virtio_balloon_config, > + actual, &actual); > } Final line is gratitous reformatting. I would leave the device *exactly* as is, ugly structure packing and all. Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/