Re: [Xen-devel] [PATCH v5 1/4] xen: move steal_page and donate_page to common code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.09.13 at 19:20, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
wrote:
> On Tue, 10 Sep 2013, Jan Beulich wrote:
>> >>> On 09.09.13 at 18:06, Stefano Stabellini 
>> >>> <stefano.stabellini@xxxxxxxxxxxxx> 
> wrote:
>> > --- a/xen/common/memory.c
>> > +++ b/xen/common/memory.c
>> > @@ -230,6 +230,91 @@ int guest_remove_page(struct domain *d, unsigned long 
> gmfn)
>> >      return 1;
>> >  }
>> >  
>> > +int donate_page(
>> > +    struct domain *d, struct page_info *page, unsigned int memflags)
>> > +{
>> > +    spin_lock(&d->page_alloc_lock);
>> > +
>> > +    if ( is_xen_heap_page(page) || (page_get_owner(page) != NULL) )
>> > +        goto fail;
>> > +
>> > +    if ( d->is_dying )
>> > +        goto fail;
>> > +
>> > +    if ( page->count_info & ~(PGC_allocated | 1) )
>> 
>> While the literal 1 was acceptable in arch specific code, I'm not sure
>> it really is in generic code. Nothing enforces that an arch has the
>> count starting at bit 0. Even if it reads a little odd, using
>> (-PGC_count_mask & PGC_count_mask) would probably be an
>> acceptable replacement here and further down.
> 
> (-PGC_count_mask & PGC_count_mask) is always 0.
> 
> I guess you mean:
> 
> if ( page->count_info & ~(PGC_count_mask | PGC_allocated) )

No, I really meant what I wrote, resulting in

    if ( page->count_info & ~(PGC_allocated | (-PGC_count_mask & 
PGC_count_mask)) )

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel