linux-kernel - RE: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 29 Aug 2017 12:57:06 +0000
From:   "Liang, Kan" <kan.liang@...el.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
CC:     Tim Chen <tim.c.chen@...ux.intel.com>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Peter Zijlstra <peterz@...radead.org>,
        "Ingo Molnar" <mingo@...e.hu>, Andi Kleen <ak@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Johannes Weiner <hannes@...xchg.org>, Jan Kara <jack@...e.cz>,
        Christopher Lameter <cl@...ux.com>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        Davidlohr Bueso <dave@...olabs.net>,
        linux-mm <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in
 wake_up_page_bit

> On Mon, Aug 28, 2017 at 7:51 AM, Liang, Kan <kan.liang@...el.com> 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 patches fix the long wait issue.

Tested-by: Kan Liang <kan.liang@...el.com>

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