From: "Jan Beulich" Subject: [PATCH] x86: simplify page reference handling for partially (in-)validated pages Date: Mon, 03 Nov 2008 07:42:47 +0000 Message-ID: <490EB987.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org As suggested by Keir, simplify general page reference management for preempted (partiall [in-]validated) pages: Reserve on reference that can be acquired without the risk of overflowing the reference count, thus allowing to have a simplified get_page() equivalent that cannot fail (but must be used with care). Doing this conversion pointed out a latent issue in the changes done previously in this area: The extra reference must be acquired before the 'normal' reference gets dropped, so the patch fixes this at once in both the alloc_page_type() and free_page_type() paths (it's really only the latter that failed to work with the change described above). Signed-off-by: Jan Beulich Index: 2008-10-27/xen/arch/x86/mm.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- 2008-10-27.orig/xen/arch/x86/mm.c 2008-10-29 16:46:52.000000000 = +0100 +++ 2008-10-27/xen/arch/x86/mm.c 2008-10-31 16:13:56.000000000 = +0100 @@ -1856,7 +1856,8 @@ int get_page(struct page_info *page, str nx =3D x + 1; d =3D nd; if ( unlikely((x & PGC_count_mask) =3D=3D 0) || /* Not allocated?= */ - unlikely((nx & PGC_count_mask) =3D=3D 0) || /* Count = overflow? */ + /* Keep one spare reference to be acquired by get_page_light(= ). */ + unlikely(((nx + 1) & PGC_count_mask) <=3D 1) || /* Overflow? = */ unlikely(d !=3D _domain) ) /* Wrong owner? */ { if ( !_shadow_mode_refcounts(domain) && !domain->is_dying ) @@ -1878,6 +1879,28 @@ int get_page(struct page_info *page, str return 1; } =20 +/* + * Special version of get_page() to be used exclusively when + * - a page is known to already have a non-zero reference count + * - the page does not need its owner to be checked + * - it will not be called more than once without dropping the thus + * acquired reference again. + * Due to get_page() reserving one reference, this call cannot fail. + */ +static void get_page_light(struct page_info *page) +{ + u32 x, nx, y =3D page->count_info; + + do { + x =3D y; + nx =3D x + 1; + BUG_ON(!(x & PGC_count_mask)); /* Not allocated? */ + BUG_ON(!(nx & PGC_count_mask)); /* Overflow? */ + y =3D cmpxchg(&page->count_info, x, nx); + } + while ( unlikely(y !=3D x) ); +} + =20 static int alloc_page_type(struct page_info *page, unsigned long type, int preemptible) @@ -1885,10 +1908,6 @@ static int alloc_page_type(struct page_i struct domain *owner =3D page_get_owner(page); int rc; =20 - /* Obtain an extra reference to retain if we set PGT_partial. */ - if ( preemptible && !get_page(page, owner) ) - return -EINVAL; - /* A page table is dirtied when its type count becomes non-zero. */ if ( likely(owner !=3D NULL) ) paging_mark_dirty(owner, page_to_mfn(page)); @@ -1922,14 +1941,10 @@ static int alloc_page_type(struct page_i wmb(); if ( rc =3D=3D -EAGAIN ) { + get_page_light(page); page->u.inuse.type_info |=3D PGT_partial; - return -EAGAIN; } - - if ( preemptible ) - put_page(page); - - if ( rc =3D=3D -EINTR ) + else if ( rc =3D=3D -EINTR ) { ASSERT((page->u.inuse.type_info & (PGT_count_mask|PGT_validated|PGT_partial)) =3D=3D 1); @@ -2037,8 +2052,8 @@ static int __put_page_type_final(struct=20 page->u.inuse.type_info--; break; case -EINTR: - ASSERT(!(page->u.inuse.type_info & - (PGT_count_mask|PGT_validated|PGT_partial))); + ASSERT((page->u.inuse.type_info & + (PGT_count_mask|PGT_validated|PGT_partial)) =3D=3D 1); if ( !(shadow_mode_enabled(page_get_owner(page)) && (page->count_info & PGC_page_table)) ) page->tlbflush_timestamp =3D tlbflush_current_time(); @@ -2047,17 +2062,13 @@ static int __put_page_type_final(struct=20 break; case -EAGAIN: wmb(); + get_page_light(page); page->u.inuse.type_info |=3D PGT_partial; - /* Must skip put_page() below. */ - preemptible =3D 0; break; default: BUG(); } =20 - if ( preemptible ) - put_page(page); - return rc; } =20 @@ -2066,10 +2077,7 @@ static int __put_page_type(struct page_i int preemptible) { unsigned long nx, x, y =3D page->u.inuse.type_info; - - /* Obtain an extra reference to retain if we set PGT_partial. */ - if ( preemptible && !get_page(page, page_get_owner(page)) ) - return -EINVAL; + int rc =3D 0; =20 for ( ; ; ) { @@ -2092,10 +2100,11 @@ static int __put_page_type(struct page_i if ( unlikely((y =3D cmpxchg(&page->u.inuse.type_info, x, nx)) !=3D x) ) continue; + /* We cleared the 'valid bit' so we do the clean up. */ + rc =3D __put_page_type_final(page, x, preemptible); if ( x & PGT_partial ) put_page(page); - /* We cleared the 'valid bit' so we do the clean up. */ - return __put_page_type_final(page, x, preemptible); + break; } =20 /* @@ -2114,17 +2123,10 @@ static int __put_page_type(struct page_i break; =20 if ( preemptible && hypercall_preempt_check() ) - { - if ( preemptible ) - put_page(page); return -EINTR; - } } =20 - if ( preemptible ) - put_page(page); - - return 0; + return rc; } =20 =20 @@ -2132,6 +2134,7 @@ static int __get_page_type(struct page_i int preemptible) { unsigned long nx, x, y =3D page->u.inuse.type_info; + int rc =3D 0; =20 ASSERT(!(type & ~(PGT_type_mask | PGT_pae_xen_l2))); =20 @@ -2227,11 +2230,7 @@ static int __get_page_type(struct page_i } =20 if ( likely((y =3D cmpxchg(&page->u.inuse.type_info, x, nx)) = =3D=3D x) ) - { - if ( (x & PGT_partial) && !(nx & PGT_partial) ) - put_page(page); break; - } =20 if ( preemptible && hypercall_preempt_check() ) return -EINTR; @@ -2258,10 +2257,13 @@ static int __get_page_type(struct page_i page->nr_validated_ptes =3D 0; page->partial_pte =3D 0; } - return alloc_page_type(page, type, preemptible); + rc =3D alloc_page_type(page, type, preemptible); } =20 - return 0; + if ( (x & PGT_partial) && !(nx & PGT_partial) ) + put_page(page); + + return rc; } =20 void put_page_type(struct page_info *page)