RE: [Xen-devel] slow xp hibernation revisited

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Sat, 4 Jun 2011, James Harper wrote:
> > 
> > On 04/06/2011 08:38, "James Harper" <james.harper@xxxxxxxxxxxxxxxx>
> wrote:
> > 
> > > Looking past the test_bit call, the next statement does another test
> and
> > > sets last_address_index to 0 and returns NULL. Is this just to
> ensure
> > > that the next access isn't just trivially accepted?
> > 
> > Yes, first test is on a potentially stale bucket. Second test is on a
> fresh
> > bucket.
> > 
> 
> How about the following patch? Is munmap the correct way to unmap or is
> an IOCTL required too?
> 

munmap is OK


> The exit condition is what would happen anyway after the remap is done
> and the page is still invalid.
> 
> diff --git a/hw/xen_machine_fv.c b/hw/xen_machine_fv.c
> index d02e23f..1ff80bb 100644
> --- a/hw/xen_machine_fv.c
> +++ b/hw/xen_machine_fv.c
> @@ -151,6 +151,24 @@ uint8_t *qemu_map_cache(target_phys_addr_t
> phys_addr, uint8_t lock)
>          pentry->next = entry;
>          qemu_remap_bucket(entry, address_index);
>      } else if (!entry->lock) {
> +        if (entry->vaddr_base && entry->paddr_index == address_index &&
> !test_bit(address_offset>>XC_PAGE_SHIFT, entry->valid_mapping))
> +        {
> +            /* The page was invalid previously. Test if it is valid now
> and only remap if so */
> +            xen_pfn_t pfn;
> +            int err;
> +            void *tmp_vaddr;
> +
> +            pfn = phys_addr >> XC_PAGE_SHIFT;
> +            tmp_vaddr = xc_map_foreign_bulk(xc_handle, domid,
> PROT_READ|PROT_WRITE, &pfn, &err, 1);
> +            if (tmp_vaddr)
> +                munmap(tmp_vaddr, PAGE_SIZE);
> +
> +            if (!tmp_vaddr || err)
> +            {
> +                last_address_index = ~0UL;
> +                return NULL;
> +            }
> +        }
>          if (!entry->vaddr_base || entry->paddr_index != address_index
> || !test_bit(address_offset>>XC_PAGE_SHIFT, entry->valid_mapping))
>              qemu_remap_bucket(entry, address_index);
>      }

This is just a matter of aesthetic, but probably something like the
following would be clearer?

if (entry->vaddr_base && entry->paddr_index == address_index) {
    if (!test_bit(address_offset>>XC_PAGE_SHIFT, entry->valid_mapping)) {
            /* your code + remap bucket */
    }
} else {
    qemu_remap_bucket(entry, address_index);
}


Could you also please send a similar patch for upstream qemu?
The code should be quite similar in this area, look at xen-mapcache.c.

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