All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Holt <holt@sgi.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Avi Kivity <avi@qumranet.com>,
	Christoph Lameter <clameter@sgi.com>,
	Andrea Arcangeli <andrea@qumranet.com>, Robin Holt <holt@sgi.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH] Minimal mmu notifiers for kvm
Date: Fri, 25 Apr 2008 06:12:43 -0500	[thread overview]
Message-ID: <20080425111243.GA17326@sgi.com> (raw)
In-Reply-To: <200804250813.00792.rusty@rustcorp.com.au>

This patch would require GRU to maintain its own page tables and hold
reference counts on the pages.  That seems like a complete waste of
memory compared to Andrea's most recent patch.  The invalidate_range_start
and invalidate_range_end pair is needed to eliminate the page reference
counts.  The _start callout sets an internal structure in a state that
prevents GRU from satisfying faults, then executes the GRU instruction
to flush the TLB entry.  The _end callout releases the block on faults.

On Fri, Apr 25, 2008 at 08:13:00AM +1000, Rusty Russell wrote:
> +static DEFINE_SPINLOCK(notifier_lock);
> +
> +/*
> + * Must not hold mmap_sem nor any other VM related lock when calling
> + * this registration function.
> + */
> +int mm_add_notifier_ops(struct mm_struct *mm,
> +			const struct mmu_notifier_ops *mops)
> +{
> +	int err;
> +
> +	spin_lock(&notifier_lock);

This one global lock will get extremely hot when a 4096 MPI rank job
is starting up and every one of them goes to use the GRU at once.  I am
not sure where x86_64 peaks out, but on ia64 going beyond approx 32 cpus
contending for the same lock made starvation a very important issue.

> +	if (mm->mmu_notifier_ops)
> +		err = -EBUSY;

So we can only use one of KVM or GRU or Quadrix or IB or (later) XPMEM
per mm?

> +	else {
> +		mm->mmu_notifier_ops = mops;
> +		err = 0;
> +	}
> +	spin_unlock(&notifier_lock);
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(mm_add_notifier_ops);

Robin

WARNING: multiple messages have this Message-ID (diff)
From: Robin Holt <holt@sgi.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Avi Kivity <avi@qumranet.com>,
	Christoph Lameter <clameter@sgi.com>,
	Andrea Arcangeli <andrea@qumranet.com>, Robin Holt <holt@sgi.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH] Minimal mmu notifiers for kvm
Date: Fri, 25 Apr 2008 06:12:43 -0500	[thread overview]
Message-ID: <20080425111243.GA17326@sgi.com> (raw)
In-Reply-To: <200804250813.00792.rusty@rustcorp.com.au>

This patch would require GRU to maintain its own page tables and hold
reference counts on the pages.  That seems like a complete waste of
memory compared to Andrea's most recent patch.  The invalidate_range_start
and invalidate_range_end pair is needed to eliminate the page reference
counts.  The _start callout sets an internal structure in a state that
prevents GRU from satisfying faults, then executes the GRU instruction
to flush the TLB entry.  The _end callout releases the block on faults.

On Fri, Apr 25, 2008 at 08:13:00AM +1000, Rusty Russell wrote:
> +static DEFINE_SPINLOCK(notifier_lock);
> +
> +/*
> + * Must not hold mmap_sem nor any other VM related lock when calling
> + * this registration function.
> + */
> +int mm_add_notifier_ops(struct mm_struct *mm,
> +			const struct mmu_notifier_ops *mops)
> +{
> +	int err;
> +
> +	spin_lock(&notifier_lock);

This one global lock will get extremely hot when a 4096 MPI rank job
is starting up and every one of them goes to use the GRU at once.  I am
not sure where x86_64 peaks out, but on ia64 going beyond approx 32 cpus
contending for the same lock made starvation a very important issue.

> +	if (mm->mmu_notifier_ops)
> +		err = -EBUSY;

So we can only use one of KVM or GRU or Quadrix or IB or (later) XPMEM
per mm?

> +	else {
> +		mm->mmu_notifier_ops = mops;
> +		err = 0;
> +	}
> +	spin_unlock(&notifier_lock);
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(mm_add_notifier_ops);

Robin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2008-04-25 11:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-24 22:13 [PATCH] Minimal mmu notifiers for kvm Rusty Russell
2008-04-24 22:13 ` Rusty Russell
2008-04-24 23:53 ` Andrea Arcangeli
2008-04-24 23:53   ` Andrea Arcangeli
2008-04-25 11:12 ` Robin Holt [this message]
2008-04-25 11:12   ` Robin Holt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080425111243.GA17326@sgi.com \
    --to=holt@sgi.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrea@qumranet.com \
    --cc=avi@qumranet.com \
    --cc=clameter@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rusty@rustcorp.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.