From: Andrea Arcangeli <aarcange@redhat.com>
To: Avi Kivity <avi@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Rik van Riel <riel@redhat.com>,
linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
Kent Overstreet <kent.overstreet@gmail.com>,
Ingo Molnar <mingo@elte.hu>
Subject: Re: [COUNTERPATCH] mm: avoid overflowing preempt_count() in mmu_take_all_locks()
Date: Thu, 1 Apr 2010 17:54:56 +0200 [thread overview]
Message-ID: <20100401155456.GR5825@random.random> (raw)
In-Reply-To: <4BB4BE44.5070507@redhat.com>
On Thu, Apr 01, 2010 at 06:39:48PM +0300, Avi Kivity wrote:
> On 04/01/2010 06:36 PM, Andrea Arcangeli wrote:
> > On Thu, Apr 01, 2010 at 01:16:32PM +0200, Peter Zijlstra wrote:
> >
> >> On Thu, 2010-04-01 at 14:13 +0300, Avi Kivity wrote:
> >>
> >>
> >>> If someone is willing to audit all code paths to make sure these locks
> >>> are always taken in schedulable context I agree that's a better fix.
> >>>
> >> They had better be, they're not irq-safe. Also that's what lockdep is
> >> for.
> >>
> > In my original patchset I included patches from Christoph to convert
> > those locks to mutexes, there was apparently no problem at all with
> > that. But frankly I think the only problem here is the warning. The
> > only compliant we ever had here is from developers, no users at
> > all. If this was a practical problem I think we should have heard
> > something by now with so many KVM users out there (and gru too).
> >
> > The only single reason I'd go for mutexes would be to accommodate
> > XPMEM requirements once and for all, no other reason.
> >
>
> There is also a minor benefit for kvm. Reduced latency over large mmu
> operations; code simplification (we now have some
> copy_from_user_inatomic() that could be simplified).
Where exactly is KVM taking these locks? KVM should only call into
GUP, and GUP itself won't iterate over rmaps either. GUP just walks
the host pagetables and trigger page faults if the pages aren't
mapped. I don't see how you're going to remove
copy_from_user_inatomic() given we don't have vmas and other metadata
to take those locks. Maybe we can stop calling GUP but even if we take
the anon_vma mutex/semaphore I think it won't still prevent munmap to
drop the anon pages from under us (even if it'd stop the VM to unmap
them through rmap). To freeze the mapping one would need to take
mmap_sem in write mode in addition to the anon_vma mutex/sem which is
unlikely a win compared to just gup+copy_from_user_inatomic. So I
don't see immediate benefits for KVM but maybe I'm missing something
of course!
next prev parent reply other threads:[~2010-04-01 16:44 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-30 17:36 [PATCH] increase PREEMPT_BITS to 12 to avoid overflow when starting KVM Rik van Riel
2010-03-30 17:56 ` Peter Zijlstra
2010-03-30 18:05 ` Rik van Riel
2010-03-30 18:34 ` Peter Zijlstra
2010-04-01 9:40 ` [COUNTERPATCH] mm: avoid overflowing preempt_count() in mmu_take_all_locks() Avi Kivity
2010-04-01 10:31 ` Peter Zijlstra
2010-04-01 11:04 ` Thomas Gleixner
2010-04-01 11:13 ` Avi Kivity
2010-04-01 11:16 ` Peter Zijlstra
2010-04-01 11:19 ` Avi Kivity
2010-04-01 15:36 ` Andrea Arcangeli
2010-04-01 15:39 ` Avi Kivity
2010-04-01 15:54 ` Andrea Arcangeli [this message]
2010-04-01 16:02 ` Avi Kivity
2010-04-01 16:12 ` Andrea Arcangeli
2010-04-01 11:17 ` Avi Kivity
2010-04-01 11:27 ` Peter Zijlstra
2010-04-01 11:43 ` Peter Zijlstra
2010-04-01 11:47 ` Avi Kivity
2010-04-01 15:42 ` Andrea Arcangeli
2010-04-01 15:50 ` Peter Zijlstra
2010-04-01 15:56 ` Peter Zijlstra
2010-04-01 16:07 ` Andrea Arcangeli
2010-04-01 16:32 ` Peter Zijlstra
2010-04-01 16:00 ` Andrea Arcangeli
2010-04-01 15:51 ` Avi Kivity
2010-04-01 15:56 ` Peter Zijlstra
2010-04-01 16:06 ` Avi Kivity
2010-04-01 16:15 ` Paul E. McKenney
2010-04-01 16:36 ` Peter Zijlstra
2010-04-01 17:02 ` Paul E. McKenney
2010-04-01 16:08 ` Andrea Arcangeli
2010-04-01 16:14 ` Paul E. McKenney
2010-04-01 16:02 ` Andrea Arcangeli
2010-04-01 16:12 ` Peter Zijlstra
2010-04-01 16:18 ` Andrea Arcangeli
2010-04-01 16:45 ` Peter Zijlstra
2010-04-01 16:49 ` Peter Zijlstra
2010-04-01 17:04 ` Andrea Arcangeli
2010-04-01 14:16 ` Rik van Riel
2010-04-01 15:32 ` Andrea Arcangeli
2010-04-01 15:37 ` Avi Kivity
2010-04-01 11:09 ` Avi Kivity
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=20100401155456.GR5825@random.random \
--to=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=avi@redhat.com \
--cc=kent.overstreet@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=riel@redhat.com \
--cc=tglx@linutronix.de \
/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).