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-devel

[Xen-devel] Re: [PATCH 3/5] xen: allow balloon driver to use more than o

To: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH 3/5] xen: allow balloon driver to use more than one memory region
From: David Vrabel <david.vrabel@xxxxxxxxxx>
Date: Wed, 7 Sep 2011 11:44:36 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 07 Sep 2011 03:42:35 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110906215706.GC24860@xxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1313765840-22084-1-git-send-email-david.vrabel@xxxxxxxxxx> <1313765840-22084-4-git-send-email-david.vrabel@xxxxxxxxxx> <20110906215706.GC24860@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.16) Gecko/20110818 Icedove/3.0.11
On 06/09/11 22:57, Konrad Rzeszutek Wilk wrote:
> On Fri, Aug 19, 2011 at 03:57:18PM +0100, David Vrabel wrote:
>> 
>> +static void __init balloon_add_memory_region(unsigned long start_pfn, 
>> unsigned long pages)
> 
> That looks suspiciously like it has more than 80 lines. You ran the
> _all_ of the patches through scripts/checkpath.pl right?

Yes, I used checkpatch.pl but I tend to treat the 80 character limit as
a guideline rather than a hard limit and only pay attention to it if
breaking a line improves readability.

I assume your preference is for an 80 character hard limit?

>>  {
>> -    domid_t domid = DOMID_SELF;
>>      unsigned long pfn, extra_pfn_end;
>>      struct page *page;
>> +
>> +    /*
>> +     * Initialise the balloon with excess memory space.  We need
>> +     * to make sure we don't add memory which doesn't exist or
>> +     * logically exist.  The E820 map can be trimmed to be smaller
>> +     * than the amount of physical memory due to the mem= command
>> +     * line parameter.  And if this is a 32-bit non-HIGHMEM kernel
>> +     * on a system with memory which requires highmem to access,
>> +     * don't try to use it.
> 
> 
> That whole comment is just confusing.. But I do know that you
> just moved it, but I wonder if it makes sense to remove it or
> alter it in the future.

I think the comment is valid.

>> +     */
>> +    extra_pfn_end = min(min(max_pfn, e820_end_of_ram_pfn()),
>> +                        start_pfn + pages);

It's this line that it's documenting.

>> --- a/include/xen/page.h
>> +++ b/include/xen/page.h
>> @@ -3,6 +3,13 @@
>>  
>>  #include <asm/xen/page.h>
>>  
>> -extern phys_addr_t xen_extra_mem_start, xen_extra_mem_size;
>> +struct xen_memory_region {
>> +    phys_addr_t start;
>> +    phys_addr_t size;
>> +};
>> +
>> +#define XEN_EXTRA_MEM_MAX_REGIONS 128 /* == E820MAX */
>> +
>> +extern struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS];
> 
> __initdata

Only the definition (in arch/x86/xen/setup.c) needs the __initdata
attribute, right?

I just checked and it does end up in the .init.data section.

David

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

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