From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] x86: fix determination of bit count for struct domain allocations Date: Mon, 24 Mar 2014 08:56:51 +0000 Message-ID: <532FF353.20203@gmail.com> References: <53232CEF020000780012443B@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6138386063535735026==" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WS0gk-0003tx-E7 for xen-devel@lists.xenproject.org; Mon, 24 Mar 2014 08:56:58 +0000 Received: by mail-la0-f44.google.com with SMTP id hr13so3458540lab.3 for ; Mon, 24 Mar 2014 01:56:55 -0700 (PDT) In-Reply-To: <53232CEF020000780012443B@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --===============6138386063535735026== Content-Type: multipart/alternative; boundary="------------030901080802070504000302" This is a multi-part message in MIME format. --------------030901080802070504000302 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit > Jan Beulich > 14 March 2014 15:23 > We can't just add in the hole shift value, as the hole may be at or > above the 44-bit boundary. Instead we need to determine the total bit > count until reaching 32 significant (not squashed out) bits in PFN > representations. > > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -180,6 +180,19 @@ void dump_pageframe_info(struct domain * > spin_unlock(&d->page_alloc_lock); > } > > +static unsigned int __init noinline _domain_struct_bits(void) > +{ > + unsigned int bits = 32 + PAGE_SHIFT; > + unsigned int sig = hweight32(~pfn_hole_mask); > + unsigned int mask = pfn_hole_mask >> 32; > + > + for ( ; bits < BITS_PER_LONG && sig < 32; ++bits, mask >>= 1 ) > + if ( !(mask & 1) ) > + ++sig; This logic took a few looks to be sure what it was doing, let alone correct. Without the patch comment I'd be even a bit more lost. I think the changeset comment, or similar, should be included as a code comment for this new function. I'm sure given there is only one hole there is a simpler form of this based on checking whether bottom_shift > 44. But this is more general I suppose, so probably better despite being harder to understand. Acked-by: Keir Fraser > + return bits; > +} > + > struct domain *alloc_domain_struct(void) > { > struct domain *d; > @@ -187,7 +200,10 @@ struct domain *alloc_domain_struct(void) > * We pack the PDX of the domain structure into a 32-bit field within > * the page_info structure. Hence the MEMF_bits() restriction. > */ > - unsigned int bits = 32 + PAGE_SHIFT + pfn_pdx_hole_shift; > + static unsigned int __read_mostly bits; > + > + if ( unlikely(!bits) ) > + bits = _domain_struct_bits(); > > BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE); > d = alloc_xenheap_pages(0, MEMF_bits(bits)); > > > --------------030901080802070504000302 Content-Type: multipart/related; boundary="------------020507030609090605020409" --------------020507030609090605020409 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit
14 March 2014 15:23
We can't just add in the hole shift value, as the hole may be at or
above the 44-bit boundary. Instead we need to determine the total bit
count until reaching 32 significant (not squashed out) bits in PFN
representations.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -180,6 +180,19 @@ void dump_pageframe_info(struct domain *
spin_unlock(&d->page_alloc_lock);
}

+static unsigned int __init noinline _domain_struct_bits(void)
+{
+ unsigned int bits = 32 + PAGE_SHIFT;
+ unsigned int sig = hweight32(~pfn_hole_mask);
+ unsigned int mask = pfn_hole_mask >> 32;
+
+ for ( ; bits < BITS_PER_LONG && sig < 32; ++bits, mask >>= 1 )
+ if ( !(mask & 1) )
+ ++sig;

This logic took a few looks to be sure what it was doing, let alone correct. Without the patch comment I'd be even a bit more lost. I think the changeset comment, or similar, should be included as a code comment for this new function.

I'm sure given there is only one hole there is a simpler form of this based on checking whether bottom_shift > 44. But this is more general I suppose, so probably better despite being harder to understand.

Acked-by: Keir Fraser <keir@xen.org>
+ return bits;
+}
+
struct domain *alloc_domain_struct(void)
{
struct domain *d;
@@ -187,7 +200,10 @@ struct domain *alloc_domain_struct(void)
* We pack the PDX of the domain structure into a 32-bit field within
* the page_info structure. Hence the MEMF_bits() restriction.
*/
- unsigned int bits = 32 + PAGE_SHIFT + pfn_pdx_hole_shift;
+ static unsigned int __read_mostly bits;
+
+ if ( unlikely(!bits) )
+ bits = _domain_struct_bits();

BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
d = alloc_xenheap_pages(0, MEMF_bits(bits));



--------------020507030609090605020409 Content-Type: image/jpeg; x-apple-mail-type=stationery; name="compose-unknown-contact.jpg" Content-Transfer-Encoding: base64 Content-ID: Content-Disposition: inline; filename="compose-unknown-contact.jpg" /9j/4AAQSkZJRgABAQEARwBHAAD/2wBDAAEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEC AQEBAQEBAgICAgICAgICAgICAgICAgICAgICAgICAgICAgL/2wBDAQEBAQEBAQICAgICAgIC AgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgL/wAAR CAAZABkDAREAAhEBAxEB/8QAGAAAAwEBAAAAAAAAAAAAAAAABgcICQr/xAA0EAABAwMCAgUK BwAAAAAAAAACAQMEBQYRABITIQcUMUF2CBUXIjI2N0JRtVRWkZOV0dL/xAAYAQEAAwEAAAAA AAAAAAAAAAADAAEEAv/EACQRAAICAAQGAwAAAAAAAAAAAAABAhEDMrHREyExM0FxgfDx/9oA DAMBAAIRAxEAPwDuEt+gW/ULet6oVC3rfqNQqFv0OfPn1GhUqfOmzZtKZlS5UqZMaNwzNwiJ VIl7eXLCaZIGwBl3TY8epPx2+jy2ZNPjvkwc9uhW8j7nCPhvOsQliYIeS7cvCpp8o50qwrC4 v3lsNSDbdmTEhvs2tahxpfV3WnmbbozJEw/gwdadbYExVRXKEKoSdvJcaOSqxE7/AAiX0gXx +a69/JSf9alIlste0VzaNpeFrcT9KKymotyiaZ0KRCnzacoE7Kjzn4gi2KqUh3jqDHDHv4mR UfruTWlMzlVUKIVNp9GguEJnAh0+IZjyAiisgyRDnu5azS8miKqjOTVkKqS/psG37fo1Fbab eg25b8eZPeFJBBJSjMG5HjMeyihnaauZwe4OGiju13GAcpOwBeN+U8/IkGbsiS8b7ryogmbz hbyc9REROfZhERO5ETShjPtvpGqTUyLErytS4siSwx5x2tRH4hPOI0DkjZtaJtFxuVEbIUUi yeNujlBUJGbJN6nM/Cyf2Hf60YgjvKA+NPSP4gT7axpcPtr51YWJnYn9dnAQWl722p4ot37y zqnlfp6FrqbwawG8/9k= --------------020507030609090605020409-- --------------030901080802070504000302-- --===============6138386063535735026== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============6138386063535735026==--