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

[Gianluca Guida <glguida@xxxxxxxxx>]

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.

> 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