On Thu, Feb 28, 2008 at 11:48:10AM -0800, Christoph Lameter wrote:
> > make it work after the VM locking will be altered (for example the
^^^^^^^^^^^^^^^
> > CONFIG_XPMEM should also switch the mmu_register/unregister locking
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > from RCU to mutex as well). XPMEM then will only compile if
^^^^^^^^^^^^^^^^^^^^^^^^^
> > CONFIG_XPMEM=y and in turn the invalidate_range_* will support
> > scheduling inside.
>
> This is not going to work even if the mutex would work as easily as you
> think since the patch here still does an rcu_lock/unlock around a callback.
See underlined.
> > +struct mmu_notifier_ops {
> > + /*
> > + * Called when nobody can register any more notifier in the mm
> > + * and after the "mn" notifier has been disarmed already.
> > + */
> > + void (*release)(struct mmu_notifier *mn,
> > + struct mm_struct *mm);
>
> Who disarms the notifier? Why is the method not called to disarm the
> notifier on exit?
The notifier is auto-disarmed by mmu_notifier_release, your patch
works the same way. ->release is further called just in case anybody
wants to know the notifier was disarmed.
> > @@ -2048,6 +2050,7 @@ void exit_mmap(struct mm_struct *mm)
> > vm_unacct_memory(nr_accounted);
> > free_pgtables(&tlb, vma, FIRST_USER_ADDRESS, 0);
> > tlb_finish_mmu(tlb, 0, end);
> > + mmu_notifier_release(mm);
>
> The release should be called much earlier to allow the driver to release
> all resources in one go. This way each vma must be processed individually.
> For our gobs of memory this method may create a scaling problem on exit().
Good point, it has to be called earlier for GRU, but it's not a
performance issue. GRU doesn't pin the pages so it should make the
global invalidate in ->release _before_ unmap_vmas. Linux can't fault
in the ptes anymore because mm_users is zero so there's no need of a
->release_begin/end, the _begin is enough.
In #v6 I was invalidating inside unmap_vmas so it was ok. The
performance issues you're talking about refers to #v6 I guess, for #v7
there's a single call.
Thanks!
diff --git a/mm/mmap.c b/mm/mmap.c
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2039,6 +2039,7 @@ void exit_mmap(struct mm_struct *mm)
unsigned long end;
/* mm's last user has gone, and its about to be pulled down */
+ mmu_notifier_release(mm);
arch_exit_mmap(mm);
lru_add_drain();
@@ -2050,7 +2051,6 @@ void exit_mmap(struct mm_struct *mm)
vm_unacct_memory(nr_accounted);
free_pgtables(&tlb, vma, FIRST_USER_ADDRESS, 0);
tlb_finish_mmu(tlb, 0, end);
- mmu_notifier_release(mm);
/*
* Walk the list again, actually closing and freeing it,
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
