On 15/11/2017 20:00, BALATON Zoltan wrote: > 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.
The problem was that a resolve_subpage=true mru_section was reused for a resolve_subpage=false mru_section. This is fixed by always making the mru_section the same as if resolve_subpage=false (mru_section is set before looking into the subpage_t). Thanks, Paolo > 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