[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] exec: Skip mru section if it's a partial page a
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] exec: Skip mru section if it's a partial page and not resolving subpage |
Date: |
Wed, 15 Nov 2017 22:14:48 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
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