Re: [Minios-devel] [PATCH v2 16/18] mini-os: map page allocator's bitmap to virtual kernel area for ballooning

[Juergen Gross <jgross@xxxxxxxx>]

On 10/08/16 22:45, Samuel Thibault wrote:
> It looks a bit odd to be remapping something that was just allocated,
> but I guess it makes portability easier.

Remapping needs the page allocator to be active as page tables need
to be allocated. :-)

> 
> Juergen Gross, on Fri 05 Aug 2016 19:36:00 +0200, wrote:
>> diff --git a/balloon.c b/balloon.c
>> index 4c18c5c..75b87c8 100644
>> --- a/balloon.c
>> +++ b/balloon.c
>> @@ -44,3 +44,20 @@ void get_max_pages(void)
>>      nr_max_pages = ret;
>>      printk("Maximum memory size: %ld pages\n", nr_max_pages);
>>  }
>> +
>> +void alloc_bitmap_remap(void)
>> +{
>> +    unsigned long i;
>> +
>> +    if ( alloc_bitmap_size >= ((nr_max_pages + 1) >> (PAGE_SHIFT + 3)) )
>> +        return;
>> +
>> +    for ( i = 0; i < alloc_bitmap_size; i += PAGE_SIZE )
>> +    {
>> +        map_frame_rw(virt_kernel_area_end + i,
>> +                     virt_to_mfn((unsigned long)(alloc_bitmap) + i));
>> +    }
>> +
>> +    alloc_bitmap = (unsigned long *)virt_kernel_area_end;
>> +    virt_kernel_area_end += round_pgup((nr_max_pages + 1) >> (PAGE_SHIFT + 
>> 3));
> 
> Ditto here, better check against hitting VIRT_DEMAND_AREA.

Okay.

> 
>> diff --git a/include/balloon.h b/include/balloon.h
>> index b8d9335..0e2340b 100644
>> --- a/include/balloon.h
>> +++ b/include/balloon.h
>> @@ -31,11 +31,13 @@ extern unsigned long virt_kernel_area_end;
>>  
>>  void get_max_pages(void);
>>  void arch_remap_p2m(unsigned long max_pfn);
>> +void alloc_bitmap_remap(void);
>>  
>>  #else /* CONFIG_BALLOON */
>>  
>>  static inline void get_max_pages(void) { }
>>  static inline void arch_remap_p2m(unsigned long max_pfn) { }
>> +static inline void alloc_bitmap_remap(void) { }
> 
> I'd say call it rather mm_alloc_bitmap_remap(). We have C namespace
> issues with the stubdom applications, and the alloc_bitmap_ prefix seems
> quite generic (even if less that bitmap_), mm_ adds some kernelish
> notion.

Okay.

> 
>>  extern unsigned long nr_free_pages;
>>  
>> +extern unsigned long *alloc_bitmap;
>> +extern unsigned long alloc_bitmap_size;
> 
> Ditto, mm_bitmap and mm_bitmap_size.

Okay.

> 
> Otherwise it looks good.

Thanks,

Juergen


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel