From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH]Add a flag for shadow pages Date: Wed, 04 Mar 2009 09:06:52 +0000 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "Jiang, Yunhong" , Tim Deegan Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 04/03/2009 08:46, "Jiang, Yunhong" wrote: > Currently we don't know that a page is a shadow page unless we are in shadow > handler. This cause error when we try to get the page owner for the shadow > page, this snippet add a flag to it. > > signed-off-by: Jiang, Yunhong You only actually use your new flag in a BUG_ON, where it might avoid false negatives but hardly looks essential. Your other changes limit count_info checks to PGC_page_table|PGC_count_info which might be an improvement (I'm sure Tim or Gianluca can say) but they're existing fields. > I'm not quite sure if the sh_put_ref() and sh_rm_write_access_from_sl1p() is > try to checking a page is shadow page (I assume so), because when a anonymous > page is allocated, the count_info is also 0 (like HVM's vlapic page), so I > change it like this patch (checking PGC_count_mask is 0). Since comments in > sh_hash_audit_bucket() has stated clearly it is to check if it is shdow, so I > replace it with test_bit(). > > Also, do we need checking in page_get_owner() also? Usually gets used where we already get_page()ed or are about to get_page(). Most Xen code knows that page uses can change under its feet unless it grabs a reference. So extra checks in page_get_owner() are probably rather pointless. -- Keir