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

Peter Zijlstra
2008-03-04T23:30:53Z

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@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..