Message-ID: <1380495986.2174.10.camel@buesod1.americas.hpqcorp.net>
Date:	Sun, 29 Sep 2013 16:06:26 -0700
From:	Davidlohr Bueso <davidlohr@...com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Ingo Molnar <mingo@...nel.org>, Waiman Long <Waiman.Long@...com>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Rik van Riel <riel@...hat.com>,
	Peter Hurley <peter@...leysoftware.com>,
	Davidlohr Bueso <davidlohr.bueso@...com>,
	Alex Shi <alex.shi@...el.com>,
	Tim Chen <tim.c.chen@...ux.intel.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Matthew R Wilcox <matthew.r.wilcox@...el.com>,
	Dave Hansen <dave.hansen@...el.com>,
	Michel Lespinasse <walken@...gle.com>,
	Andi Kleen <andi@...stfloor.org>,
	"Chandramouleeswaran, Aswin" <aswin@...com>,
	"Norton, Scott J" <scott.norton@...com>
Subject: Re: [PATCH] rwsem: reduce spinlock contention in wakeup code path

On Sat, 2013-09-28 at 11:55 -0700, Linus Torvalds wrote:
> On Sat, Sep 28, 2013 at 12:41 AM, Ingo Molnar <mingo@...nel.org> wrote:
> >
> >
> > Yeah, I fully agree. The reason I'm still very sympathetic to Tim's
> > efforts is that they address a regression caused by a mechanic
> > mutex->rwsem conversion:
> >
> >   5a505085f043 mm/rmap: Convert the struct anon_vma::mutex to an rwsem
> >
> > ... and Tim's patches turn that regression into an actual speedup.
> 
> Btw, I really hate that thing. I think we should turn it back into a
> spinlock. None of what it protects needs a mutex or an rwsem.

The same should apply to i_mmap_mutex, having a similar responsibility
to the anon-vma lock with file backed pages. A few months ago I had
suggested changing that lock to rwsem, giving some pretty reasonable
performance improvement numbers.

http://lwn.net/Articles/556342/

> 
> Because you guys talk about the regression of turning it into a rwsem,
> but nobody talks about the *original* regression.
> 
> And it *used* to be a spinlock, and it was changed into a mutex back
> in 2011 by commit 2b575eb64f7a. That commit doesn't even have a reason
> listed for it, although my dim memory of it is that the reason was
> preemption latency.
> 
> And that caused big regressions too.

After testing Ingo's anon-vma rwlock_t conversion (v2) on a 8 socket, 80
core system with aim7, I am quite surprised about the numbers -
considering the lack of queuing in rwlocks. A lot of the tests didn't
show hardly any difference, but those that really contend this lock
(with high amounts of users) benefited quite nicely:

Alltests: +28% throughput after 1000 users and runtime was reduced from
7.2 to 6.6 secs.

Custom: +61% throughput after 100 users and runtime was reduced from 7
to 4.9 secs.

High_systime: +40% throughput after 1000 users and runtime was reduced
from 19 to 15.5 secs.

Shared: +30.5% throughput after 100 users and runtime was reduced from
6.5 to 5.1 secs.

Short: Lots of variance in the numbers, but avg of +29% throughput - no
particular performance degradation either.

The only workload that took a hit was fserver with a -6% throughput for
small amounts of users (10-100).

Theoretically at least, adding queuing to the rwlocks should only help
the situation furthermore. I'll also run some test for a similar
conversion for the i_mmap mutex next week.

Going back to the rwsems, I believe adding optimistic spinning is still
valid, independent of the anon-vma lock as it benefits all users.

Thanks,
Davidlohr

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists