On Wed, Jul 20, 2016 at 4:05 AM, Artturi Alm <[email protected]> wrote: > #define PV_BEEN_EXECD(f) (((f) & (PVF_REF | PVF_EXEC)) == (PVF_REF | > PVF_EXEC)) > #define PV_BEEN_REFD(f) (((f) & PVF_REF) != 0) > > and from pmap.h: > * The PVF_MOD and PVF_REF flags are stored in the mdpage for each > * page. PVF_WIRED, PVF_WRITE, and PVF_NC are kept in individual > * pv_entry's for each page. They live in the same "namespace" so > * that we can clear multiple attributes at a time. > > well, this isn't stricly about clearing, but using PVF_REF in those macros > does mean you are likely to want | attrs too.
Hmm, in there an invariant that this code is supposed to maintain between the bits set in the pvh_attrs and the pv_flags under it? It *looks* like it's trying to enforce that the mdpage.pvh_attrs's PVF_REF and PVF_MOD bits are set if they're set in any of the pv_entries listed under it, yes? They can be spuriously set even though they're clear in all the entries under it, but if they're clear, they're clear everywhere. If that's not the invariant, then what do those bits in pvh_attrs mean? Anyway, assuming that invariant... > - if (PV_BEEN_EXECD(oflags)) > + if (PV_BEEN_EXECD(oflags | oattrs)) > pmap_tlb_flushID_SE(pm, va); > else > - if (PV_BEEN_REFD(oflags)) > + if (PV_BEEN_REFD(oflags | oattrs)) > pmap_tlb_flushD_SE(pm, va); If the page's mapping at *this* VA wasn't referenced before this pmap_enter() changes it, then why do we need to flush the TLBs for it merely because that page has been referenced at some other VA? We're only flushing the TLB for this VA anyway and not that other VA... Philip Guenther
