From: Hugh Dickins <hugh@veritas.com>
To: Nick Piggin <npiggin@suse.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
linux-arch@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux Memory Management List <linux-mm@kvack.org>
Subject: Re: [rfc] optimise unlock_page
Date: Thu, 10 May 2007 20:14:52 +0100 (BST) [thread overview]
Message-ID: <Pine.LNX.4.64.0705101935590.18496@blonde.wat.veritas.com> (raw)
In-Reply-To: <20070510033736.GA19196@wotan.suse.de>
On Thu, 10 May 2007, Nick Piggin wrote:
>
> OK, I found a simple bug after pulling out my hair for a while :)
> With this, a 4-way system survives a couple of concurrent make -j250s
> quite nicely (wheras they eventually locked up before).
>
> The problem is that the bit wakeup function did not go through with
> the wakeup if it found the bit (ie. PG_locked) set. This meant that
> waiters would not get a chance to reset PG_waiters.
That makes a lot of sense. And this version seems stable to me,
I've found no problems so far: magic!
Well, on the x86_64 I have seen a few of your io_schedule_timeout
printks under load; but suspect those are no fault of your changes,
but reflect some actual misbehaviour down towards the disk end (when
kernel default moved from AS to CFQ, I had to stick with AS because
CFQ ran my tests very much slower on that one machine: something odd
going on that I've occasionally wasted time looking into but never
tracked down - certainly long-locked pages are a feature of that).
> However you probably weren't referring to that particular problem
> when you imagined the need for a full count, or the slippery 3rd
> task... I wasn't able to derive any such problems with the basic
> logic, so if there was a bug there, it would still be unfixed in this
> patch.
I've been struggling to conjure up and exorcise the race that seemed
so obvious to me yesterday. I was certainly imagining one task on
its way between SetPageWaiters and io_schedule, when the unlock_page
comes, wakes, and lets another waiter take the lock. Probably I was
forgetting the essence of prepare_to_wait, that this task would then
fall through io_schedule as if woken as part of that batch. Until
demonstrated otherwise, let's assume I was utterly mistaken.
In addition to 3 hours of load on the three machines, I've gone back
and applied this new patch (and the lock bitops; remembering to shift
PG_waiters up) to 2.6.21-rc3-mm2 on which I did the earlier lmbench
testing, on those three machines.
On the PowerPC G5, these changes pretty much balance out your earlier
changes (not just the one fix-fault-vs-invalidate patch, but the whole
group which came in with that - it'd take me a while to tell exactly
what, easiest to send you a diff if you want it), in those lmbench
fork, exec, sh, mmap, fault tests. On the P4 Xeons, they improve
the numbers significantly, but only retrieve half the regression.
So here it looks like a good change; but not enough to atone ;)
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2007-05-10 19:14 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20070508113709.GA19294@wotan.suse.de>
2007-05-08 11:40 ` Nick Piggin
2007-05-08 20:08 ` Hugh Dickins
2007-05-08 21:30 ` Benjamin Herrenschmidt
2007-05-08 22:41 ` Nick Piggin
2007-05-08 22:50 ` Nick Piggin
2007-05-09 19:33 ` Hugh Dickins
2007-05-09 21:21 ` Benjamin Herrenschmidt
2007-05-10 3:37 ` Nick Piggin
2007-05-10 19:14 ` Hugh Dickins [this message]
2007-05-11 8:54 ` Nick Piggin
2007-05-11 13:15 ` Hugh Dickins
2007-05-13 3:32 ` Nick Piggin
2007-05-13 4:39 ` Hugh Dickins
2007-05-13 6:52 ` Nick Piggin
2007-05-16 17:54 ` Hugh Dickins
2007-05-16 18:18 ` Nick Piggin
2007-05-16 19:28 ` Hugh Dickins
2007-05-16 19:47 ` Linus Torvalds
2007-05-17 6:27 ` Nick Piggin
2007-05-16 17:21 ` Hugh Dickins
2007-05-16 17:38 ` Nick Piggin
2007-05-08 12:13 ` David Howells
2007-05-08 22:35 ` Nick Piggin
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=Pine.LNX.4.64.0705101935590.18496@blonde.wat.veritas.com \
--to=hugh@veritas.com \
--cc=akpm@linux-foundation.org \
--cc=benh@kernel.crashing.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=npiggin@suse.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