Re: CONFIG_VMAP_STACK, on-stack struct, and wake_up_bit
From: Mel Gorman <hidden>
Date: 2016-10-27 09:18:10
Also in:
lkml
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:quoted
quoted
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.
quoted hunk ↗ jump to hunk
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 -- 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>