linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nick Piggin <nickpiggin@yahoo.com.au>
To: William Lee Irwin III <wli@holomorphy.com>
Cc: Christoph Lameter <christoph@lameter.com>,
	Linux Memory Management <linux-mm@kvack.org>
Subject: Re: [PATCH 0/7] abstract pagetable locking and pte updates
Date: Tue, 02 Nov 2004 12:34:22 +1100	[thread overview]
Message-ID: <4186E41E.5080909@yahoo.com.au> (raw)
In-Reply-To: <20041102005439.GQ2583@holomorphy.com>

William Lee Irwin III wrote:
> On Fri, 29 Oct 2004, William Lee Irwin III wrote:
> 
>>>This raises the rather serious question of what you actually did
>>>besides rearranging Lameter's code. It had all the same problems;
>>>resolving them is a prerequisite to going anywhere with all this.
> 
> 
> On Mon, Nov 01, 2004 at 04:15:41PM -0800, Christoph Lameter wrote:
> 
>>Could you be specific as to the actual problems? I have worked through
>>several archs over time and my code offers a fallback to the use of the
>>page_table_lock if an arch does not provide the necessary atomic ops.
>>So what are the issues with my code? I fixed the PAE code based on Nick's
>>work. AFAIK this was the only known issue.
> 
> 
> Well, I'm not going to sit around and look for holes in this all day
> (that should have been done by the author), however it's not a priori
> true that decoupling locking surrounding tlb_flush_mmu() from pte
> locking is correct.
> 

It still turns preempt off, so you're pinned to the CPU.

It is not a problem for the arch independant code. tlb_xxx
are done after eg. ptep_get_and_clear, so there is no more
serialisation needed there.

The only problem would be architectures that rely on it in their
flush implementations; i386 doesn't look like it needs that. It
would only stop two flushes to the same mm from going on in
parallel, but its already taking a global lock there where it
matters anyway...

...And architectures like SPARC64 that rely on it for external
synchronisation. In that case you saw my trivial patch to revert
to the old behaviour for that arch.

But aside from all that, Christoph's patch _doesn't_ move the
locking out of tlb_gather operations IIRC.

> The audits behind this need to be better.
> 

Sure I haven't audited all architectures. I thought it was
pretty clear that one could fall back to the old behaviour
if required.

Why do you say the audits need to be better? No doubt there will
still be bugs, but I didn't just say "ahh let's remove the lock
from around the tlb operations and pray it works".

I could very well be missing something though - You must be
seeing some fundamental problems or nasty bugs to say that it's
been designed it in a vacuum, and that the audits are no good...
What are they please?
--
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:"aart@kvack.org"> aart@kvack.org </a>

  reply	other threads:[~2004-11-02  1:34 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-10-29  7:20 Nick Piggin
2004-10-29  7:20 ` [PATCH 1/7] " Nick Piggin
2004-10-29  7:21   ` [PATCH 2/7] " Nick Piggin
2004-10-29  7:21     ` [PATCH 3/7] " Nick Piggin
2004-10-29  7:21       ` [PATCH 4/7] " Nick Piggin
2004-10-29  7:22         ` [PATCH 5/7] " Nick Piggin
2004-10-29  7:23           ` [PATCH 6/7] " Nick Piggin
2004-10-29  7:23             ` [PATCH 7/7] " Nick Piggin
2004-10-29  7:46 ` [PATCH 0/7] " William Lee Irwin III
2004-11-02  0:15   ` Christoph Lameter
2004-11-02  0:54     ` William Lee Irwin III
2004-11-02  1:34       ` Nick Piggin [this message]
2004-11-02  1:55         ` William Lee Irwin III
2004-11-02  2:38           ` Nick Piggin
2004-11-02  6:57             ` William Lee Irwin III
2004-11-02 17:55         ` Christoph Lameter
2004-10-29 11:45 ` Nick Piggin
2004-10-29 20:52   ` William Lee Irwin III
2004-10-30  2:46     ` Nick Piggin
2004-11-02  0:19       ` Christoph Lameter

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=4186E41E.5080909@yahoo.com.au \
    --to=nickpiggin@yahoo.com.au \
    --cc=christoph@lameter.com \
    --cc=linux-mm@kvack.org \
    --cc=wli@holomorphy.com \
    /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