Cc: Nick Piggin <npiggin@...>, Peter Zijlstra <a.p.zijlstra@...>, <linux-mm@...>, Benjamin Herrenschmidt <benh@...>, <steiner@...>, <linux-kernel@...>, Avi Kivity <avi@...>, <kvm-devel@...>, <daniel.blueman@...>, Robin Holt <holt@...>, Hugh Dickins <hugh@...>
On Wed, Jan 30, 2008 at 04:01:31PM -0800, Christoph Lameter wrote:
My assumption is that that "int lock" exists just because
unmap_mapping_range_vma exists. If I'm right then my suggestion was to
move the invalidate_range after dropping the i_mmap_lock and not to
invoke it inside zap_page_range.
Scale is the wrong word. The PT lock will prevent any other cpu to
trash on the mmu_lock, so it's a fixed cost for each pte_clear with no
scalability risk, nor any complexity issue. Certainly we could average
certain fixed costs over more than one pte_clear to boost performance,
and that's good idea. Not really a short term concern, we need to swap
reliably first ;).
That can be optimized adding a __ptep_clear_flush and an
invalidate_pages (let's call it pages to better show it's an
'clustered' version of invalidate_page, to avoid the confusion with
_range_before/after that does an entirely different thing). Also for
_range I tend to like before/after, as a means to say before the
pte_clear and after the pte_clear but any other meaning is ok with me.
We add invalidate_page and invalidate_pages
immediately. invalidate_pages may never be called initially by the
linux VM, we can start calling it later as we replace ptep_clear_flush
with __ptep_clear_flush (or local_ptep_clear_flush).
I don't see any problem with this approach and it looks quite clean to
me and it leaves you full room for experimenting in practice with
range_before/after while knowing those range_before/after won't
require many changes.
And for things like the age_page it will never happen that you could
call the respective ptep_clear_flush_young w/o mmu notifier age_page
after it, so you won't ever risk having to add an age_pages or a
__ptep_clear_flush_young.
There's no reason why KVM should take any risk of corrupting memory
due to a single missing mmu notifier, with not taking the
refcount. get_user_pages will take it for us, so we have to pay the
atomic-op anyway. It sure worth doing the atomic_dec inside the mmu
notifier, and not immediately like this:
get_user_pages(pages)
__free_page(pages[0])
The idea is that what works for GRU, works for KVM too. So we do a
single invalidate_page and clustered invalidate_pages, we add that,
and then we make sure all places are covered so GRU will not
kernel-crash, and KVM won't risk to run oom or to generate _userland_
corruption.
--
