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. Linus wanted it done this two step way because he was worried about too many IPIs. I guess it doesn't make too much difference though because near all callers only change single pages. The flush could be probably folded back into c_p_a(). BTW we know the sequence for doing this is not quite as recommended by Intel (TODO item) but afaik the TLB flushing works. > 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. Originally attribute was only the caching attribute, but later changed to include NX and RW for slab (but that area was always a bit hackish and might have some bugs) For non default to another non default changes the count should not change. If it doesn't work this way that would be a bug. > Finally, at least for the kernel image range it would seem to me that it might > be beneficial to recombine mappings into large ones even when the > attributes are not at their default anymore, but consistent across an entire > 2Mb/4Mb range (i.e. after write-protecting .text). At the same time I wonder, > though, whether it wouldn't be safer to remove execute permission from > anything but .text along with write-protecting read-only regions under > CONFIG_DEBUG_RODATA. Yes I guess that would be a useful optimization. Just getting the reference counting right with that might be tricky. At least the i386 code will probably change significantly soon as I clean up the GB page patches, which require some restructuring in c_p_a(). -Andi - 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/
