[ofa-general] Re: [RFC] Notifier for Externally Mapped Memory (EMM)

Andrea Arcangeli andrea at qumranet.com
Tue Mar 4 14:20:30 PST 2008


On Tue, Mar 04, 2008 at 11:00:31AM -0800, Christoph Lameter wrote:
> But as you pointed out before that path is a slow path anyways. Its rarely 

It's a slow path but I don't see why you think two hooks are better
than one, when only one is necessary.

I once ripped invalidate_page while working on #v8 but then I
reintroduced it because I thought reducing the total number of hooks
was beneficial to the core linux VM (even if only a
microoptimization, I sure agree about that, but it's trivial to add
one hook instead of two hooks there, so a microoptimization was worth
it IMHO).

Your API is also too restrictive, if we'll happen to need one more
method that doesn't take just (start,end) we'll have to cause all
drivers to have significant changes instead of one-liners to use
whatever new feature.

> taken. Having a single eviction callback simplifies design.

IMHO the design is actually same and I don't understand why you
rewrote it once more time in a less flexibile way (on a style side
you're not even using hlist), dropping RCU (not sure how you replace
it with), etc....

Your implementation has the same bug you had in your first V1, see how
you're not clearing the spte young bits if the pte young bit is
set. Once you fix that, your change in the ptep_clear_flush_young path
will look remarkably similar to the patch I posted incremental with
#v8 to make ->clear_flush_young sleep capable...

Converging in a single design is great, but it'd be nice if we could
converge into a single implementation, and my last patch doesn't have
any bug and I think it's quite nicer too (also including Nick cleanup
work) but then I may be biased ;).

But as usual I'm entirely satisfied by your brand new EMM Notifier to
be merged and all perfecting work done on my MMU notifier patch over
the weeks by multiple developers (including you) to be dropped for
good, as long as we can enable the new advanced KVM features in
2.6.25.



More information about the general mailing list