linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Mel Gorman <mgorman@techsingularity.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Andreas Gruenbacher <agruenba@redhat.com>,
	Andy Lutomirski <luto@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Bob Peterson <rpeterso@redhat.com>,
	Steven Whitehouse <swhiteho@redhat.com>,
	linux-mm <linux-mm@kvack.org>
Subject: Re: CONFIG_VMAP_STACK, on-stack struct, and wake_up_bit
Date: Thu, 27 Oct 2016 11:44:49 +0200	[thread overview]
Message-ID: <20161027094449.GL3102@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <20161027090742.GG2699@techsingularity.net>

On Thu, Oct 27, 2016 at 10:07:42AM +0100, Mel Gorman wrote:
> > Something like so could work I suppose, but then there's a slight
> > regression in the page_unlock() path, where we now do an unconditional
> > spinlock; iow. we loose the unlocked waitqueue_active() test.
> > 
> 
> I can't convince myself it's worthwhile. At least, I can't see a penalty
> of potentially moving one of the two bits to the high word. It's the
> same cache line and the same op when it matters.

I'm having trouble connecting these here two paragraphs. Or were you
replying to something else?

So the current unlock code does:

  wake_up_page()
    if (waitqueue_active())
      __wake_up() /* takes waitqueue spinlocks here */

While the new one does:

  spin_lock(&q->lock);
  if (waitqueue_active()) {
    __wake_up_common()
  }
  spin_unlock(&q->lock);

Which is an unconditional atomic op (which go for about ~20 cycles each,
when uncontended).


> > +++ b/include/linux/page-flags.h
> > @@ -73,6 +73,14 @@
> >   */
> >  enum pageflags {
> >  	PG_locked,		/* Page is locked. Don't touch. */
> > +#ifdef CONFIG_NUMA
> > +	/*
> > +	 * This bit must end up in the same word as PG_locked (or any other bit
> > +	 * we're waiting on), as per all architectures their bitop
> > +	 * implementations.
> > +	 */
> > +	PG_waiters,		/* The hashed waitqueue has waiters */
> > +#endif
> >  	PG_error,
> >  	PG_referenced,
> >  	PG_uptodate,
> 
> I don't see why it should be NUMA-specific even though with Linus'
> patch, NUMA is a concern. Even then, you still need a 64BIT check
> because 32BIT && NUMA is allowed on a number of architectures.

Oh, I thought we killed 32bit NUMA and didn't check. I can make it
CONFIG_64BIT and be done with it. s/CONFIG_NUMA/CONFIG_64BIT/ on the
patch should do :-)

> Otherwise, nothing jumped out at me but glancing through it looked very
> similar to the previous patch.

Right, all the difference was in the bit being conditional and having a
different name.

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

  reply	other threads:[~2016-10-27  9:44 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAHc6FU4e5sueLi7pfeXnSbuuvnc5PaU3xo5Hnn=SvzmQ+ZOEeg@mail.gmail.com>
     [not found] ` <CALCETrUt+4ojyscJT1AFN5Zt3mKY0rrxcXMBOUUJzzLMWXFXHg@mail.gmail.com>
2016-10-26 16:32   ` Linus Torvalds
2016-10-26 17:15     ` Linus Torvalds
2016-10-26 17:58       ` Linus Torvalds
2016-10-26 18:04         ` Bob Peterson
2016-10-26 18:10           ` Linus Torvalds
2016-10-26 19:11             ` Bob Peterson
2016-10-26 21:01             ` Bob Peterson
2016-10-26 21:30               ` Linus Torvalds
2016-10-26 22:45                 ` Borislav Petkov
2016-10-26 23:13               ` Borislav Petkov
2016-10-27  0:37                 ` Bob Peterson
2016-10-27 12:36                   ` Borislav Petkov
2016-10-27 18:51                     ` Bob Peterson
2016-10-27 19:19                       ` Borislav Petkov
2016-10-27 21:03                         ` Bob Peterson
2016-10-27 21:19                           ` Borislav Petkov
2016-10-28  8:37                     ` [tip:x86/urgent] x86/microcode/AMD: Fix more fallout from CONFIG_RANDOMIZE_MEMORY=y tip-bot for Borislav Petkov
2016-10-26 20:31       ` CONFIG_VMAP_STACK, on-stack struct, and wake_up_bit Mel Gorman
2016-10-26 21:26         ` Linus Torvalds
2016-10-26 22:03           ` Mel Gorman
2016-10-26 22:09             ` Linus Torvalds
2016-10-26 23:07               ` Mel Gorman
2016-10-27  8:08                 ` Peter Zijlstra
2016-10-27  9:07                   ` Mel Gorman
2016-10-27  9:44                     ` Peter Zijlstra [this message]
2016-10-27  9:59                       ` Mel Gorman
2016-10-27 11:56                   ` 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=20161027094449.GL3102@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=agruenba@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mgorman@techsingularity.net \
    --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