From: Nicholas Piggin <npiggin@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
Dave Hansen <dave.hansen@linux.intel.com>,
Bob Peterson <rpeterso@redhat.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Steven Whitehouse <swhiteho@redhat.com>,
Andrew Lutomirski <luto@kernel.org>,
Andreas Gruenbacher <agruenba@redhat.com>,
Mel Gorman <mgorman@techsingularity.net>,
linux-mm <linux-mm@kvack.org>, Hugh Dickins <hughd@google.com>
Subject: Re: [RFC][PATCH] make global bitlock waitqueues per-node
Date: Thu, 22 Dec 2016 05:01:30 +1000 [thread overview]
Message-ID: <20161222050130.49d93982@roar.ozlabs.ibm.com> (raw)
In-Reply-To: <20161222043331.31aab9cc@roar.ozlabs.ibm.com>
On Thu, 22 Dec 2016 04:33:31 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:
> On Wed, 21 Dec 2016 10:02:27 -0800
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> > I do think your approach of just re-using the existing bit waiting
> > with just a page-specific waiting function is nicer than Nick's "let's
> > just roll new waiting functions" approach. It also avoids the extra
> > initcall.
> >
> > Nick, comments?
>
> Well yes we should take my patch 1 and use the new bit for this
> purpose regardless of what way we go with patch 2. I'll reply to
> that in the other mail.
Actually when I hit send, I thought your next mail was addressing a
different subject. So back here.
Peter's patch is less code and in that regard a bit nicer. I tried
going that way once, but I just thought it was a bit too sloppy to
do nicely with wait bit APIs.
- The page can be added to waitqueue without PageWaiters being set.
This is transient condition where the lock is retested, but it
remains that PageWaiters is not quite the same as waitqueue_active
to some degree.
- This set + retest means every time a page gets a waiter, the cost
is 2 test-and-set for the lock bit plus 2 spin_lock+spin_unlock for
the waitqueue.
- Setting PageWaiters is done outside the waitqueue lock, so you also
have a new interleavings to think about versus clearing the bit.
- It fails to clear up the bit and return to fastpath when there are
hash collisions. Yes I know this is a rare case and on average it
probably does not matter. But jitter is important, but also we
really *want* to keep the waitqueue table small and lean like you
have made it if possible. None of this 100KB per zone crap -- I do
want to keep it small and tolerating collisions better would help
that.
Anyway that's about my 2c. Keep in mind Mel just said he might have
seen a lockup with Peter's patch, and mine has not been hugely tested
either, so let's wait for a bit more testing before merging either.
Although we could start pipelining the process by merging patch 1 if
Hugh acks it (cc'ed), then I'll resend with SOB and Ack.
Thanks,
Nick
--
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:[~2016-12-21 19:02 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-19 22:58 [RFC][PATCH] make global bitlock waitqueues per-node Dave Hansen
2016-12-19 23:07 ` Linus Torvalds
2016-12-19 23:12 ` Linus Torvalds
2016-12-20 0:20 ` Dave Hansen
2016-12-20 2:31 ` Nicholas Piggin
2016-12-20 12:58 ` Mel Gorman
2016-12-20 13:21 ` Nicholas Piggin
2016-12-20 17:31 ` Linus Torvalds
2016-12-20 18:02 ` Linus Torvalds
2016-12-21 8:09 ` Peter Zijlstra
2016-12-21 8:32 ` Peter Zijlstra
2016-12-21 18:02 ` Linus Torvalds
2016-12-21 18:33 ` Nicholas Piggin
2016-12-21 19:01 ` Nicholas Piggin [this message]
2016-12-21 19:50 ` Linus Torvalds
2016-12-22 2:07 ` Nicholas Piggin
2016-12-22 19:28 ` Hugh Dickins
2016-12-21 10:26 ` Nicholas Piggin
2016-12-20 2:26 ` Nicholas Piggin
2016-12-21 12:30 ` Nicholas Piggin
2016-12-21 18:12 ` Linus Torvalds
2016-12-21 18:40 ` Nicholas 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=20161222050130.49d93982@roar.ozlabs.ibm.com \
--to=npiggin@gmail.com \
--cc=agruenba@redhat.com \
--cc=dave.hansen@linux.intel.com \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@kernel.org \
--cc=mgorman@techsingularity.net \
--cc=peterz@infradead.org \
--cc=rpeterso@redhat.com \
--cc=swhiteho@redhat.com \
--cc=torvalds@linux-foundation.org \
/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).