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]

Message-ID: <CA+55aFzotfXc07UoVtxvDpQOP8tEt8pgxeYe+cGs=BDUC_A4pA@mail.gmail.com>
Date:   Mon, 28 Aug 2017 09:48:34 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Liang, Kan" <kan.liang@...el.com>
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 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

View attachment "0001-sched-wait-Break-up-long-wake-list-walk.patch" of type "text/x-patch" (7004 bytes)

View attachment "0002-sched-wait-Introduce-wakeup-boomark-in-wake_up_page_.patch" of type "text/x-patch" (3759 bytes)