Cc: Andrew Morton <akpm@...>, Christoph Lameter <clameter@...>, Robin Holt <holt@...>, Nick Piggin <npiggin@...>, Peter Zijlstra <a.p.zijlstra@...>, <kvm-devel@...>, Kanoj Sarcar <kanojsarcar@...>, Roland Dreier <rdreier@...>, Steve Wise <swise@...>, <linux-kernel@...>, Avi Kivity <avi@...>, <linux-mm@...>, <general@...>, Hugh Dickins <hugh@...>, Rusty Russell <rusty@...>, Anthony Liguori <aliguori@...>, Chris Wright <chrisw@...>, Marcelo Tosatti <marcelo@...>, Eric Dumazet <dada1@...>, Paul E. McKenney <paulmck@...>
On Mon, May 05, 2008 at 12:25:06PM -0500, Jack Steiner wrote:
No problem.
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.
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);
--
