WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ppc-devel

Re: [XenPPC] [PATCH 3 of 6] [PATCH] xen: implement guest_physmap_max_mem

To: Christian Ehrhardt <ehrhardt@xxxxxxxxxxxxxxxxxx>
Subject: Re: [XenPPC] [PATCH 3 of 6] [PATCH] xen: implement guest_physmap_max_mem for ppc
From: Ryan Harper <ryanh@xxxxxxxxxx>
Date: Thu, 22 Feb 2007 10:04:03 -0600
Cc: xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 22 Feb 2007 08:03:11 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <45DD9D13.7000101@xxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-ppc-devel-request@lists.xensource.com?subject=help>
List-id: Xen PPC development <xen-ppc-devel.lists.xensource.com>
List-post: <mailto:xen-ppc-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ppc-devel>, <mailto:xen-ppc-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ppc-devel>, <mailto:xen-ppc-devel-request@lists.xensource.com?subject=unsubscribe>
References: <35fd77200dff7e73fe39.1172103421@xxxxxxxxxxxxxxxxxxxxx> <45DD7D88.1000107@xxxxxxxxxxxxxxxxxx> <45DD9D13.7000101@xxxxxxxxxxxxxxxxxx>
Sender: xen-ppc-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.6+20040907i
* Christian Ehrhardt <ehrhardt@xxxxxxxxxxxxxxxxxx> [2007-02-22 07:40]:
> >>+int do_guest_physmap_max_mem(struct domain *d, unsigned long new_max)
> >>+{
> >>+    ulong *p2m_array = NULL;
> >>+    ulong *p2m_old = NULL;
> >>+    ulong p2m_pages;
> >>+    ulong copy_end = 0;
> >>+
> >>+    /* we don't handle shrinking max_pages */
> >>+    if ( new_max < d->max_pages )
> >>+    {
> >>+        printk("Can't shrink %d max_mem\n", d->domain_id);
> >>  
> >
> >Just some wording, but maybe printk("Can't shrink max_mem of domain 
> >%d\n", d->domain_id); would prevent some users from mis-interpreting 
> >the number as an unit of memory size.

Yeah, definitely worth being more specific.

> >>+    /* free old p2m array if present */
> >>+    if ( p2m_old )
> >>+        free_xenheap_pages(d->arch.p2m, 
> >>get_order_from_pages(d->max_pages));
> >>  
> >Maybe I just don't get it right because I'm new in this area, but if 
> >an old mapping exists you do here summarized:
> >1. save old d->arch.p2m in p2m_old
> >2. create a new p2m_array including the old mapping as copy
> >3. assigning the new array to d->arch.p2m
> >4. ?? if an old mapping was present you free the new one in 
> >d->arch.p2m ??
> >=> this would leave one unreferenced heap allocation in memory 
> >(p2m_old) without a chance to free it after the local variable p2m_old 
> >disappears and the actively used table d->arch.p2m point to freed heap.
> >I assume the freeing code should be
> >
> >+    /* free old p2m array if present */
> >+    if ( p2m_old )
> >+        free_xenheap_pages(p2m_old, get_order_from_pages(d->max_pages));

Yep, good catch.  Thanks.

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253   T/L: 678-9253
ryanh@xxxxxxxxxx

_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel

<Prev in Thread] Current Thread [Next in Thread>