RE: [Xen-devel] [PATCH] Do not set page's count_info directly

["Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>]

Gianluca Guida <mailto:glguida@xxxxxxxxx> wrote:
> On Thu, Mar 5, 2009 at 10:11 AM, Jiang, Yunhong
> <yunhong.jiang@xxxxxxxxx> wrote:
>> Page offline patch add several flag to
> page_info->count_info. However, currently some code will try
> to set count_info after alloc_domheap_pages without using "&"
> or "|" operation, this may cause the new flags lost, since
> there are no protection. This patch try to make sure all write
> to count_info will only impact specific field.
> 
> Hm, wouldn't be better to add some comments in mm.h or do this through
> a macro? I think that one would normally tend to suppose, after you
> allocate a page, that the count_info is all yours to set. It usually
> is, since the offlining should be a rare event (I hope?), so catching
> this kind of bug would be very hard, and make the whole mechanism very
> fragile. 

Yes, I considered how to prevent this kind of mis-use and in the end find it is 
impossible. The caller can always set the count_info if they like it.

One option is to use another bitmap to keep the offline information instead of 
page_info, but that will wast too many memory, since page offline is rare case 
as you pointed out.
Another option is to enable this bitmap only in debug version. When free page, 
we will check the bitmap, if the bitmap is set while the count_info is not set, 
it means something wrong happens and raise a BUG().

Any suggestion?

Thanks
Yunhong Jiang

> 
>> Also currently shadow code assume count_info is 0 for shadow
> page, however, this is invalid after the new flags. Change
> some assert in shadow code.
> 
> Yes, that's correct. I find, though, (count_info & PGC_count_mask !=
> 0) as a check if the page is a shadow *very* confusing. Could you
> define a macro with something like page_is_a_shadow_page() and hide this
> mm.c dirty secret? 
> 
> Thanks,
> Gianluca
> 
> --
> It was a type of people I did not know, I found them very strange and
> they did not inspire confidence at all. Later I learned that I had been
> introduced to electronic engineers.
>                                                  E. W. Dijkstra
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel