linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hugh Dickins <hugh@veritas.com>
To: Dave McCracken <dmccr@us.ibm.com>
Cc: Andrew Morton <akpm@osdl.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Linux Memory Management <linux-mm@kvack.org>
Subject: Re: [PATCH 1/1] Implement shared page tables
Date: Wed, 31 Aug 2005 12:44:24 +0100 (BST)	[thread overview]
Message-ID: <Pine.LNX.4.61.0508311143340.15467@goblin.wat.veritas.com> (raw)
In-Reply-To: <7C49DFF721CB4E671DB260F9@[10.1.1.4]>

On Tue, 30 Aug 2005, Dave McCracken wrote:
> 
> This patch implements page table sharing for all shared memory regions that
> span an entire page table page.  It supports sharing at multiple page
> levels, depending on the architecture.
> 
> Performance testing has shown no degradation with this patch for tests with
> small processes.  Preliminary tests with large benchmarks have shown as
> much as 3% improvement in overall results.

Hmm.  A few points.

> The patch is against 2.6.13.

So you don't have Nick's test at the start of copy_page_range():
	if (!(vma->vm_flags & (VM_HUGETLB|VM_NONLINEAR|VM_RESERVED))) {
		if (!vma->anon_vma)
			return 0;
	}
Experimental, yes, but Linus likes it enough to have fast-tracked it into
his tree for 2.6.14.  My guess is that that patch (if its downsides prove
manageable) takes away a lot of the point of shared page tables -
I wonder how much of your "3% improvement".

I was going to say, doesn't randomize_va_space take away the rest of
the point?  But no, it appears "randomize_va_space", as it currently
appears in mainline anyway, is somewhat an exaggeration: it just shifts
the stack a little, with no effect on the rest of the va space.
But if it is to do more later, it may conflict with your interest.

The pud sharing and pmd sharing: perhaps they complicate the patch for
negligible benefit?

> +		if ((vma->vm_start <= base) &&
> +	    (vma->vm_end >= end))
> +		return 1;
> 
New Adventures in Coding Style ;)

But most seriously: search the patch for the string "lock" and I find
no change whatever to locking.  You're introducing page tables shared
between different mms yet relying on the old mm->page_table_lock?
You're searching a prio_tree for suitable matches to share, but
taking no lock on that?  You're counting shares in an atomic,
but not detecting when the count falls to 0 atomically?

And allied with that point on locking mms: there's no change to rmap.c,
so how is its TLB flushing and cache flushing now supposed to work?
page_referenced_one and try_to_unmap_one will visit all the vmas
sharing the page table, yes, but (usually) only the first will
satisfy the conditions and get flushed.

I'm not sure if it's worth pursuing shared page tables again or not.

You certainly need to sort the locking out to do so.  Wait a couple
of weeks and I should have sent all the per-page-table-page locking
in to -mm (to replace the pte xchging currently there): that should
give what you need for locking pts independent of the mm.

Hugh

  reply	other threads:[~2005-08-31 11:42 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-08-30 22:13 [PATCH 1/1] Implement shared page tables Dave McCracken
2005-08-31 11:44 ` Hugh Dickins [this message]
2005-08-31 11:51   ` Arjan van de Ven
2005-08-31 13:42     ` Hugh Dickins
2005-08-31 14:31       ` Martin J. Bligh
2005-08-31 14:41         ` Arjan van de Ven
2005-08-31 15:06         ` Hugh Dickins
2005-08-31 15:39           ` Martin J. Bligh
2005-08-31 16:40   ` Dave McCracken
2005-09-02  1:58 ` Chen, Kenneth W
2005-09-02 16:40   ` Dave McCracken
2005-09-02  4:26 ` Chen, Kenneth W

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=Pine.LNX.4.61.0508311143340.15467@goblin.wat.veritas.com \
    --to=hugh@veritas.com \
    --cc=akpm@osdl.org \
    --cc=dmccr@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).