linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: David Rientjes <rientjes@google.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Greg Thelen <gthelen@google.com>,
	Minchan Kim <minchan@kernel.org>, Mel Gorman <mgorman@suse.de>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Michal Nazarewicz <mina86@mina86.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Christoph Lameter <cl@linux.com>, Rik van Riel <riel@redhat.com>
Subject: Re: [RFC PATCH 6/6] mm, compaction: don't migrate in blocks that cannot be fully compacted in async direct compaction
Date: Thu, 12 Jun 2014 14:18:12 +0200	[thread overview]
Message-ID: <53999A84.7010105@suse.cz> (raw)
In-Reply-To: <alpine.DEB.2.02.1406090156340.24247@chino.kir.corp.google.com>

On 06/09/2014 11:06 AM, David Rientjes wrote:
> On Fri, 6 Jun 2014, Vlastimil Babka wrote:
>
>>> Agreed.  I was thinking higher than 1GB would be possible once we have
>>> your series that does the pageblock skip for thp, I think the expense
>>> would be constant because we won't needlessly be migrating pages unless it
>>> has a good chance at succeeding.
>>
>> Looks like a counter of iterations actually done in scanners, maintained in
>> compact_control, would work better than any memory size based limit? It could
>> better reflect the actual work done and thus latency. Maybe increase the counter
>> also for migrations, with a higher cost than for a scanner iteration.
>>
>
> I'm not sure we can expose that to be configurable by userspace in any
> meaningful way.  We'll want to be able to tune this depending on the size
> of the machine if we are to truly remove the need_resched() heuristic and
> give it a sane default.  I was thinking it would be similar to
> khugepaged's pages_to_scan value that it uses on each wakeup.

Perhaps userspace can see the value in memory size unit, which would be 
translated to pages_to_scan assuming the worst case, i.e. scanning each 
page? Which would be used to limit the iterations, so if we end up 
skipping blocks of pages instead of single pages for whatever reasons, 
we can effectively scan a bigger memory size with the same effort?

>>> This does beg the question about parallel direct compactors, though, that
>>> will be contending on the same coarse zone->lru_lock locks and immediately
>>> aborting and falling back to PAGE_SIZE pages for thp faults that will be
>>> more likely if your patch to grab the high-order page and return it to the
>>> page allocator is merged.
>>
>> Hm can you explain how the page capturing makes this worse? I don't see it.
>>
>
> I was expecting that your patch to capture the high-order page made a
> difference because the zone watermark check doesn't imply the high-order
> page will be allocatable after we return to the page allocator to allocate
> it.  In that case, we terminated compaction prematurely.

In fact compact_finished() uses both a watermark check and then a 
free_list check. Only if both pass, it exits. But page allocation then 
does another watermark check which may fail (due to its raciness and 
drift) even though the page is still available on the free_list.

> If that's true,
> then it seems like no parallel thp allocator will be able to allocate
> memory that another direct compactor has freed without entering compaction
> itself on a fragmented machine, and thus an increase in zone->lru_lock
> contention if there's migratable memory.

I think it's only fair if someone who did the compaction work can 
allocate the page. Another compaction then has to do its own work, so in 
the end it's 2 units of work for 2 allocations (assuming success). 
Without the fairness, it might be 2 units of work by single allocator, 
for 2 successful allocations of two allocators. Or, as you seem to 
imply, 1 unit of work for 1 successful allocation, because the one doing 
the work will terminate prematurely and end up without allocation.
If we really rely on this premature termination as a contention 
prevention, then it seems quite unfair and fragile to me.

> Having 32 cpus fault thp memory and all entering compaction and contending
> (and aborting because of contention, currently) on zone->lru_lock is a
> really bad situation.

I'm not sure if the premature termination could prevent this reliably. I 
rather doubt that. The lock contention checks should work just fine in 
this case. And also I don't think it's that bad if they abort due to 
contention, if it happens quickly. It means that in such situation, it's 
simply a better performance tradeoff to give up on THP and fallback to 
4k allocation. Also you say "currently" but we are not going to change 
that for lock contention, are we?

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

  reply	other threads:[~2014-06-12 12:18 UTC|newest]

Thread overview: 135+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-01  0:45 [patch 1/2] mm, migration: add destination page freeing callback David Rientjes
2014-05-01  0:45 ` [patch 2/2] mm, compaction: return failed migration target pages back to freelist David Rientjes
2014-05-01  5:10   ` Naoya Horiguchi
2014-05-01 21:02     ` David Rientjes
2014-05-01  5:08 ` [patch 1/2] mm, migration: add destination page freeing callback Naoya Horiguchi
     [not found] ` <5361d71e.236ec20a.1b3d.ffffc8aeSMTPIN_ADDED_BROKEN@mx.google.com>
2014-05-01 21:02   ` David Rientjes
2014-05-01 21:35 ` [patch v2 1/4] " David Rientjes
2014-05-01 21:35   ` [patch v2 2/4] mm, compaction: return failed migration target pages back to freelist David Rientjes
2014-05-02 10:11     ` Mel Gorman
2014-05-02 15:23     ` Vlastimil Babka
2014-05-02 15:26       ` [PATCH] mm/compaction: do not count migratepages when unnecessary Vlastimil Babka
2014-05-06 21:18         ` Naoya Horiguchi
     [not found]         ` <1399411134-k43fsr0p@n-horiguchi@ah.jp.nec.com>
2014-05-07  9:33           ` Vlastimil Babka
2014-05-02 15:27       ` [PATCH 2/2] mm/compaction: avoid rescanning pageblocks in isolate_freepages Vlastimil Babka
2014-05-06 22:19         ` Naoya Horiguchi
     [not found]         ` <1399414778-xakujfb3@n-horiguchi@ah.jp.nec.com>
2014-05-07  9:22           ` Vlastimil Babka
2014-05-02 15:29       ` [PATCH 1/2] mm/compaction: do not count migratepages when unnecessary Vlastimil Babka
2014-05-01 21:35   ` [patch v2 3/4] mm, compaction: add per-zone migration pfn cache for async compaction David Rientjes
2014-05-05  9:34     ` Vlastimil Babka
2014-05-05  9:51       ` David Rientjes
2014-05-05 14:24         ` Vlastimil Babka
2014-05-06  0:29           ` David Rientjes
2014-05-06 11:52             ` Vlastimil Babka
2014-05-01 21:35   ` [patch v2 4/4] mm, thp: do not perform sync compaction on pagefault David Rientjes
2014-05-02 10:22     ` Mel Gorman
2014-05-02 11:22       ` David Rientjes
2014-05-02 11:58         ` Mel Gorman
2014-05-02 20:29           ` David Rientjes
2014-05-05 14:48             ` Vlastimil Babka
2014-05-06  8:55             ` Mel Gorman
2014-05-06 15:05               ` Vlastimil Babka
2014-05-02 10:10   ` [patch v2 1/4] mm, migration: add destination page freeing callback Mel Gorman
2014-05-07  2:22   ` [patch v3 1/6] " David Rientjes
2014-05-07  2:22     ` [patch v3 2/6] mm, compaction: return failed migration target pages back to freelist David Rientjes
2014-05-07 14:14       ` Naoya Horiguchi
2014-05-07 21:15       ` Andrew Morton
2014-05-07 21:21         ` David Rientjes
2014-05-12  8:35           ` Vlastimil Babka
2014-05-07 21:39         ` Greg Thelen
2014-05-12  8:37           ` Vlastimil Babka
2014-05-07  2:22     ` [patch v3 3/6] mm, compaction: add per-zone migration pfn cache for async compaction David Rientjes
2014-05-07  9:34       ` Vlastimil Babka
2014-05-07 20:56       ` Naoya Horiguchi
2014-05-07  2:22     ` [patch v3 4/6] mm, compaction: embed migration mode in compact_control David Rientjes
2014-05-07  9:55       ` Vlastimil Babka
2014-05-07 10:36         ` [patch v4 " David Rientjes
2014-05-09 22:03           ` Andrew Morton
2014-05-07  2:22     ` [patch v3 5/6] mm, thp: avoid excessive compaction latency during fault David Rientjes
2014-05-07  9:39       ` Mel Gorman
2014-05-08  5:30       ` [patch -mm] mm, thp: avoid excessive compaction latency during fault fix David Rientjes
2014-05-13 10:00         ` Vlastimil Babka
2014-05-22  2:49           ` David Rientjes
2014-05-22  8:43             ` Vlastimil Babka
2014-05-07  2:22     ` [patch v3 6/6] mm, compaction: terminate async compaction when rescheduling David Rientjes
2014-05-07  9:41       ` Mel Gorman
2014-05-07 12:09       ` [PATCH v2 1/2] mm/compaction: do not count migratepages when unnecessary Vlastimil Babka
2014-05-07 12:09         ` [PATCH v2 2/2] mm/compaction: avoid rescanning pageblocks in isolate_freepages Vlastimil Babka
2014-05-07 21:47           ` David Rientjes
2014-05-07 22:06           ` Naoya Horiguchi
2014-05-08  5:28           ` Joonsoo Kim
2014-05-12  9:09             ` Vlastimil Babka
2014-05-13  1:15               ` Joonsoo Kim
2014-05-09 15:49           ` Michal Nazarewicz
2014-05-19 10:14           ` Vlastimil Babka
2014-05-22  2:51             ` David Rientjes
2014-05-07 21:44         ` [PATCH v2 1/2] mm/compaction: do not count migratepages when unnecessary David Rientjes
2014-05-09 15:48         ` Michal Nazarewicz
2014-05-12  9:51           ` Vlastimil Babka
2014-05-07 12:10       ` [patch v3 6/6] mm, compaction: terminate async compaction when rescheduling Vlastimil Babka
2014-05-07 21:20       ` Andrew Morton
2014-05-07 21:28         ` David Rientjes
2014-05-08  5:17       ` Joonsoo Kim
2014-05-12 14:15         ` [PATCH] mm, compaction: properly signal and act upon lock and need_sched() contention Vlastimil Babka
2014-05-12 15:34           ` Naoya Horiguchi
     [not found]           ` <1399908847-ouuxeneo@n-horiguchi@ah.jp.nec.com>
2014-05-12 15:45             ` Vlastimil Babka
2014-05-12 15:53               ` Naoya Horiguchi
2014-05-12 20:28           ` David Rientjes
2014-05-13  8:50             ` Vlastimil Babka
2014-05-13  0:44           ` Joonsoo Kim
2014-05-13  8:54             ` Vlastimil Babka
2014-05-15  2:21               ` Joonsoo Kim
2014-05-16  9:47           ` [PATCH v2] " Vlastimil Babka
2014-05-16 17:33             ` Michal Nazarewicz
2014-05-19 23:37             ` Andrew Morton
2014-05-21 14:13               ` Vlastimil Babka
2014-05-21 20:11                 ` Andrew Morton
2014-05-22  3:20             ` compaction is still too expensive for thp (was: [PATCH v2] mm, compaction: properly signal and act upon lock and need_sched() contention) David Rientjes
2014-05-22  8:10               ` compaction is still too expensive for thp Vlastimil Babka
2014-05-22  8:55                 ` David Rientjes
2014-05-22 12:03                   ` Vlastimil Babka
2014-06-04  0:29                     ` [patch -mm 1/3] mm: rename allocflags_to_migratetype for clarity David Rientjes
2014-06-04  0:29                       ` [patch -mm 2/3] mm, compaction: pass gfp mask to compact_control David Rientjes
2014-06-04  0:30                       ` [patch -mm 3/3] mm, compaction: avoid compacting memory for thp if pageblock cannot become free David Rientjes
2014-06-04 11:04                         ` Mel Gorman
2014-06-04 22:02                           ` David Rientjes
2014-06-04 16:07                         ` Vlastimil Babka
2014-06-04 16:11               ` [RFC PATCH 1/6] mm, compaction: periodically drop lock and restore IRQs in scanners Vlastimil Babka
2014-06-04 16:11                 ` [RFC PATCH 2/6] mm, compaction: skip rechecks when lock was already held Vlastimil Babka
2014-06-04 23:46                   ` David Rientjes
2014-06-04 16:11                 ` [RFC PATCH 3/6] mm, compaction: remember position within pageblock in free pages scanner Vlastimil Babka
2014-06-04 16:11                 ` [RFC PATCH 4/6] mm, compaction: skip buddy pages by their order in the migrate scanner Vlastimil Babka
2014-06-05  0:02                   ` David Rientjes
2014-06-05  9:24                     ` Vlastimil Babka
2014-06-05 21:30                       ` David Rientjes
2014-06-06  7:20                         ` Vlastimil Babka
2014-06-09  9:09                           ` David Rientjes
2014-06-09 11:35                             ` Vlastimil Babka
2014-06-09 22:25                               ` David Rientjes
2014-06-10  7:26                                 ` Vlastimil Babka
2014-06-10 23:54                                   ` David Rientjes
2014-06-11 12:18                                     ` Vlastimil Babka
2014-06-12  0:21                                       ` David Rientjes
2014-06-12 11:56                                         ` Vlastimil Babka
2014-06-12 21:48                                           ` David Rientjes
2014-06-04 16:11                 ` [RFC PATCH 5/6] mm, compaction: try to capture the just-created high-order freepage Vlastimil Babka
2014-06-04 16:11                 ` [RFC PATCH 6/6] mm, compaction: don't migrate in blocks that cannot be fully compacted in async direct compaction Vlastimil Babka
2014-06-05  0:08                   ` David Rientjes
2014-06-05 15:38                     ` Vlastimil Babka
2014-06-05 21:38                       ` David Rientjes
2014-06-06  7:33                         ` Vlastimil Babka
2014-06-09  9:06                           ` David Rientjes
2014-06-12 12:18                             ` Vlastimil Babka [this message]
2014-06-04 23:39                 ` [RFC PATCH 1/6] mm, compaction: periodically drop lock and restore IRQs in scanners David Rientjes
2014-06-05  9:05                   ` Vlastimil Babka
2014-05-22 23:49             ` [PATCH v2] mm, compaction: properly signal and act upon lock and need_sched() contention Kevin Hilman
2014-05-23  2:48               ` Shawn Guo
2014-05-23  8:34                 ` Vlastimil Babka
2014-05-23 10:49                   ` Shawn Guo
2014-05-23 15:07                   ` Kevin Hilman
2014-05-30 16:59                   ` Stephen Warren
2014-06-02 13:35                   ` Fabio Estevam
2014-06-02 14:33                     ` [PATCH -mm] mm, compaction: properly signal and act upon lock and need_sched() contention - fix Vlastimil Babka
2014-06-02 15:18                       ` Fabio Estevam
2014-06-02 20:09                       ` David Rientjes
2014-05-02 13:16 ` [patch 1/2] mm, migration: add destination page freeing callback Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53999A84.7010105@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=gthelen@google.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mina86@mina86.com \
    --cc=minchan@kernel.org \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox