Re: [Qemu-devel] [PATCH] exec: Skip mru section if it's a partial page and not resolving subpage

[BALATON Zoltan]

On Wed, 15 Nov 2017, Paolo Bonzini wrote:
> This is another possibility:
>
> diff --git a/exec.c b/exec.c
> index 97a24a875e..3bb9fcf257 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -410,22 +410,16 @@ static MemoryRegionSection 
> *address_space_lookup_region(AddressSpaceDispatch *d,
> {
>     MemoryRegionSection *section = atomic_read(&d->mru_section);
>     subpage_t *subpage;
> -    bool update;
>
> -    if (section && section != &d->map.sections[PHYS_SECTION_UNASSIGNED] &&
> -        section_covers_addr(section, addr)) {
> -        update = false;
> -    } else {
> +    if (!section || section == &d->map.sections[PHYS_SECTION_UNASSIGNED] ||
> +        !section_covers_addr(section, addr)) {
>         section = phys_page_find(d, addr);
> -        update = true;
> +        atomic_set(&d->mru_section, section);
>     }
>     if (resolve_subpage && section->mr->subpage) {
>         subpage = container_of(section->mr, subpage_t, iomem);
>         section = &d->map.sections[subpage->sub_section[SUBPAGE_IDX(addr)]];
>     }
> -    if (update) {
> -        atomic_set(&d->mru_section, section);
> -    }
>     return section;
> }
>
>
> It will skip the expensive phys_page_find but not the cheap subpage lookup.
> Does it work for you?

This also works but I can't understand why becuase the problematic call to 
this function which got the wrong result from section_covers_addr was with 
resolve_subpage=false and in that case this looks identical to the 
original which did not work. Unless this function is called multiple times 
and another call with resolve_subpage=true now somehow works better with 
this change or replacing the mru_section earlier before subpage lookup 
makes a difference which is not obvious from this function.

Anyway, it fixes the problem I see so you can use this instead of my 
patch.

Thank you,
BALATON Zoltan