>>> Andi Kleen <[EMAIL PROTECTED]> 05.04.07 14:43 >>> >On Thursday 05 April 2007 11:32:49 Jan Beulich wrote: >> Looking at both the i386 and x86-64 implementations I fail to understand why >> there is an explicit requirement on calling global_flush_tlb() after >> change_page_attr(), yet actual TLB flushing will not normally happen (on i386 >> it will happen if the CPU doesn't support clflush, but if it does, or on >> x86-64, >> the flushing depends on the list of deferred pages being non-empty, which >> can only happen when a large page gets re-combined). Is it assumed that >> the callers additionally call tlb_flush_all() (I think none of them do)? > >Not sure I understand the question? global_flush_tlb is perhaps a little >misnamed, but it only flushes the pages changed in change_page_attr. >This works because it uses INVLPG which should ignore the G bits, >so not additional global flush is needed.
That is the point - I don't see this invlpg. If you look at x86-64's global_flush_tlb(), then you will note that it passes the list of pages grabbed from deferred_pages. If that list has no entries, no single __flush_tlb_one will be called, and the only place entries are added to this list is in save_page(), which in turn only gets called if page_private(kpte_page) is zero (i.e. the page was just reverted back to a big one). This is also hardened by the fact that the flushing and the freeing of pages happens walking the same list, hence only pages being freed get flushed. Am I missing something here? >> Further, change_page_attr()'s reference counting in a split large page's >> page table appears to imply that attributes are only changed from or back to >> the reference attributes, but not from one kind of non-default ones to the >> same or another set of non-default ones (otherwise the reference count >> will never again drop to zero), > and also not from default to default (i.e. >> the >> caller trying to revert attributes to normal not knowing what state they are >> currently in) - this would BUG() if the large page was already reverted, or >> screw the reference count otherwise. Is all of this intentional? I think it >> will need to be changed as a prerequisite to supporting on-the-fly attribute >> changes in the SMP alternatives code, which was requested as a follow-up >> to the tightening of the CONFIG_DEBUG_RODATA effects. > >The reference count is just to count pages that have a non default attribute >in the PMD range so that we know when to revert to a large page. > >For non default to another non default changes the count should not change. That is what it should be, but it also gets bumped when a page already had non-default attributes, because the increment just depends on pgprot_val(prot) != pgprot_val(ref_prot) (but specifically not on the attributes the page had before the change). Jan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
