Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935898AbcJ0JSK (ORCPT ); Thu, 27 Oct 2016 05:18:10 -0400 Received: from outbound-smtp08.blacknight.com ([46.22.139.13]:47670 "EHLO outbound-smtp08.blacknight.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935124AbcJ0JRm (ORCPT ); Thu, 27 Oct 2016 05:17:42 -0400 Date: Thu, 27 Oct 2016 10:07:42 +0100 From: Mel Gorman To: Peter Zijlstra Cc: Linus Torvalds , Andy Lutomirski , Andreas Gruenbacher , Andy Lutomirski , LKML , Bob Peterson , Steven Whitehouse , linux-mm Subject: Re: CONFIG_VMAP_STACK, on-stack struct, and wake_up_bit Message-ID: <20161027090742.GG2699@techsingularity.net> References: <20161026203158.GD2699@techsingularity.net> <20161026220339.GE2699@techsingularity.net> <20161026230726.GF2699@techsingularity.net> <20161027080852.GC3568@worktop.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <20161027080852.GC3568@worktop.programming.kicks-ass.net> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2744 Lines: 77 On Thu, Oct 27, 2016 at 10:08:52AM +0200, Peter Zijlstra wrote: > On Thu, Oct 27, 2016 at 12:07:26AM +0100, Mel Gorman wrote: > > > but I consider PeterZ's > > > patch the fix to that, so I wouldn't worry about it. > > > > > > > Agreed. Peter, do you plan to finish that patch? > > I was waiting for you guys to hash out the 32bit issue. But if we're now > OK with having this for 64bit only, I can certainly look at doing a new > version. > I've no problem with it being 64-bit only. > I'll have to look at fixing Alpha's bitops for that first though, > because as is that patch relies on atomics to the same word not needing > ordering, but placing the contended/waiters bit in the high word for > 64bit only sorta breaks that. > I see the problem assuming you're referring to the requirement that locked and waiter bits are on the same word. Without it, you need a per-arch helper that forces ordering or takes a spinlock. I doubt it's worth the trouble. > Hurm, we could of course play games with the layout, the 64bit only > flags don't _have_ to be at the end. > > 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. > We could re-instate this with an #ifndef CONFIG_NUMA I suppose.. not > pretty though. > > Also did the s/contended/waiters/ rename per popular request. > > --- > include/linux/page-flags.h | 19 ++++++++ > include/linux/pagemap.h | 25 ++++++++-- > include/trace/events/mmflags.h | 7 +++ > mm/filemap.c | 94 +++++++++++++++++++++++++++++++++++++---- > 4 files changed, 130 insertions(+), 15 deletions(-) > > --- a/include/linux/page-flags.h > +++ 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. Otherwise, nothing jumped out at me but glancing through it looked very similar to the previous patch. -- Mel Gorman SUSE Labs