From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751940AbdASBqN convert rfc822-to-8bit (ORCPT ); Wed, 18 Jan 2017 20:46:13 -0500 Received: from mga09.intel.com ([134.134.136.24]:60459 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751183AbdASBqA (ORCPT ); Wed, 18 Jan 2017 20:46:00 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,251,1477983600"; d="scan'208";a="1095981422" From: "Li, Liang Z" To: "Michael S. Tsirkin" CC: "kvm@vger.kernel.org" , "virtio-dev@lists.oasis-open.org" , "qemu-devel@nongnu.org" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "virtualization@lists.linux-foundation.org" , "amit.shah@redhat.com" , "Hansen, Dave" , "cornelia.huck@de.ibm.com" , "pbonzini@redhat.com" , "david@redhat.com" , "aarcange@redhat.com" , "dgilbert@redhat.com" , "quintela@redhat.com" Subject: RE: [PATCH v6 kernel 3/5] virtio-balloon: speed up inflate/deflate process Thread-Topic: [PATCH v6 kernel 3/5] virtio-balloon: speed up inflate/deflate process Thread-Index: AQHSW1evV/kH7yiC1ku9/YmVsAmoeKE8r2iAgAElHsCAAC4yAIABLlNg Date: Thu, 19 Jan 2017 01:44:36 +0000 Message-ID: References: <1482303148-22059-1-git-send-email-liang.z.li@intel.com> <1482303148-22059-4-git-send-email-liang.z.li@intel.com> <20170117211131-mutt-send-email-mst@kernel.org> <20170118172401-mutt-send-email-mst@kernel.org> In-Reply-To: <20170118172401-mutt-send-email-mst@kernel.org> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Wed, Jan 18, 2017 at 04:56:58AM +0000, Li, Liang Z wrote: > > > > - virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL); > > > > - virtqueue_kick(vq); > > > > +static void do_set_resp_bitmap(struct virtio_balloon *vb, > > > > + unsigned long base_pfn, int pages) > > > > > > > > - /* When host has read buffer, this completes via balloon_ack */ > > > > - wait_event(vb->acked, virtqueue_get_buf(vq, &len)); > > > > +{ > > > > + __le64 *range = vb->resp_data + vb->resp_pos; > > > > > > > > + if (pages > (1 << VIRTIO_BALLOON_NR_PFN_BITS)) { > > > > + /* when the length field can't contain pages, set it to 0 to > > > > > > /* > > > * Multi-line > > > * comments > > > * should look like this. > > > */ > > > > > > Also, pls start sentences with an upper-case letter. > > > > > > > Sorry for that. > > > > > > + * indicate the actual length is in the next __le64; > > > > + */ > > > > > > This is part of the interface so should be documented as such. > > > > > > > + *range = cpu_to_le64((base_pfn << > > > > + VIRTIO_BALLOON_NR_PFN_BITS) | 0); > > > > + *(range + 1) = cpu_to_le64(pages); > > > > + vb->resp_pos += 2; > > > > > > Pls use structs for this kind of stuff. > > > > I am not sure if you mean to use > > > > struct range { > > __le64 pfn: 52; > > __le64 nr_page: 12 > > } > > Instead of the shift operation? > > Not just that. You want to add a pages field as well. > pages field? Could you give more hints? > Generally describe the format in the header in some way so host and guest > can easily stay in sync. 'VIRTIO_BALLOON_NR_PFN_BITS' is for this purpose and it will be passed to the related function in page_alloc.c as a parameter. Thanks! Liang > All the pointer math and void * means we get zero type safety and I'm not > happy about it. > > It's not good that virtio format seeps out to page_alloc anyway. > If unavoidable it is not a good idea to try to hide this fact, people will assume > they can change the format at will. > > -- > MST