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

[Peter Zijlstra]

On Tue, 2008-03-04 at 15:14 -0800, Christoph Lameter wrote:
> 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.

OK, that constraint on removal makes it work.

> 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;

referenced++; seems more in-style with the rest of that code..

>  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)

We generally require comments around barriers..