On Mon, May 05, 2008 at 12:25:06PM -0500, Jack Steiner wrote:
> Agree. My apologies... I should have caught it.
No problem.
> __mmu_notifier_register/__mmu_notifier_unregister seems like a better way to
> go, although either is ok.
If you also like __mmu_notifier_register more I'll go with it. The
bitflags seems like a bit of overkill as I can't see the need of any
other bitflag other than this one and they also can't be removed as
easily in case you'll find a way to call it outside the lock later.
> Let me finish my testing. At one time, I did not use ->release but
> with all the locking & teardown changes, I need to do some reverification.
If you didn't implement it you shall apply this patch but you shall
read carefully the comment I written that covers that usage case.
diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -29,10 +29,25 @@ struct mmu_notifier_ops {
/*
* Called either by mmu_notifier_unregister or when the mm is
* being destroyed by exit_mmap, always before all pages are
- * freed. It's mandatory to implement this method. This can
- * run concurrently with other mmu notifier methods and it
+ * freed. This can run concurrently with other mmu notifier
+ * methods (the ones invoked outside the mm context) and it
* should tear down all secondary mmu mappings and freeze the
- * secondary mmu.
+ * secondary mmu. If this method isn't implemented you've to
+ * be sure that nothing could possibly write to the pages
+ * through the secondary mmu by the time the last thread with
+ * tsk->mm == mm exits.
+ *
+ * As side note: the pages freed after ->release returns could
+ * be immediately reallocated by the gart at an alias physical
+ * address with a different cache model, so if ->release isn't
+ * implemented because all memory accesses through the
+ * secondary mmu implicitly are terminated by the time the
+ * last thread of this mm quits, you've also to be sure that
+ * speculative hardware operations can't allocate dirty
+ * cachelines in the cpu that could not be snooped and made
+ * coherent with the other read and write operations happening
+ * through the gart alias address, leading to memory
+ * corruption.
*/
void (*release)(struct mmu_notifier *mn,
struct mm_struct *mm);
diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
--- a/mm/mmu_notifier.c
+++ b/mm/mmu_notifier.c
@@ -59,7 +59,8 @@ void __mmu_notifier_release(struct mm_st
* from establishing any more sptes before all the
* pages in the mm are freed.
*/
- mn->ops->release(mn, mm);
+ if (mn->ops->release)
+ mn->ops->release(mn, mm);
srcu_read_unlock(&mm->mmu_notifier_mm->srcu, srcu);
spin_lock(&mm->mmu_notifier_mm->lock);
}
@@ -251,7 +252,8 @@ void mmu_notifier_unregister(struct mmu_
* guarantee ->release is called before freeing the
* pages.
*/
- mn->ops->release(mn, mm);
+ if (mn->ops->release)
+ mn->ops->release(mn, mm);
srcu_read_unlock(&mm->mmu_notifier_mm->srcu, srcu);
} else
spin_unlock(&mm->mmu_notifier_mm->lock);
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
