From: Nicholas Piggin <firstname.lastname@example.org> To: Linus Torvalds <email@example.com> Cc: Dave Hansen <firstname.lastname@example.org>, Bob Peterson <email@example.com>, Linux Kernel Mailing List <firstname.lastname@example.org>, Steven Whitehouse <email@example.com>, Andrew Lutomirski <firstname.lastname@example.org>, Andreas Gruenbacher <email@example.com>, Peter Zijlstra <firstname.lastname@example.org>, linux-mm <email@example.com>, Mel Gorman <firstname.lastname@example.org> Subject: Re: [PATCH 2/2] mm: add PageWaiters indicating tasks are waiting for a page bit Date: Thu, 29 Dec 2016 15:26:15 +1000 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <CA+55aFxGz8R8J9jLvKpLUgyhWVYcgtObhbHBP7eZzZyc05AODw@mail.gmail.com> On Wed, 28 Dec 2016 20:16:56 -0800 Linus Torvalds <firstname.lastname@example.org> wrote: > On Wed, Dec 28, 2016 at 8:08 PM, Nicholas Piggin <email@example.com> wrote: > > > > Okay. The name could be a bit better though I think, for readability. > > Just a BUILD_BUG_ON if it is not constant and correct bit numbers? > > I have a slightly edited patch - moved the comments around and added > some new comments (about both the sign bit, but also about how the > smp_mb() shouldn't be necessary even for the non-atomic fallback). That's a good point -- they're in the same byte, so all architectures will be able to avoid the extra barrier regardless of how the primitives are implemented. Good. > > I also did a BUILD_BUG_ON(), except the other way around - keeping it > about the sign bit in the byte, just just verifying that yes, > PG_waiters is that sign bit. Yep. I still don't like the name, but now you've got PG_waiters commented there at least. I'll have to live with it. If we get more cases that want to use a similar function, we might make a more general primitive for architectures that can optimize these multi bit ops better than x86. Actually even x86 would prefer to do load ; cmpxchg rather than bitop ; load for the cases where condition code can't be used to check result. > > > BTW. I just notice in your patch too that you didn't use "nr" in the > > generic version. > > And I fixed that too. > > Of course, I didn't test the changes (apart from building it). But > I've been running the previous version since yesterday, so far no > issues. It looks good to me. Thanks, Nick
next prev parent reply other threads:[~2016-12-29 5:26 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-12-25 3:00 [PATCH 0/2] PageWaiters again Nicholas Piggin 2016-12-25 3:00 ` [PATCH 1/2] mm: Use owner_priv bit for PageSwapCache, valid when PageSwapBacked Nicholas Piggin 2016-12-25 5:13 ` Hugh Dickins 2016-12-25 3:00 ` [PATCH 2/2] mm: add PageWaiters indicating tasks are waiting for a page bit Nicholas Piggin 2016-12-25 21:51 ` Linus Torvalds 2016-12-26 1:16 ` Nicholas Piggin 2016-12-26 19:07 ` Linus Torvalds 2016-12-27 11:19 ` Nicholas Piggin 2016-12-27 18:58 ` Linus Torvalds 2016-12-27 19:23 ` Linus Torvalds 2016-12-27 19:24 ` Linus Torvalds 2016-12-27 19:40 ` Linus Torvalds 2016-12-27 20:17 ` Linus Torvalds 2016-12-28 3:53 ` Nicholas Piggin 2016-12-28 19:17 ` Linus Torvalds 2016-12-29 4:08 ` Nicholas Piggin 2016-12-29 4:16 ` Linus Torvalds 2016-12-29 5:26 ` Nicholas Piggin [this message] 2017-01-03 10:24 ` Mel Gorman 2017-01-03 12:29 ` Nicholas Piggin 2017-01-03 17:18 ` Mel Gorman 2016-12-29 22:16 ` [PATCH] mm/filemap: fix parameters to test_bit() Olof Johansson
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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH 2/2] mm: add PageWaiters indicating tasks are waiting for a page bit' \ /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
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).