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

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> 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?

> --- xen-unstable.hg-4.1.22459.orig/xen/arch/x86/mm/p2m.c
> +++ xen-unstable.hg-4.1.22459/xen/arch/x86/mm/p2m.c
> @@ -2805,13 +2806,13 @@ void p2m_mem_paging_populate(struct p2m_
>      /* Pause domain */
>      if ( v->domain->domain_id == d->domain_id )
>      {
> -        vcpu_pause_nosync(v);
>          req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;
> +        ret = 1;
>      }
>      else if ( p2mt != p2m_ram_paging_out && p2mt != p2m_ram_paged )
>      {
>          /* 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)?

>      }
>  
>      /* Send request to pager */
> @@ -2820,6 +2821,14 @@ void p2m_mem_paging_populate(struct p2m_
>      req.vcpu_id = v->vcpu_id;
>  
>      mem_event_put_request(d, &req);
> +
> +    if ( req.flags & MEM_EVENT_FLAG_VCPU_PAUSED )
> +    {
> +        wait_event(d->wq, mfn_valid(gfn_to_mfn(p2m, gfn, &p2mt)) && 
> !p2m_is_paging(p2mt));
> +    }
> +
> +populate_out:
> +    return ret;
>  }
>  
>  int p2m_mem_paging_prep(struct p2m_domain *p2m, unsigned long gfn)
> --- xen-unstable.hg-4.1.22459.orig/xen/include/asm-x86/p2m.h
> +++ xen-unstable.hg-4.1.22459/xen/include/asm-x86/p2m.h
> @@ -474,7 +474,8 @@ int p2m_mem_paging_evict(struct p2m_doma
>  /* Tell xenpaging to drop a paged out frame */
>  void p2m_mem_paging_drop_page(struct p2m_domain *p2m, unsigned long gfn);
>  /* Start populating a paged out frame */
> -void p2m_mem_paging_populate(struct p2m_domain *p2m, unsigned long gfn);
> +/* 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.

Jan


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