linux-kernel - Re: REGRESSION: Performance regressions from switching anon

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 17 Jun 2011 11:01:08 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Hugh Dickins <hughd@...gle.com>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Tim Chen <tim.c.chen@...ux.intel.com>,
	Andi Kleen <ak@...ux.intel.com>,
	Shaohua Li <shaohua.li@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	David Miller <davem@...emloft.net>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Russell King <rmk@....linux.org.uk>,
	Paul Mundt <lethal@...ux-sh.org>,
	Jeff Dike <jdike@...toit.com>,
	Richard Weinberger <richard@....at>,
	"Luck, Tony" <tony.luck@...el.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Mel Gorman <mel@....ul.ie>, Nick Piggin <npiggin@...nel.dk>,
	Namhyung Kim <namhyung@...il.com>,
	"Shi, Alex" <alex.shi@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>
Subject: Re: REGRESSION: Performance regressions from switching anon_vma->lock
 to mutex

On Fri, Jun 17, 2011 at 10:41 AM, Hugh Dickins <hughd@...gle.com> wrote:
>
> Applying load with those two patches applied (combined patch shown at
> the bottom, in case you can tell me I misunderstood what to apply,
> and have got the wrong combination on), lockdep very soon protested.

Combined patch looks good, it's just the one without the NULL ptr tests removed.

And yup, that makes sense. Since we now hold the anon_vma lock over an
allocation, the memory allocation might want to start to free things.

> I've not given it _any_ thought, and won't be able to come back to
> it for a couple of hours: chucked over the wall for your delectation.

It's a mis-feature of "page_referenced()": we can call it without
holding any page locks ("is_locked=0"), and that function will then do
a try_lock() on the page, and just consider it referenced if it
failed.

HOWEVER, the code to then do the same thing for the anon_vma lock
doesn't do the same trylock model, because it used to be a spinlock:
so there was "no need" (since you can never do a non-atomic allocation
from within a spinlock).

So this is arguably a bug in memory reclaim, but sicne the
mutex->spinlock conversion had been pretty mindless, nobody noticed
until the mutex region grew.

So I do think that "page_referenced_anon()" should do a trylock, and
return "referenced" if the trylock fails. Comments?

That said, we have a few other mutexes that are just not allowed to be
held over an allocation. page_referenced_file() has that
mapping->i_mmap_mutex lock, for example. So maybe the rule just has to
be "you cannot hold anon_vma lock over an allocation". Which would be
sad: one of the whole _points_ of turning it from a spinlock to a
mutex would be that it relaxes the locking rules a lot (and not just
the preemptibility)

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