WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 12/17] xenpaging: handle HVMCOPY_gfn_paged_out in

To: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 12/17] xenpaging: handle HVMCOPY_gfn_paged_out in copy_from/to_user
From: Olaf Hering <olaf@xxxxxxxxx>
Date: Tue, 7 Dec 2010 10:45:35 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Tue, 07 Dec 2010 01:46:44 -0800
Dkim-signature: v=1; a=rsa-sha1; c=relaxed/relaxed; t=1291715138; l=1745; s=domk; d=aepfle.de; h=In-Reply-To:Content-Type:MIME-Version:References:Subject:Cc:To:From: Date:X-RZG-CLASS-ID:X-RZG-AUTH; bh=+2/657HBSOT8JnF3fDcIl4aI3KY=; b=L5J990cYh6y1VmQKoOZtWmpNDelB/KfjCwemVwZtiTiEFvU8ga+WH4FxsEyEMx8vATJ foRch4dPe4CbkMlZ5gOqKCRfvHmQIDetiyM8BKvhoQ3QqRIWq1K9CkyFBAKcOyllXPkVZ oUz1dodATauUJn6DcaoNvKnwfYW3moyF+6w=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4CFE0BF90200007800026536@xxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20101206205907.848643876@xxxxxxxxx> <20101206205912.343173055@xxxxxxxxx> <4CFE0BF90200007800026536@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.20 (2009-06-14)
On Tue, Dec 07, Jan Beulich wrote:

> >>> On 06.12.10 at 21:59, Olaf Hering <olaf@xxxxxxxxx> wrote:
> > --- xen-unstable.hg-4.1.22459.orig/xen/arch/x86/mm/guest_walk.c
> > +++ xen-unstable.hg-4.1.22459/xen/arch/x86/mm/guest_walk.c
> > @@ -93,11 +93,12 @@ static inline void *map_domain_gfn(struc
> >                                     uint32_t *rc) 
> >  {
> >      /* Translate the gfn, unsharing if shared */
> > +retry:
> >      *mfn = gfn_to_mfn_unshare(p2m, gfn_x(gfn), p2mt, 0);
> >      if ( p2m_is_paging(*p2mt) )
> >      {
> > -        p2m_mem_paging_populate(p2m, gfn_x(gfn));
> > -
> > +        if ( p2m_mem_paging_populate(p2m, gfn_x(gfn)) )
> > +            goto retry;
> >          *rc = _PAGE_PAGED;
> >          return NULL;
> >      }
> 
> Is this retry loop (and similar ones later in the patch) guaranteed
> to be bounded in some way?

This needs to be fixed, yes.
For the plain __hvm_copy case, with nothing else being modified, the
'return HVMCOPY_gfn_paged_out' could be just a 'continue'. But even
then, something needs to break the loop.

> >          /* gfn is already on its way back and vcpu is not paused */
> > -        return;
> > +        goto populate_out;
> 
> Do you really need a goto here (i.e. are you foreseeing to get stuff
> added between the label and the return below)?

Thats something for my debug patch, I have a trace_var at the end of
each function.

> > +/* retval 1 means the page is present on return */
> > +int p2m_mem_paging_populate(struct p2m_domain *p2m, unsigned long gfn);
> 
> Isn't this a case where you absolutely need the return value checked?
> If so, you will want to add __must_check here.

Yes, that would be a good addition.
Maybe the wait_event/wake_up could be done unconditionally, independent
if the p2m domain differs from the vcpu domain.


Olaf

> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>