All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Nick Piggin <nickpiggin@yahoo.com.au>,
	linux-mm@kvack.org, Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH] mm: disable preemption in apply_to_pte_range
Date: Sat, 14 Feb 2009 10:56:01 +0100	[thread overview]
Message-ID: <1234605361.4698.23.camel@laptop> (raw)
In-Reply-To: <4995ACD5.9000201@goop.org>

On Fri, 2009-02-13 at 09:24 -0800, Jeremy Fitzhardinge wrote:
> Peter Zijlstra wrote:
> >> The specific rules are that 
> >> arch_enter_lazy_mmu_mode()/arch_leave_lazy_mmu_mode() require you to be 
> >> holding the appropriate pte locks for the ptes you're updating, so 
> >> preemption is naturally disabled in that case.
> >>     
> >
> > Right, except on -rt where the pte lock is a mutex.
> >   
> 
> Hm, that's interesting.  The requirement isn't really "no preemption", 
> its "must not migrate to another cpu".  Is there a better way to express 
> that?

Not really, in the past something like migrate_disable() has been
proposed, however that's problematic in that it can generate latencies
that are _very_ hard to track down, so we've always resisted that and
found other ways.

> >> This all goes a bit strange with init_mm's non-requirement for taking 
> >> pte locks.  The caller has to arrange for some kind of serialization on 
> >> updating the range in question, and that could be a mutex.  Explicitly 
> >> disabling preemption in enter_lazy_mmu_mode would make sense for this 
> >> case, but it would be redundant for the common case of batched updates 
> >> to usermode ptes.
> >>     
> >
> > I really utterly hate how you just plonk preempt_disable() in there
> > unconditionally and without very clear comments on how and why.
> >   
> 
> Well, there's the commit comment.  They're important, right?  That's why 
> we spend time writing good commit comments?  So they get read?  ;)

Andrew taught me that indeed, but still when looking at the code its
good to have some text there explaining things too.


WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Nick Piggin <nickpiggin@yahoo.com.au>,
	linux-mm@kvack.org, Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH] mm: disable preemption in apply_to_pte_range
Date: Sat, 14 Feb 2009 10:56:01 +0100	[thread overview]
Message-ID: <1234605361.4698.23.camel@laptop> (raw)
In-Reply-To: <4995ACD5.9000201@goop.org>

On Fri, 2009-02-13 at 09:24 -0800, Jeremy Fitzhardinge wrote:
> Peter Zijlstra wrote:
> >> The specific rules are that 
> >> arch_enter_lazy_mmu_mode()/arch_leave_lazy_mmu_mode() require you to be 
> >> holding the appropriate pte locks for the ptes you're updating, so 
> >> preemption is naturally disabled in that case.
> >>     
> >
> > Right, except on -rt where the pte lock is a mutex.
> >   
> 
> Hm, that's interesting.  The requirement isn't really "no preemption", 
> its "must not migrate to another cpu".  Is there a better way to express 
> that?

Not really, in the past something like migrate_disable() has been
proposed, however that's problematic in that it can generate latencies
that are _very_ hard to track down, so we've always resisted that and
found other ways.

> >> This all goes a bit strange with init_mm's non-requirement for taking 
> >> pte locks.  The caller has to arrange for some kind of serialization on 
> >> updating the range in question, and that could be a mutex.  Explicitly 
> >> disabling preemption in enter_lazy_mmu_mode would make sense for this 
> >> case, but it would be redundant for the common case of batched updates 
> >> to usermode ptes.
> >>     
> >
> > I really utterly hate how you just plonk preempt_disable() in there
> > unconditionally and without very clear comments on how and why.
> >   
> 
> Well, there's the commit comment.  They're important, right?  That's why 
> we spend time writing good commit comments?  So they get read?  ;)

Andrew taught me that indeed, but still when looking at the code its
good to have some text there explaining things too.

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

  reply	other threads:[~2009-02-14  9:56 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-13  0:21 [PATCH] mm: disable preemption in apply_to_pte_range Jeremy Fitzhardinge
2009-02-13  0:35 ` Jeremy Fitzhardinge
2009-02-13  0:55   ` Andrew Morton
2009-02-13  0:55     ` Andrew Morton
2009-02-13  1:39     ` Jeremy Fitzhardinge
2009-02-13  1:39       ` Jeremy Fitzhardinge
2009-02-13 11:48       ` Peter Zijlstra
2009-02-13 11:48         ` Peter Zijlstra
2009-02-13 13:30         ` Nick Piggin
2009-02-13 13:30           ` Nick Piggin
2009-02-13 14:16           ` Peter Zijlstra
2009-02-13 14:16             ` Peter Zijlstra
2009-02-13 14:30             ` Nick Piggin
2009-02-13 14:30               ` Nick Piggin
2009-02-13 14:38               ` Peter Zijlstra
2009-02-13 14:38                 ` Peter Zijlstra
2009-02-13 17:41                 ` Jeremy Fitzhardinge
2009-02-13 17:41                   ` Jeremy Fitzhardinge
2009-02-14  9:46                   ` Peter Zijlstra
2009-02-14  9:46                     ` Peter Zijlstra
2009-02-13 17:24         ` Jeremy Fitzhardinge
2009-02-13 17:24           ` Jeremy Fitzhardinge
2009-02-14  9:56           ` Peter Zijlstra [this message]
2009-02-14  9:56             ` Peter Zijlstra

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=1234605361.4698.23.camel@laptop \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@elte.hu \
    --cc=nickpiggin@yahoo.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.