From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933061AbcJYBVc convert rfc822-to-8bit (ORCPT ); Mon, 24 Oct 2016 21:21:32 -0400 Received: from mga09.intel.com ([134.134.136.24]:51283 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758166AbcJYBVa (ORCPT ); Mon, 24 Oct 2016 21:21:30 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,544,1473145200"; d="scan'208";a="23291757" From: "Li, Liang Z" To: "Hansen, Dave" , "mst@redhat.com" CC: "linux-kernel@vger.kernel.org" , "virtualization@lists.linux-foundation.org" , "linux-mm@kvack.org" , "virtio-dev@lists.oasis-open.org" , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" , "quintela@redhat.com" , "dgilbert@redhat.com" , "pbonzini@redhat.com" , "cornelia.huck@de.ibm.com" , "amit.shah@redhat.com" Subject: RE: [RESEND PATCH v3 kernel 2/7] virtio-balloon: define new feature bit and page bitmap head Thread-Topic: [RESEND PATCH v3 kernel 2/7] virtio-balloon: define new feature bit and page bitmap head Thread-Index: AQHSK2WRkZsNTGeXVka44lM97FAiR6C3UMeAgAEToBA= Date: Tue, 25 Oct 2016 01:21:21 +0000 Message-ID: References: <1477031080-12616-1-git-send-email-liang.z.li@intel.com> <1477031080-12616-3-git-send-email-liang.z.li@intel.com> <580E3C07.701@intel.com> In-Reply-To: <580E3C07.701@intel.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZGFkNTRlYWUtYmUxMS00N2RmLWJiYmYtMGUwMjZkOGYxYzZiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Im1JQ3JKaUpjajRZeDFWeDNpclJpVWpQWDY5dTlqdk14aW4wUFlaUmE5SVk9In0= x-ctpclassification: CTP_IC 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 10/20/2016 11:24 PM, Liang Li wrote: > > Add a new feature which supports sending the page information with a > > bitmap. The current implementation uses PFNs array, which is not very > > efficient. Using bitmap can improve the performance of > > inflating/deflating significantly > > Why is it not efficient? How is using a bitmap more efficient? What kinds of > cases is the bitmap inefficient? > > > The page bitmap header will used to tell the host some information > > about the page bitmap. e.g. the page size, page bitmap length and > > start pfn. > > Why did you choose to add these features to the structure? What benefits > do they add? > > Could you describe your solution a bit here, and describe its strengths and > weaknesses? > Will elaborate the solution in V4. > > /* Size of a PFN in the balloon interface. */ #define > > VIRTIO_BALLOON_PFN_SHIFT 12 @@ -82,4 +83,22 @@ struct > > virtio_balloon_stat { > > __virtio64 val; > > } __attribute__((packed)); > > > > +/* Page bitmap header structure */ > > +struct balloon_bmap_hdr { > > + /* Used to distinguish different request */ > > + __virtio16 cmd; > > + /* Shift width of page in the bitmap */ > > + __virtio16 page_shift; > > + /* flag used to identify different status */ > > + __virtio16 flag; > > + /* Reserved */ > > + __virtio16 reserved; > > + /* ID of the request */ > > + __virtio64 req_id; > > + /* The pfn of 0 bit in the bitmap */ > > + __virtio64 start_pfn; > > + /* The length of the bitmap, in bytes */ > > + __virtio64 bmap_len; > > +}; > > FWIW this is totally unreadable. Please do something like this: > > > +struct balloon_bmap_hdr { > > + __virtio16 cmd; /* Used to distinguish different ... > > + __virtio16 page_shift; /* Shift width of page in the bitmap */ > > + __virtio16 flag; /* flag used to identify different... > > + __virtio16 reserved; /* Reserved */ > > + __virtio64 req_id; /* ID of the request */ > > + __virtio64 start_pfn; /* The pfn of 0 bit in the bitmap */ > > + __virtio64 bmap_len; /* The length of the bitmap, in bytes */ > > +}; > > and please make an effort to add useful comments. "/* Reserved */" > seems like a waste of bytes to me. OK. Maybe 'padding' is better than 'reserved' . Thanks for your comments! Liang From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Li, Liang Z" Subject: RE: [RESEND PATCH v3 kernel 2/7] virtio-balloon: define new feature bit and page bitmap head Date: Tue, 25 Oct 2016 01:21:21 +0000 Message-ID: References: <1477031080-12616-1-git-send-email-liang.z.li@intel.com> <1477031080-12616-3-git-send-email-liang.z.li@intel.com> <580E3C07.701@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: "virtio-dev@lists.oasis-open.org" , "kvm@vger.kernel.org" , "amit.shah@redhat.com" , "qemu-devel@nongnu.org" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "pbonzini@redhat.com" , "virtualization@lists.linux-foundation.org" , "dgilbert@redhat.com" To: "Hansen, Dave" , "mst@redhat.com" Return-path: In-Reply-To: <580E3C07.701@intel.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org List-Id: kvm.vger.kernel.org > On 10/20/2016 11:24 PM, Liang Li wrote: > > Add a new feature which supports sending the page information with a > > bitmap. The current implementation uses PFNs array, which is not very > > efficient. Using bitmap can improve the performance of > > inflating/deflating significantly > > Why is it not efficient? How is using a bitmap more efficient? What kinds of > cases is the bitmap inefficient? > > > The page bitmap header will used to tell the host some information > > about the page bitmap. e.g. the page size, page bitmap length and > > start pfn. > > Why did you choose to add these features to the structure? What benefits > do they add? > > Could you describe your solution a bit here, and describe its strengths and > weaknesses? > Will elaborate the solution in V4. > > /* Size of a PFN in the balloon interface. */ #define > > VIRTIO_BALLOON_PFN_SHIFT 12 @@ -82,4 +83,22 @@ struct > > virtio_balloon_stat { > > __virtio64 val; > > } __attribute__((packed)); > > > > +/* Page bitmap header structure */ > > +struct balloon_bmap_hdr { > > + /* Used to distinguish different request */ > > + __virtio16 cmd; > > + /* Shift width of page in the bitmap */ > > + __virtio16 page_shift; > > + /* flag used to identify different status */ > > + __virtio16 flag; > > + /* Reserved */ > > + __virtio16 reserved; > > + /* ID of the request */ > > + __virtio64 req_id; > > + /* The pfn of 0 bit in the bitmap */ > > + __virtio64 start_pfn; > > + /* The length of the bitmap, in bytes */ > > + __virtio64 bmap_len; > > +}; > > FWIW this is totally unreadable. Please do something like this: > > > +struct balloon_bmap_hdr { > > + __virtio16 cmd; /* Used to distinguish different ... > > + __virtio16 page_shift; /* Shift width of page in the bitmap */ > > + __virtio16 flag; /* flag used to identify different... > > + __virtio16 reserved; /* Reserved */ > > + __virtio64 req_id; /* ID of the request */ > > + __virtio64 start_pfn; /* The pfn of 0 bit in the bitmap */ > > + __virtio64 bmap_len; /* The length of the bitmap, in bytes */ > > +}; > > and please make an effort to add useful comments. "/* Reserved */" > seems like a waste of bytes to me. OK. Maybe 'padding' is better than 'reserved' . Thanks for your comments! Liang From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f197.google.com (mail-pf0-f197.google.com [209.85.192.197]) by kanga.kvack.org (Postfix) with ESMTP id 791556B0282 for ; Mon, 24 Oct 2016 21:21:26 -0400 (EDT) Received: by mail-pf0-f197.google.com with SMTP id i85so132782382pfa.5 for ; Mon, 24 Oct 2016 18:21:26 -0700 (PDT) Received: from mga06.intel.com (mga06.intel.com. [134.134.136.31]) by mx.google.com with ESMTPS id q199si18198903pgq.205.2016.10.24.18.21.25 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 18:21:25 -0700 (PDT) From: "Li, Liang Z" Subject: RE: [RESEND PATCH v3 kernel 2/7] virtio-balloon: define new feature bit and page bitmap head Date: Tue, 25 Oct 2016 01:21:21 +0000 Message-ID: References: <1477031080-12616-1-git-send-email-liang.z.li@intel.com> <1477031080-12616-3-git-send-email-liang.z.li@intel.com> <580E3C07.701@intel.com> In-Reply-To: <580E3C07.701@intel.com> Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Sender: owner-linux-mm@kvack.org List-ID: To: "Hansen, Dave" , "mst@redhat.com" Cc: "linux-kernel@vger.kernel.org" , "virtualization@lists.linux-foundation.org" , "linux-mm@kvack.org" , "virtio-dev@lists.oasis-open.org" , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" , "quintela@redhat.com" , "dgilbert@redhat.com" , "pbonzini@redhat.com" , "cornelia.huck@de.ibm.com" , "amit.shah@redhat.com" > On 10/20/2016 11:24 PM, Liang Li wrote: > > Add a new feature which supports sending the page information with a > > bitmap. The current implementation uses PFNs array, which is not very > > efficient. Using bitmap can improve the performance of > > inflating/deflating significantly >=20 > Why is it not efficient? How is using a bitmap more efficient? What kin= ds of > cases is the bitmap inefficient? >=20 > > The page bitmap header will used to tell the host some information > > about the page bitmap. e.g. the page size, page bitmap length and > > start pfn. >=20 > Why did you choose to add these features to the structure? What benefits > do they add? >=20 > Could you describe your solution a bit here, and describe its strengths a= nd > weaknesses? >=20 Will elaborate the solution in V4. > > /* Size of a PFN in the balloon interface. */ #define > > VIRTIO_BALLOON_PFN_SHIFT 12 @@ -82,4 +83,22 @@ struct > > virtio_balloon_stat { > > __virtio64 val; > > } __attribute__((packed)); > > > > +/* Page bitmap header structure */ > > +struct balloon_bmap_hdr { > > + /* Used to distinguish different request */ > > + __virtio16 cmd; > > + /* Shift width of page in the bitmap */ > > + __virtio16 page_shift; > > + /* flag used to identify different status */ > > + __virtio16 flag; > > + /* Reserved */ > > + __virtio16 reserved; > > + /* ID of the request */ > > + __virtio64 req_id; > > + /* The pfn of 0 bit in the bitmap */ > > + __virtio64 start_pfn; > > + /* The length of the bitmap, in bytes */ > > + __virtio64 bmap_len; > > +}; >=20 > FWIW this is totally unreadable. Please do something like this: >=20 > > +struct balloon_bmap_hdr { > > + __virtio16 cmd; /* Used to distinguish different ... > > + __virtio16 page_shift; /* Shift width of page in the bitmap */ > > + __virtio16 flag; /* flag used to identify different... > > + __virtio16 reserved; /* Reserved */ > > + __virtio64 req_id; /* ID of the request */ > > + __virtio64 start_pfn; /* The pfn of 0 bit in the bitmap */ > > + __virtio64 bmap_len; /* The length of the bitmap, in bytes */ > > +}; >=20 > and please make an effort to add useful comments. "/* Reserved */" > seems like a waste of bytes to me. OK. Maybe 'padding' is better than 'reserved' . Thanks for your comments! Liang -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58574) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byqQl-0005qb-25 for qemu-devel@nongnu.org; Mon, 24 Oct 2016 21:21:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byqQh-00058i-2w for qemu-devel@nongnu.org; Mon, 24 Oct 2016 21:21:31 -0400 Received: from mga06.intel.com ([134.134.136.31]:48945) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1byqQg-00058N-Mx for qemu-devel@nongnu.org; Mon, 24 Oct 2016 21:21:27 -0400 From: "Li, Liang Z" Date: Tue, 25 Oct 2016 01:21:21 +0000 Message-ID: References: <1477031080-12616-1-git-send-email-liang.z.li@intel.com> <1477031080-12616-3-git-send-email-liang.z.li@intel.com> <580E3C07.701@intel.com> In-Reply-To: <580E3C07.701@intel.com> Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [Qemu-devel] [RESEND PATCH v3 kernel 2/7] virtio-balloon: define new feature bit and page bitmap head List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Hansen, Dave" , "mst@redhat.com" Cc: "linux-kernel@vger.kernel.org" , "virtualization@lists.linux-foundation.org" , "linux-mm@kvack.org" , "virtio-dev@lists.oasis-open.org" , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" , "quintela@redhat.com" , "dgilbert@redhat.com" , "pbonzini@redhat.com" , "cornelia.huck@de.ibm.com" , "amit.shah@redhat.com" > On 10/20/2016 11:24 PM, Liang Li wrote: > > Add a new feature which supports sending the page information with a > > bitmap. The current implementation uses PFNs array, which is not very > > efficient. Using bitmap can improve the performance of > > inflating/deflating significantly >=20 > Why is it not efficient? How is using a bitmap more efficient? What kin= ds of > cases is the bitmap inefficient? >=20 > > The page bitmap header will used to tell the host some information > > about the page bitmap. e.g. the page size, page bitmap length and > > start pfn. >=20 > Why did you choose to add these features to the structure? What benefits > do they add? >=20 > Could you describe your solution a bit here, and describe its strengths a= nd > weaknesses? >=20 Will elaborate the solution in V4. > > /* Size of a PFN in the balloon interface. */ #define > > VIRTIO_BALLOON_PFN_SHIFT 12 @@ -82,4 +83,22 @@ struct > > virtio_balloon_stat { > > __virtio64 val; > > } __attribute__((packed)); > > > > +/* Page bitmap header structure */ > > +struct balloon_bmap_hdr { > > + /* Used to distinguish different request */ > > + __virtio16 cmd; > > + /* Shift width of page in the bitmap */ > > + __virtio16 page_shift; > > + /* flag used to identify different status */ > > + __virtio16 flag; > > + /* Reserved */ > > + __virtio16 reserved; > > + /* ID of the request */ > > + __virtio64 req_id; > > + /* The pfn of 0 bit in the bitmap */ > > + __virtio64 start_pfn; > > + /* The length of the bitmap, in bytes */ > > + __virtio64 bmap_len; > > +}; >=20 > FWIW this is totally unreadable. Please do something like this: >=20 > > +struct balloon_bmap_hdr { > > + __virtio16 cmd; /* Used to distinguish different ... > > + __virtio16 page_shift; /* Shift width of page in the bitmap */ > > + __virtio16 flag; /* flag used to identify different... > > + __virtio16 reserved; /* Reserved */ > > + __virtio64 req_id; /* ID of the request */ > > + __virtio64 start_pfn; /* The pfn of 0 bit in the bitmap */ > > + __virtio64 bmap_len; /* The length of the bitmap, in bytes */ > > +}; >=20 > and please make an effort to add useful comments. "/* Reserved */" > seems like a waste of bytes to me. OK. Maybe 'padding' is better than 'reserved' . Thanks for your comments! Liang