| Message ID | 20200319212150.2651419-1-dwmw2@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Handle | expand |
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of David Woodhouse > Sent: 19 March 2020 21:22 > To: xen-devel@lists.xenproject.org > Cc: Stefano Stabellini <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Wei Liu <wl@xen.org>; > Andrew Cooper <andrew.cooper3@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; George Dunlap > <george.dunlap@citrix.com>; hongyxia@amazon.com; Jan Beulich <jbeulich@suse.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com>; Roger Pau Monné <roger.pau@citrix.com> > Subject: [Xen-devel] [PATCH 1/2] xen/mm: fold PGC_broken into PGC_state bits > > From: David Woodhouse <dwmw@amazon.co.uk> > > Only PGC_state_offlining and PGC_state_offlined are valid in conjunction > with PGC_broken. The other two states (free and inuse) were never valid > for a broken page. > > By folding PGC_broken in, we can have three bits for PGC_state which > allows up to 8 states, of which 6 are currently used and 2 are available > for new use cases. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > xen/arch/x86/domctl.c | 2 +- > xen/common/page_alloc.c | 66 ++++++++++++++++++++++------------------ > xen/include/asm-arm/mm.h | 38 +++++++++++++++-------- > xen/include/asm-x86/mm.h | 36 ++++++++++++++++------ > 4 files changed, 89 insertions(+), 53 deletions(-) > > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c > index ed86762fa6..a411f64afa 100644 > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -422,7 +422,7 @@ long arch_do_domctl( > if ( page->u.inuse.type_info & PGT_pinned ) > type |= XEN_DOMCTL_PFINFO_LPINTAB; > > - if ( page->count_info & PGC_broken ) > + if ( page_is_broken(page) ) > type = XEN_DOMCTL_PFINFO_BROKEN; > } > > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c > index 76d37226df..8d72a64f4e 100644 > --- a/xen/common/page_alloc.c > +++ b/xen/common/page_alloc.c > @@ -1093,7 +1093,7 @@ static int reserve_offlined_page(struct page_info *head) > struct page_info *pg; > int next_order; > > - if ( page_state_is(cur_head, offlined) ) > + if ( page_is_offlined(cur_head) ) > { > cur_head++; > if ( first_dirty != INVALID_DIRTY_IDX && first_dirty ) > @@ -1113,7 +1113,7 @@ static int reserve_offlined_page(struct page_info *head) > for ( i = (1 << cur_order), pg = cur_head + (1 << cur_order ); > i < (1 << next_order); > i++, pg++ ) > - if ( page_state_is(pg, offlined) ) > + if ( page_is_offlined(pg) ) > break; > if ( i == ( 1 << next_order) ) > { > @@ -1145,16 +1145,20 @@ static int reserve_offlined_page(struct page_info *head) > > for ( cur_head = head; cur_head < head + ( 1UL << head_order); cur_head++ ) > { > - if ( !page_state_is(cur_head, offlined) ) > + struct page_list_head *list; > + > + if ( page_state_is(cur_head, offlined) ) > + list = &page_offlined_list; > + else if (page_state_is(cur_head, broken) ) > + list = &page_broken_list; > + else > continue; > > avail[node][zone]--; > total_avail_pages--; > ASSERT(total_avail_pages >= 0); > > - page_list_add_tail(cur_head, > - test_bit(_PGC_broken, &cur_head->count_info) ? > - &page_broken_list : &page_offlined_list); > + page_list_add_tail(cur_head, list); > > count++; > } > @@ -1404,13 +1408,16 @@ static void free_heap_pages( > switch ( pg[i].count_info & PGC_state ) > { > case PGC_state_inuse: > - BUG_ON(pg[i].count_info & PGC_broken); > pg[i].count_info = PGC_state_free; > break; > > case PGC_state_offlining: > - pg[i].count_info = (pg[i].count_info & PGC_broken) | > - PGC_state_offlined; > + pg[i].count_info = PGC_state_offlined; > + tainted = 1; > + break; > + > + case PGC_state_broken_offlining: > + pg[i].count_info = PGC_state_broken; > tainted = 1; > break; > > @@ -1527,16 +1534,16 @@ static unsigned long mark_page_offline(struct page_info *pg, int broken) > do { > nx = x = y; > > - if ( ((x & PGC_state) != PGC_state_offlined) && > - ((x & PGC_state) != PGC_state_offlining) ) > - { > - nx &= ~PGC_state; > - nx |= (((x & PGC_state) == PGC_state_free) > - ? PGC_state_offlined : PGC_state_offlining); > - } > + nx &= ~PGC_state; > > - if ( broken ) > - nx |= PGC_broken; > + /* If it was already broken, it stays broken */ > + if ( pgc_is_broken(x) ) > + broken = 1; > + > + if ( pgc_is_offlined(x) || pgc_is(x, free) ) > + nx |= broken ? PGC_state_broken : PGC_state_offlined; > + else > + nx |= broken ? PGC_state_broken_offlining : PGC_state_offlining; > > if ( x == nx ) > break; > @@ -1609,7 +1616,7 @@ int offline_page(mfn_t mfn, int broken, uint32_t *status) > * need to prevent malicious guest access the broken page again. > * Under such case, hypervisor shutdown guest, preventing recursive mce. > */ > - if ( (pg->count_info & PGC_broken) && (owner = page_get_owner(pg)) ) > + if ( page_is_broken(pg) && (owner = page_get_owner(pg)) ) > { > *status = PG_OFFLINE_AGAIN; > domain_crash(owner); > @@ -1620,7 +1627,7 @@ int offline_page(mfn_t mfn, int broken, uint32_t *status) > > old_info = mark_page_offline(pg, broken); > > - if ( page_state_is(pg, offlined) ) > + if ( page_is_offlined(pg) ) > { > reserve_heap_page(pg); > > @@ -1699,19 +1706,18 @@ unsigned int online_page(mfn_t mfn, uint32_t *status) > do { > ret = *status = 0; > > - if ( y & PGC_broken ) > + if ( pgc_is_broken(y) ) > { > ret = -EINVAL; > - *status = PG_ONLINE_FAILED |PG_ONLINE_BROKEN; > + *status = PG_ONLINE_FAILED | PG_ONLINE_BROKEN; Whitespace fix. Ought to be called out in the commit comment. > break; > } > - > - if ( (y & PGC_state) == PGC_state_offlined ) > + else if ( pgc_is(y, offlined) ) > { > page_list_del(pg, &page_offlined_list); > *status = PG_ONLINE_ONLINED; > } > - else if ( (y & PGC_state) == PGC_state_offlining ) > + else if ( pgc_is(y, offlining) ) > { > *status = PG_ONLINE_ONLINED; > } > @@ -1726,7 +1732,7 @@ unsigned int online_page(mfn_t mfn, uint32_t *status) > > spin_unlock(&heap_lock); > > - if ( (y & PGC_state) == PGC_state_offlined ) > + if ( pgc_is(y, offlined) ) > free_heap_pages(pg, 0, false); > > return ret; > @@ -1747,11 +1753,11 @@ int query_page_offline(mfn_t mfn, uint32_t *status) > > pg = mfn_to_page(mfn); > > - if ( page_state_is(pg, offlining) ) > + if ( page_is_offlining(pg) ) > *status |= PG_OFFLINE_STATUS_OFFLINE_PENDING; > - if ( pg->count_info & PGC_broken ) > + if ( page_is_broken(pg) ) > *status |= PG_OFFLINE_STATUS_BROKEN; > - if ( page_state_is(pg, offlined) ) > + if ( page_is_offlined(pg) ) > *status |= PG_OFFLINE_STATUS_OFFLINED; > > spin_unlock(&heap_lock); > @@ -2519,7 +2525,7 @@ __initcall(pagealloc_keyhandler_init); > > void scrub_one_page(struct page_info *pg) > { > - if ( unlikely(pg->count_info & PGC_broken) ) > + if ( unlikely(page_is_broken(pg)) ) > return; > > #ifndef NDEBUG > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h > index 7df91280bc..a877791d1c 100644 > --- a/xen/include/asm-arm/mm.h > +++ b/xen/include/asm-arm/mm.h > @@ -108,21 +108,35 @@ struct page_info > /* Page is Xen heap? */ > #define _PGC_xen_heap PG_shift(2) > #define PGC_xen_heap PG_mask(1, 2) > -/* ... */ > -/* Page is broken? */ > -#define _PGC_broken PG_shift(7) > -#define PGC_broken PG_mask(1, 7) > - /* Mutually-exclusive page states: { inuse, offlining, offlined, free }. */ > -#define PGC_state PG_mask(3, 9) > -#define PGC_state_inuse PG_mask(0, 9) > -#define PGC_state_offlining PG_mask(1, 9) > -#define PGC_state_offlined PG_mask(2, 9) > -#define PGC_state_free PG_mask(3, 9) > -#define page_state_is(pg, st) (((pg)->count_info&PGC_state) == PGC_state_##st) > + /* > + * Mutually-exclusive page states: > + * { inuse, offlining, offlined, free, broken_offlining, broken } > + */ > +#define PGC_state PG_mask(7, 9) > +#define PGC_state_inuse PG_mask(0, 9) > +#define PGC_state_offlining PG_mask(1, 9) > +#define PGC_state_offlined PG_mask(2, 9) > +#define PGC_state_free PG_mask(3, 9) > +#define PGC_state_broken_offlining PG_mask(4, 9) /* Broken and offlining */ > +#define PGC_state_broken PG_mask(5, 9) /* Broken and offlined */ > + > +#define pgc_is(pgc, st) (((pgc) & PGC_state) == PGC_state_##st) > +#define page_state_is(pg, st) pgc_is((pg)->count_info, st) > + > +#define pgc_is_broken(pgc) (pgc_is(pgc, broken) || \ > + pgc_is(pgc, broken_offlining)) > +#define pgc_is_offlined(pgc) (pgc_is(pgc, offlined) || \ > + pgc_is(pgc, broken)) > +#define pgc_is_offlining(pgc) (pgc_is(pgc, offlining) || \ > + pgc_is(pgc, broken_offlining)) > + > +#define page_is_broken(pg) (pgc_is_broken((pg)->count_info)) > +#define page_is_offlined(pg) (pgc_is_broken((pg)->count_info)) > +#define page_is_offlining(pg) (pgc_is_broken((pg)->count_info)) > + > /* Page is not reference counted */ > #define _PGC_extra PG_shift(10) > #define PGC_extra PG_mask(1, 10) > - Extraneous whitespace change. > /* Count of references to this frame. */ > #define PGC_count_width PG_shift(10) > #define PGC_count_mask ((1UL<<PGC_count_width)-1) > diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h > index a06b2fb81f..1203f1b179 100644 > --- a/xen/include/asm-x86/mm.h > +++ b/xen/include/asm-x86/mm.h > @@ -67,16 +67,32 @@ > /* 3-bit PAT/PCD/PWT cache-attribute hint. */ > #define PGC_cacheattr_base PG_shift(6) > #define PGC_cacheattr_mask PG_mask(7, 6) > - /* Page is broken? */ > -#define _PGC_broken PG_shift(7) > -#define PGC_broken PG_mask(1, 7) > - /* Mutually-exclusive page states: { inuse, offlining, offlined, free }. */ > -#define PGC_state PG_mask(3, 9) > -#define PGC_state_inuse PG_mask(0, 9) > -#define PGC_state_offlining PG_mask(1, 9) > -#define PGC_state_offlined PG_mask(2, 9) > -#define PGC_state_free PG_mask(3, 9) > -#define page_state_is(pg, st) (((pg)->count_info&PGC_state) == PGC_state_##st) > + /* > + * Mutually-exclusive page states: > + * { inuse, offlining, offlined, free, broken_offlining, broken } > + */ > +#define PGC_state PG_mask(7, 9) > +#define PGC_state_inuse PG_mask(0, 9) > +#define PGC_state_offlining PG_mask(1, 9) > +#define PGC_state_offlined PG_mask(2, 9) > +#define PGC_state_free PG_mask(3, 9) > +#define PGC_state_broken_offlining PG_mask(4, 9) /* Broken and offlining */ > +#define PGC_state_broken PG_mask(5, 9) /* Broken and offlined */ > + > +#define pgc_is(pgc, st) (((pgc) & PGC_state) == PGC_state_##st) Maybe pgc_state_is() for consistency? Might also draw attention to the difference between e.g.: pgc_is(pgc, offlined) and pgc_is_offlined(pgc) > +#define page_state_is(pg, st) pgc_is((pg)->count_info, st) Indentation looks wrong. ^^ Same for the arm code. Paul > + > +#define pgc_is_broken(pgc) (pgc_is(pgc, broken) || \ > + pgc_is(pgc, broken_offlining)) > +#define pgc_is_offlined(pgc) (pgc_is(pgc, offlined) || \ > + pgc_is(pgc, broken)) > +#define pgc_is_offlining(pgc) (pgc_is(pgc, offlining) || \ > + pgc_is(pgc, broken_offlining)) > + > +#define page_is_broken(pg) (pgc_is_broken((pg)->count_info)) > +#define page_is_offlined(pg) (pgc_is_broken((pg)->count_info)) > +#define page_is_offlining(pg) (pgc_is_broken((pg)->count_info)) > + > /* Page is not reference counted */ > #define _PGC_extra PG_shift(10) > #define PGC_extra PG_mask(1, 10) > -- > 2.21.0 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
On 19.03.2020 22:21, David Woodhouse wrote: > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -422,7 +422,7 @@ long arch_do_domctl( > if ( page->u.inuse.type_info & PGT_pinned ) > type |= XEN_DOMCTL_PFINFO_LPINTAB; > > - if ( page->count_info & PGC_broken ) > + if ( page_is_broken(page) ) > type = XEN_DOMCTL_PFINFO_BROKEN; Coming back to an earlier request of mine: There are no locks being held here afaics, so with #define page_is_broken(pg) (pgc_is_broken((pg)->count_info)) and #define pgc_is_broken(pgc) (pgc_is(pgc, broken) || \ pgc_is(pgc, broken_offlining)) there's a chance that the page gets transitioned from broken_offlining to broken (by another CPU) between these two checks, resulting in wrong returned state. Either the latter macro uses an intermediate variable and ACCESS_ONCE() or, as suggested before, enumerators get arranged such that the check can be done (e.g. using binary masking operations) with a single evaluation of pgc. This may or may not also be an issue for the other two pgc_is_*(), but I think at least for symmetry they should then follow suit. At the very least all three macros need to be immune to uses like page_is_offlined(pg++) or similar argument expressions with side effects. > @@ -1699,19 +1706,18 @@ unsigned int online_page(mfn_t mfn, uint32_t *status) > do { > ret = *status = 0; > > - if ( y & PGC_broken ) > + if ( pgc_is_broken(y) ) > { > ret = -EINVAL; > - *status = PG_ONLINE_FAILED |PG_ONLINE_BROKEN; > + *status = PG_ONLINE_FAILED | PG_ONLINE_BROKEN; > break; > } > - > - if ( (y & PGC_state) == PGC_state_offlined ) > + else if ( pgc_is(y, offlined) ) At the risk of getting flamed again: Even if it was a matter of taste in new code whether to use "else" in a case like this one, this isn't new code, and it is in no way necessary to change what we have for the purpose of this patch. I.e. without even having to resort to the question of whether personal taste decisions are to be accepted, this simply falls under "no unrelated / unnecessary changes please". (FAOD this includes the deletion of the blank line then as well.) > --- a/xen/include/asm-x86/mm.h > +++ b/xen/include/asm-x86/mm.h > @@ -67,16 +67,32 @@ > /* 3-bit PAT/PCD/PWT cache-attribute hint. */ > #define PGC_cacheattr_base PG_shift(6) > #define PGC_cacheattr_mask PG_mask(7, 6) > - /* Page is broken? */ > -#define _PGC_broken PG_shift(7) > -#define PGC_broken PG_mask(1, 7) > - /* Mutually-exclusive page states: { inuse, offlining, offlined, free }. */ > -#define PGC_state PG_mask(3, 9) > -#define PGC_state_inuse PG_mask(0, 9) > -#define PGC_state_offlining PG_mask(1, 9) > -#define PGC_state_offlined PG_mask(2, 9) > -#define PGC_state_free PG_mask(3, 9) > -#define page_state_is(pg, st) (((pg)->count_info&PGC_state) == PGC_state_##st) > + /* > + * Mutually-exclusive page states: > + * { inuse, offlining, offlined, free, broken_offlining, broken } > + */ > +#define PGC_state PG_mask(7, 9) > +#define PGC_state_inuse PG_mask(0, 9) > +#define PGC_state_offlining PG_mask(1, 9) > +#define PGC_state_offlined PG_mask(2, 9) > +#define PGC_state_free PG_mask(3, 9) > +#define PGC_state_broken_offlining PG_mask(4, 9) /* Broken and offlining */ > +#define PGC_state_broken PG_mask(5, 9) /* Broken and offlined */ > + > +#define pgc_is(pgc, st) (((pgc) & PGC_state) == PGC_state_##st) > +#define page_state_is(pg, st) pgc_is((pg)->count_info, st) > + > +#define pgc_is_broken(pgc) (pgc_is(pgc, broken) || \ > + pgc_is(pgc, broken_offlining)) > +#define pgc_is_offlined(pgc) (pgc_is(pgc, offlined) || \ > + pgc_is(pgc, broken)) > +#define pgc_is_offlining(pgc) (pgc_is(pgc, offlining) || \ > + pgc_is(pgc, broken_offlining)) > + > +#define page_is_broken(pg) (pgc_is_broken((pg)->count_info)) > +#define page_is_offlined(pg) (pgc_is_broken((pg)->count_info)) > +#define page_is_offlining(pg) (pgc_is_broken((pg)->count_info)) Copy-and-paste mistake (rhs is the same for all three; same for Arm)? Also there's no need here for the outer pairs of parentheses. Also, for the next version, may I ask that you number versions in the subject's tag and that you provide a brief description of changes from the previous version (if any, but there ought to be some in a series for there to be a point to send out)? Thanks. Jan
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index ed86762fa6..a411f64afa 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -422,7 +422,7 @@ long arch_do_domctl( if ( page->u.inuse.type_info & PGT_pinned ) type |= XEN_DOMCTL_PFINFO_LPINTAB; - if ( page->count_info & PGC_broken ) + if ( page_is_broken(page) ) type = XEN_DOMCTL_PFINFO_BROKEN; } diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 76d37226df..8d72a64f4e 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -1093,7 +1093,7 @@ static int reserve_offlined_page(struct page_info *head) struct page_info *pg; int next_order; - if ( page_state_is(cur_head, offlined) ) + if ( page_is_offlined(cur_head) ) { cur_head++; if ( first_dirty != INVALID_DIRTY_IDX && first_dirty ) @@ -1113,7 +1113,7 @@ static int reserve_offlined_page(struct page_info *head) for ( i = (1 << cur_order), pg = cur_head + (1 << cur_order ); i < (1 << next_order); i++, pg++ ) - if ( page_state_is(pg, offlined) ) + if ( page_is_offlined(pg) ) break; if ( i == ( 1 << next_order) ) { @@ -1145,16 +1145,20 @@ static int reserve_offlined_page(struct page_info *head) for ( cur_head = head; cur_head < head + ( 1UL << head_order); cur_head++ ) { - if ( !page_state_is(cur_head, offlined) ) + struct page_list_head *list; + + if ( page_state_is(cur_head, offlined) ) + list = &page_offlined_list; + else if (page_state_is(cur_head, broken) ) + list = &page_broken_list; + else continue; avail[node][zone]--; total_avail_pages--; ASSERT(total_avail_pages >= 0); - page_list_add_tail(cur_head, - test_bit(_PGC_broken, &cur_head->count_info) ? - &page_broken_list : &page_offlined_list); + page_list_add_tail(cur_head, list); count++; } @@ -1404,13 +1408,16 @@ static void free_heap_pages( switch ( pg[i].count_info & PGC_state ) { case PGC_state_inuse: - BUG_ON(pg[i].count_info & PGC_broken); pg[i].count_info = PGC_state_free; break; case PGC_state_offlining: - pg[i].count_info = (pg[i].count_info & PGC_broken) | - PGC_state_offlined; + pg[i].count_info = PGC_state_offlined; + tainted = 1; + break; + + case PGC_state_broken_offlining: + pg[i].count_info = PGC_state_broken; tainted = 1; break; @@ -1527,16 +1534,16 @@ static unsigned long mark_page_offline(struct page_info *pg, int broken) do { nx = x = y; - if ( ((x & PGC_state) != PGC_state_offlined) && - ((x & PGC_state) != PGC_state_offlining) ) - { - nx &= ~PGC_state; - nx |= (((x & PGC_state) == PGC_state_free) - ? PGC_state_offlined : PGC_state_offlining); - } + nx &= ~PGC_state; - if ( broken ) - nx |= PGC_broken; + /* If it was already broken, it stays broken */ + if ( pgc_is_broken(x) ) + broken = 1; + + if ( pgc_is_offlined(x) || pgc_is(x, free) ) + nx |= broken ? PGC_state_broken : PGC_state_offlined; + else + nx |= broken ? PGC_state_broken_offlining : PGC_state_offlining; if ( x == nx ) break; @@ -1609,7 +1616,7 @@ int offline_page(mfn_t mfn, int broken, uint32_t *status) * need to prevent malicious guest access the broken page again. * Under such case, hypervisor shutdown guest, preventing recursive mce. */ - if ( (pg->count_info & PGC_broken) && (owner = page_get_owner(pg)) ) + if ( page_is_broken(pg) && (owner = page_get_owner(pg)) ) { *status = PG_OFFLINE_AGAIN; domain_crash(owner); @@ -1620,7 +1627,7 @@ int offline_page(mfn_t mfn, int broken, uint32_t *status) old_info = mark_page_offline(pg, broken); - if ( page_state_is(pg, offlined) ) + if ( page_is_offlined(pg) ) { reserve_heap_page(pg); @@ -1699,19 +1706,18 @@ unsigned int online_page(mfn_t mfn, uint32_t *status) do { ret = *status = 0; - if ( y & PGC_broken ) + if ( pgc_is_broken(y) ) { ret = -EINVAL; - *status = PG_ONLINE_FAILED |PG_ONLINE_BROKEN; + *status = PG_ONLINE_FAILED | PG_ONLINE_BROKEN; break; } - - if ( (y & PGC_state) == PGC_state_offlined ) + else if ( pgc_is(y, offlined) ) { page_list_del(pg, &page_offlined_list); *status = PG_ONLINE_ONLINED; } - else if ( (y & PGC_state) == PGC_state_offlining ) + else if ( pgc_is(y, offlining) ) { *status = PG_ONLINE_ONLINED; } @@ -1726,7 +1732,7 @@ unsigned int online_page(mfn_t mfn, uint32_t *status) spin_unlock(&heap_lock); - if ( (y & PGC_state) == PGC_state_offlined ) + if ( pgc_is(y, offlined) ) free_heap_pages(pg, 0, false); return ret; @@ -1747,11 +1753,11 @@ int query_page_offline(mfn_t mfn, uint32_t *status) pg = mfn_to_page(mfn); - if ( page_state_is(pg, offlining) ) + if ( page_is_offlining(pg) ) *status |= PG_OFFLINE_STATUS_OFFLINE_PENDING; - if ( pg->count_info & PGC_broken ) + if ( page_is_broken(pg) ) *status |= PG_OFFLINE_STATUS_BROKEN; - if ( page_state_is(pg, offlined) ) + if ( page_is_offlined(pg) ) *status |= PG_OFFLINE_STATUS_OFFLINED; spin_unlock(&heap_lock); @@ -2519,7 +2525,7 @@ __initcall(pagealloc_keyhandler_init); void scrub_one_page(struct page_info *pg) { - if ( unlikely(pg->count_info & PGC_broken) ) + if ( unlikely(page_is_broken(pg)) ) return; #ifndef NDEBUG diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h index 7df91280bc..a877791d1c 100644 --- a/xen/include/asm-arm/mm.h +++ b/xen/include/asm-arm/mm.h @@ -108,21 +108,35 @@ struct page_info /* Page is Xen heap? */ #define _PGC_xen_heap PG_shift(2) #define PGC_xen_heap PG_mask(1, 2) -/* ... */ -/* Page is broken? */ -#define _PGC_broken PG_shift(7) -#define PGC_broken PG_mask(1, 7) - /* Mutually-exclusive page states: { inuse, offlining, offlined, free }. */ -#define PGC_state PG_mask(3, 9) -#define PGC_state_inuse PG_mask(0, 9) -#define PGC_state_offlining PG_mask(1, 9) -#define PGC_state_offlined PG_mask(2, 9) -#define PGC_state_free PG_mask(3, 9) -#define page_state_is(pg, st) (((pg)->count_info&PGC_state) == PGC_state_##st) + /* + * Mutually-exclusive page states: + * { inuse, offlining, offlined, free, broken_offlining, broken } + */ +#define PGC_state PG_mask(7, 9) +#define PGC_state_inuse PG_mask(0, 9) +#define PGC_state_offlining PG_mask(1, 9) +#define PGC_state_offlined PG_mask(2, 9) +#define PGC_state_free PG_mask(3, 9) +#define PGC_state_broken_offlining PG_mask(4, 9) /* Broken and offlining */ +#define PGC_state_broken PG_mask(5, 9) /* Broken and offlined */ + +#define pgc_is(pgc, st) (((pgc) & PGC_state) == PGC_state_##st) +#define page_state_is(pg, st) pgc_is((pg)->count_info, st) + +#define pgc_is_broken(pgc) (pgc_is(pgc, broken) || \ + pgc_is(pgc, broken_offlining)) +#define pgc_is_offlined(pgc) (pgc_is(pgc, offlined) || \ + pgc_is(pgc, broken)) +#define pgc_is_offlining(pgc) (pgc_is(pgc, offlining) || \ + pgc_is(pgc, broken_offlining)) + +#define page_is_broken(pg) (pgc_is_broken((pg)->count_info)) +#define page_is_offlined(pg) (pgc_is_broken((pg)->count_info)) +#define page_is_offlining(pg) (pgc_is_broken((pg)->count_info)) + /* Page is not reference counted */ #define _PGC_extra PG_shift(10) #define PGC_extra PG_mask(1, 10) - /* Count of references to this frame. */ #define PGC_count_width PG_shift(10) #define PGC_count_mask ((1UL<<PGC_count_width)-1) diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h index a06b2fb81f..1203f1b179 100644 --- a/xen/include/asm-x86/mm.h +++ b/xen/include/asm-x86/mm.h @@ -67,16 +67,32 @@ /* 3-bit PAT/PCD/PWT cache-attribute hint. */ #define PGC_cacheattr_base PG_shift(6) #define PGC_cacheattr_mask PG_mask(7, 6) - /* Page is broken? */ -#define _PGC_broken PG_shift(7) -#define PGC_broken PG_mask(1, 7) - /* Mutually-exclusive page states: { inuse, offlining, offlined, free }. */ -#define PGC_state PG_mask(3, 9) -#define PGC_state_inuse PG_mask(0, 9) -#define PGC_state_offlining PG_mask(1, 9) -#define PGC_state_offlined PG_mask(2, 9) -#define PGC_state_free PG_mask(3, 9) -#define page_state_is(pg, st) (((pg)->count_info&PGC_state) == PGC_state_##st) + /* + * Mutually-exclusive page states: + * { inuse, offlining, offlined, free, broken_offlining, broken } + */ +#define PGC_state PG_mask(7, 9) +#define PGC_state_inuse PG_mask(0, 9) +#define PGC_state_offlining PG_mask(1, 9) +#define PGC_state_offlined PG_mask(2, 9) +#define PGC_state_free PG_mask(3, 9) +#define PGC_state_broken_offlining PG_mask(4, 9) /* Broken and offlining */ +#define PGC_state_broken PG_mask(5, 9) /* Broken and offlined */ + +#define pgc_is(pgc, st) (((pgc) & PGC_state) == PGC_state_##st) +#define page_state_is(pg, st) pgc_is((pg)->count_info, st) + +#define pgc_is_broken(pgc) (pgc_is(pgc, broken) || \ + pgc_is(pgc, broken_offlining)) +#define pgc_is_offlined(pgc) (pgc_is(pgc, offlined) || \ + pgc_is(pgc, broken)) +#define pgc_is_offlining(pgc) (pgc_is(pgc, offlining) || \ + pgc_is(pgc, broken_offlining)) + +#define page_is_broken(pg) (pgc_is_broken((pg)->count_info)) +#define page_is_offlined(pg) (pgc_is_broken((pg)->count_info)) +#define page_is_offlining(pg) (pgc_is_broken((pg)->count_info)) + /* Page is not reference counted */ #define _PGC_extra PG_shift(10) #define PGC_extra PG_mask(1, 10)