Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1720257 > unrolled thread

[PATCH 1/2 v2] sched/wait: Break up long wake list walk

Started byTim Chen <tim.c.chen@linux.intel.com>
First post2017-08-25 18:40 +0200
Last post2017-08-25 19:50 +0200
Articles 6 on this page of 26 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 1/2 v2] sched/wait: Break up long wake list walk Tim Chen <tim.c.chen@linux.intel.com> - 2017-08-25 18:40 +0200
    [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Tim Chen <tim.c.chen@linux.intel.com> - 2017-08-25 18:40 +0200
      Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-25 22:00 +0200
        Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit Tim Chen <tim.c.chen@linux.intel.com> - 2017-08-26 00:20 +0200
          Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-26 01:10 +0200
            Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-26 02:40 +0200
              Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-26 05:00 +0200
                Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-26 20:20 +0200
                  Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-27 23:50 +0200
                  Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-27 23:50 +0200
                    Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-28 01:20 +0200
                      Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit Nicholas Piggin <npiggin@gmail.com> - 2017-08-28 03:20 +0200
                        Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit Nicholas Piggin <npiggin@gmail.com> - 2017-08-28 03:40 +0200
                          Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-28 07:20 +0200
                            Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit Nicholas Piggin <npiggin@gmail.com> - 2017-08-28 09:20 +0200
                  RE: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit "Liang, Kan" <kan.liang@intel.com> - 2017-08-28 17:00 +0200
                    Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-28 18:50 +0200
                      Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit Tim Chen <tim.c.chen@linux.intel.com> - 2017-08-28 22:10 +0200
                      RE: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit "Liang, Kan" <kan.liang@intel.com> - 2017-08-29 15:00 +0200
                        Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-29 18:10 +0200
                          Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit Tim Chen <tim.c.chen@linux.intel.com> - 2017-08-29 18:20 +0200
                            Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-29 18:30 +0200
                              Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit Tim Chen <tim.c.chen@linux.intel.com> - 2017-08-29 19:00 +0200
                      Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in  wake_up_page_bit Tim Chen <tim.c.chen@linux.intel.com> - 2017-08-29 18:20 +0200
                        Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-29 18:30 +0200
    Re: [PATCH 1/2 v2] sched/wait: Break up long wake list walk Christopher Lameter <cl@linux.com> - 2017-08-25 19:50 +0200

Page 2 of 2 — ← Prev page 1 [2]


#1722595 — Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit

FromTim Chen <tim.c.chen@linux.intel.com>
Date2017-08-29 18:20 +0200
SubjectRe: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit
Message-ID<ujRp8-4QC-7@gated-at.bofh.it>
In reply to#1722589
On 08/29/2017 09:01 AM, Linus Torvalds wrote:
> On Tue, Aug 29, 2017 at 5:57 AM, Liang, Kan <kan.liang@intel.com> wrote:
>>>
>>> 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@intel.com>
> 
> Ok. I'm not 100% comfortable applying them at rc7, so let me think
> about it. There's only one known load triggering this, and by "known"
> I mean "not really known" since we don't even know what the heck it
> does outside of intel and whoever your customer is.
> 
> So I suspect I'll apply the patches next merge window, and we can
> maybe mark them for stable if this actually ends up mattering.
> 
> Can you tell if the problem is actually hitting _production_ use or
> was some kind of benchmark stress-test?
> 
> 

It is affecting not a production use, but the customer's acceptance
test for their systems.  So I suspect it is a stress test.

Tim

[toc] | [prev] | [next] | [standalone]


#1722601 — Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2017-08-29 18:30 +0200
SubjectRe: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit
Message-ID<ujRyO-4TP-11@gated-at.bofh.it>
In reply to#1722595
On Tue, Aug 29, 2017 at 9:13 AM, Tim Chen <tim.c.chen@linux.intel.com> wrote:
>
> It is affecting not a production use, but the customer's acceptance
> test for their systems.  So I suspect it is a stress test.

Can you gently poke them and ask if they might make theie stress test
code available?

Tell them that we have a fix, but right now it's delayed into 4.14
because we have no visibility into what it is that it actually fixes,
and whether it's all that critical or just some microbenchmark.

Thanks,

           Linus

[toc] | [prev] | [next] | [standalone]


#1722612 — Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit

FromTim Chen <tim.c.chen@linux.intel.com>
Date2017-08-29 19:00 +0200
SubjectRe: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit
Message-ID<ujS1P-54r-3@gated-at.bofh.it>
In reply to#1722601
On 08/29/2017 09:24 AM, Linus Torvalds wrote:
> On Tue, Aug 29, 2017 at 9:13 AM, Tim Chen <tim.c.chen@linux.intel.com> wrote:
>>
>> It is affecting not a production use, but the customer's acceptance
>> test for their systems.  So I suspect it is a stress test.
> 
> Can you gently poke them and ask if they might make theie stress test
> code available?
> 
> Tell them that we have a fix, but right now it's delayed into 4.14
> because we have no visibility into what it is that it actually fixes,
> and whether it's all that critical or just some microbenchmark.
> 

Thanks. We'll do that.

Tim

[toc] | [prev] | [next] | [standalone]


#1722594 — Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit

FromTim Chen <tim.c.chen@linux.intel.com>
Date2017-08-29 18:20 +0200
SubjectRe: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit
Message-ID<ujRp8-4QC-5@gated-at.bofh.it>
In reply to#1721806
On 08/28/2017 09:48 AM, Linus Torvalds wrote:
> On Mon, Aug 28, 2017 at 7:51 AM, Liang, Kan <kan.liang@intel.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:

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
> 

[toc] | [prev] | [next] | [standalone]


#1722599 — Re: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2017-08-29 18:30 +0200
SubjectRe: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit
Message-ID<ujRyN-4TP-3@gated-at.bofh.it>
In reply to#1722594
On Tue, Aug 29, 2017 at 9:17 AM, Tim Chen <tim.c.chen@linux.intel.com> wrote:
>
> BTW, are you going to add the chunk below separately as part of your
> wait queue cleanup patch?

I did.

Commit 9c3a815f471a ("page waitqueue: always add new entries at the end")

               Linus

[toc] | [prev] | [next] | [standalone]


#1720315

FromChristopher Lameter <cl@linux.com>
Date2017-08-25 19:50 +0200
Message-ID<uiqU3-71g-23@gated-at.bofh.it>
In reply to#1720257
On Fri, 25 Aug 2017, Tim Chen wrote:

> for a long time.  It is a result of the numa balancing migration of hot
> pages that are shared by many threads.

I think that would also call for some work to limit numa balacing of hot
shared pages. The cache lines of hot pages are likely in present the low
level processor caches anyways so moving them would not cause a
performance benefit. Limiting the migration there could stop wasting a lot
of effort.

[toc] | [prev] | [standalone]


Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel


csiph-web