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

[Christoph Lameter]

On Tue, 4 Mar 2008, Peter Zijlstra wrote:

> 
> On Tue, 2008-03-04 at 14:35 -0800, Christoph Lameter wrote:
> 
> > RCU means that the callbacks occur in an atomic context.
> 
> Not really, if it requires moving the VM locks to sleepable locks under
> a .config option, I think its also fair to require PREEMPT_RCU.

Which would make the patchset pretty complex. RCU is not needed with a 
single linked list. Linked list operations can exploit atomic pointer 
updates and we only tear down the list when a single execution thread 
remains.


Having said that: Here a couple of updates to address Andrea's complaint 
that we not check the referenced bit from the external mapper when the 
rerferences bit is set on an OS pte.

Plus two barriers to ensure that a new emm notifier object becomes
visible before the base pointer is updated.

Signed-off-by: Christoph Lameter <clameter at sgi.com>

---
 mm/rmap.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: linux-2.6/mm/rmap.c
===================================================================
--- linux-2.6.orig/mm/rmap.c	2008-03-04 14:36:36.321922321 -0800
+++ linux-2.6/mm/rmap.c	2008-03-04 15:10:46.159429369 -0800
@@ -298,10 +298,10 @@ static int page_referenced_one(struct pa
 
 	(*mapcount)--;
 	pte_unmap_unlock(pte, ptl);
-	if (!referenced)
-		/* rmap lock held */
-		referenced = emm_notify(mm, emm_referenced,
-					address, address + PAGE_SIZE);
+
+	/* rmap lock held */
+	if (emm_notify(mm, emm_referenced, address, address + PAGE_SIZE))
+			referenced = 1;
 out:
 	return referenced;
 }
@@ -1057,6 +1057,7 @@ EXPORT_SYMBOL_GPL(emm_notifier_release);
 void emm_notifier_register(struct emm_notifier *e, struct mm_struct *mm)
 {
 	e->next = mm->emm_notifier;
+	smp_wmb();
 	mm->emm_notifier = e;
 }
 EXPORT_SYMBOL_GPL(emm_notifier_register);
@@ -1069,6 +1070,7 @@ int __emm_notify(struct mm_struct *mm, e
 	int x;
 
 	while (e) {
+		smp_rmb();
 		if (e->func) {
 			x = e->func(e, mm, op, start, end);
 			if (x)