Re: Switching TestSetPageLocked to trylock_page

[Andrew Morton <akpm@linux-foundation.org>]

On Tue, 5 Aug 2008 13:41:53 +1000 Nick Piggin <nickpiggin@yahoo.com.au> wrote:

> On Tuesday 05 August 2008 13:28, Linus Torvalds wrote:
> > On Tue, 5 Aug 2008, Nick Piggin wrote:
> > > On Thursday 31 July 2008 17:26, Nick Piggin wrote:
> > > > Hi,
> > > >
> > > > I'm wondering if I could get a patch merged which changes all
> > > > TestSetPageLocked and replaces them with trylock_page?
> > >
> > > Yes? No?
> > >
> > > The alternative is try to merge it via -mm or -next, but that just
> > > wastes everybodies time with conflicts of having these differences
> > > between -mm and mainline.
> >
> > Heh. I had just been _assuming_ this would go through -mm, since it's
> > exactly the kind of thing that usually does go through there.
> >
> > So I hadn't even really considered it.
> 
> OK... it just causes Andrew headaches I suspect. But if he prefers
> to hold onto it for an entire release cycle... Andrew?
> 

Yup.  A mechanical rename-foo-to-bar can be prepared and merged late -
there's little payback for the pain of maintaining it for a couple of
months.

Would prefer either that we hold off until after 2.6.27 is released or
just do it now.

> 
> > I don't mind the patch per se, but can you give some background on what
> > the pending optimization is that makes such a big difference?
> 
> Using the lock semantics bitops is the first one. While it is true
> that we could just hack them into TestSetPageLocked, I really prefer
> callers to require at least a cursory glance to convert them, and
> understand that this is a lock lock, and not a test_and_set bitop
> with full barrier semantics.

This patch wouldn't do anything to ensure that callers get the review
which you describe, would it?

(and we don't need to patch the code to just read it!)

> lock semantics bitops obviously doesn't help x86 a bit. It really
> helps powerpc though.
> 
> The part that helps x86 is another patch further down my stack, which
> can avoid looking at the hashed page waitqueue at unlock_page-time.
> Basically this eliminates the entire page waitqueue from the cache
> footprint of fastpath workloads where the lock is uncontended (often:
> page faulting, write(2), writeout etc).
> 
> I'll send all that stuff off to -mm for 2.6.28 if this gets upstream
> (or into -mm).

I'm OK with that.