amd64/arm64 pmap: consistently clear PGA_WRITEABLE on fictitious, managed pages
ClosedPublic
Actions

Authored by alc on Sun, Sep 6, 6:33 PM.

Details

Reviewers

kib
markj
andrew
manu

Commits

rG37321270b630: amd64/arm64 pmap: consistently clear PGA_WRITEABLE

Summary

While optimizing icache management on arm64, I found that we don't consistently clear PGA_WRITEABLE on fictitious, managed pages. Some functions do, e.g., pmap_remove_all(), but several do not. At worst, this is just a pessimization, but there is no good reason to be inconsistent Clear PGA_WRITEABLE in those that previously did not.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

alc created this revision.Sun, Sep 6, 6:33 PM

Herald added a reviewer: andrew. · View Herald TranscriptSun, Sep 6, 6:33 PM

Herald added a reviewer: andrew. · View Herald Transcript

Herald added a reviewer: manu. · View Herald Transcript

Herald added a subscriber: emaste. · View Herald Transcript

alc requested review of this revision.Sun, Sep 6, 6:33 PM

kib added inline comments.Sun, Sep 6, 10:49 PM

sys/amd64/amd64/pmap.c

5321

Is it worth making a static function like this

static void
pmap_might_be_clear_writeable(vm_page_t m)
{
    if (!TAILQ_EMPTY(&m->md.pv_list))
        return;
    if ((m->flags & PG_FICTITIOUS) == 0 &&
        !TAILQ_EMPTY(&pa_to_pvh(tpte & PG_FRAME)->pv_list)
           return;
      vm_page_aflag_clear(m, PGA_WRITEABLE);
}

Introduce pmap_page_is_mapped_locked().

markj accepted this revision.Mon, Sep 7, 1:17 PM

markj added inline comments.

sys/amd64/amd64/pmap.c
7279	Some sites check whether PGA_WRITEABLE is set before checking whether or not to clear it. Others don't, but I can't see a good reason for the inconsistency.

This revision is now accepted and ready to land.Mon, Sep 7, 1:17 PM

alc added inline comments.Mon, Sep 7, 4:07 PM

sys/amd64/amd64/pmap.c
7279	This one is the easiest to explain. It is most likely replacing a mapping to a COW page, and the additional check avoids what would be a pointless atomic. The other is pmap_remove_pages(), where we are also destroying a lot of R/O and COW mappings. In the other cases, I think it is less clear what type of mapping it is.