linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] [ARM] Do not call flush_cache_user_range with mmap_sem held
Date: Thu, 29 Apr 2010 19:24:55 +0100	[thread overview]
Message-ID: <20100429182455.GH4877@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20100429181636.GA25894@shareable.org>

On Thu, Apr 29, 2010 at 07:16:36PM +0100, Jamie Lokier wrote:
> Russell King - ARM Linux wrote:
> > On Wed, Apr 28, 2010 at 12:32:11AM -0700, Dima Zavin wrote:
> > > We can't be holding the mmap_sem while calling flush_cache_user_range
> > > because the flush can fault. If we fault on a user address, the
> > > page fault handler will try to take mmap_sem again. Since both places
> > > acquire the read lock, most of the time it succeeds. However, if another
> > > thread tries to acquire the write lock on the mmap_sem (e.g. mmap) in
> > > between the call to flush_cache_user_range and the fault, the down_read
> > > in do_page_fault will deadlock.
> > 
> > That's a non-issue.  If do_cache_op is holding a read lock, _nothing_
> > _else_ can be holding that lock in write mode.  So, holding the lock in
> > read mode ensures that when faults occur, the fault handler can be sure
> > that its read lock will succeed.
> 
> read-write locks will block a reader when there is a already blocked
> writer.  Otherwise the writer can be permanently starved due to new
> readers always arriving so reader count doesn't reach zero.

Hmm, true, and rather unfortunate.

As I've already said, we can not do this cache maintainence outside of
the lock.

The point of this code is to first validate that the region we're working
on is valid.  As soon as we drop the lock, we lose the guarantee that
the region is valid while we operate on it - indeed, the region could be
unmapped and remapped by a different thread.

I think the only reasonable solution is to also walk the page tables
and do the cache handling on a per-page basis, which will make this
interface quite a bit slower - but that's the price we pay for
correctness.

  reply	other threads:[~2010-04-29 18:24 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-28  7:32 [PATCH] [ARM] Do not call flush_cache_user_range with mmap_sem held Dima Zavin
2010-04-28  7:35 ` Dima Zavin
2010-04-29 13:00 ` Russell King - ARM Linux
2010-04-29 18:16   ` Jamie Lokier
2010-04-29 18:24     ` Russell King - ARM Linux [this message]
2010-04-29 19:23       ` Dima Zavin
2010-05-04  4:07         ` Dima Zavin
2010-05-04  7:40           ` Russell King - ARM Linux
2010-05-06 15:00       ` [PATCH] [ARM] Do not call flush_cache_user_range with mmap_semheld Catalin Marinas
2010-05-06 16:01         ` Jamie Lokier
2010-05-06 16:07           ` Jamie Lokier
2010-05-06 16:24             ` Catalin Marinas
2010-05-06 16:21           ` Catalin Marinas
2010-05-06 15:08 ` [PATCH] [ARM] Do not call flush_cache_user_range with mmap_sem held Catalin Marinas
  -- strict thread matches above, loose matches on Subject: below --
2011-11-07 17:33 [PATCH] ARM: " Catalin Marinas
2011-11-16 21:23 ` Olof Johansson
2011-11-16 23:50   ` Russell King - ARM Linux
2011-11-17  0:16     ` Olof Johansson
2011-11-17  0:20       ` Olof Johansson
2011-11-17 10:26       ` Catalin Marinas
2011-11-17 10:49         ` Russell King - ARM Linux
2011-11-17 10:45       ` Russell King - ARM Linux
2011-11-20 17:54         ` Olof Johansson
2011-11-17 10:22     ` Catalin Marinas
2011-11-17 10:42       ` Russell King - ARM Linux
2011-11-17 10:59         ` Catalin Marinas
2011-11-17 11:03           ` Russell King - ARM Linux
2011-11-17 11:25             ` Catalin Marinas
2012-04-09  5:58               ` Dirk Behme
2012-04-09 14:24                 ` Olof Johansson
2012-04-10 17:17                   ` Will Deacon
2012-04-18 15:05                     ` Will Deacon
2012-04-18 15:27                       ` Russell King - ARM Linux
2012-04-18 16:27                         ` Will Deacon
2012-04-18 17:15                         ` Catalin Marinas
2012-04-18  8:40                   ` Catalin Marinas

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=20100429182455.GH4877@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.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).