On 08/28/2017 09:48 AM, Linus Torvalds wrote: > On Mon, Aug 28, 2017 at 7:51 AM, Liang, Kan <kan.liang@xxxxxxxxx> wrote: >> >> I tried this patch and https://lkml.org/lkml/2017/8/27/222 together. >> But they don't fix the issue. I can still get the similar call stack. > > So the main issue was that I *really* hated Tim's patch #2, and the > patch to clean up the page wait queue should now make his patch series > much more palatable. > > Attached is an ALMOST COMPLETELY UNTESTED forward-port of those two > patches, now without that nasty WQ_FLAG_ARRIVALS logic, because we now > always put the new entries at the end of the waitqueue. > > The attached patches just apply directly on top of plain 4.13-rc7. > > That makes patch #2 much more palatable, since it now doesn't need to > play games and worry about new arrivals. > > But note the lack of testing. I've actually booted this and am running > these two patches right now, but honestly, you should consider them > "untested" simply because I can't trigger the page waiters contention > case to begin with. > > But it's really just Tim's patches, modified for the page waitqueue > cleanup which makes patch #2 become much simpler, and now it's > palatable: it's just using the same bookmark thing that the normal > wakeup uses, no extra hacks. > > So Tim should look these over, and they should definitely be tested on > that load-from-hell that you guys have, but if this set works, at > least I'm ok with it now. > > Tim - did I miss anything? I added a "cpu_relax()" in there between > the release lock and irq and re-take it, I'm not convinced it makes > any difference, but I wanted to mark that "take a breather" thing. > > Oh, there's one more case I only realized after the patches: the > stupid add_page_wait_queue() code still adds to the head of the list. > So technically you need this too: BTW, are you going to add the chunk below separately as part of your wait queue cleanup patch? Tim > > diff --git a/mm/filemap.c b/mm/filemap.c > index 74123a298f53..598c3be57509 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1061,7 +1061,7 @@ void add_page_wait_queue(struct page *page, > wait_queue_entry_t *waiter) > unsigned long flags; > > spin_lock_irqsave(&q->lock, flags); > - __add_wait_queue(q, waiter); > + __add_wait_queue_entry_tail(q, waiter); > SetPageWaiters(page); > spin_unlock_irqrestore(&q->lock, flags); > } > > but that only matters if you actually use the cachefiles thing, which > I hope/assume you don't. > > Linus > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
