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


Groups > linux.kernel > #1601158 > unrolled thread

[PATCH v2 0/5] mm: support parallel free of memory

Started byAaron Lu <aaron.lu@intel.com>
First post2017-03-15 10:10 +0100
Last post2017-03-17 03:30 +0100
Articles 20 on this page of 29 — 8 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-15 10:10 +0100
    [PATCH v2 2/5] mm: parallel free pages Aaron Lu <aaron.lu@intel.com> - 2017-03-15 10:10 +0100
      Re: [PATCH v2 2/5] mm: parallel free pages "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2017-03-15 10:50 +0100
        Re: [PATCH v2 2/5] mm: parallel free pages Aaron Lu <aaron.lu@intel.com> - 2017-03-15 13:00 +0100
    Re: [PATCH v2 0/5] mm: support parallel free of memory Michal Hocko <mhocko@kernel.org> - 2017-03-15 15:30 +0100
      Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-15 16:50 +0100
        Re: [PATCH v2 0/5] mm: support parallel free of memory Michal Hocko <mhocko@kernel.org> - 2017-03-15 17:30 +0100
          Re: [PATCH v2 0/5] mm: support parallel free of memory Tim Chen <tim.c.chen@linux.intel.com> - 2017-03-15 22:40 +0100
            Re: [PATCH v2 0/5] mm: support parallel free of memory Michal Hocko <mhocko@kernel.org> - 2017-03-16 10:10 +0100
              Re: [PATCH v2 0/5] mm: support parallel free of memory Tim Chen <tim.c.chen@linux.intel.com> - 2017-03-16 19:40 +0100
                Re: [PATCH v2 0/5] mm: support parallel free of memory Minchan Kim <minchan@kernel.org> - 2017-03-17 09:10 +0100
                Re: [PATCH v2 0/5] mm: support parallel free of memory Michal Hocko <mhocko@kernel.org> - 2017-03-17 09:30 +0100
                  Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-17 13:40 +0100
                    Re: [PATCH v2 0/5] mm: support parallel free of memory Michal Hocko <mhocko@kernel.org> - 2017-03-17 14:00 +0100
                    Re: [PATCH v2 0/5] mm: support parallel free of memory Peter Zijlstra <peterz@infradead.org> - 2017-03-17 14:20 +0100
                  Re: [PATCH v2 0/5] mm: support parallel free of memory Peter Zijlstra <peterz@infradead.org> - 2017-03-17 14:00 +0100
                    Re: [PATCH v2 0/5] mm: support parallel free of memory Michal Hocko <mhocko@kernel.org> - 2017-03-17 14:30 +0100
              Re: [PATCH v2 0/5] mm: support parallel free of memory Dave Hansen <dave.hansen@intel.com> - 2017-03-21 16:10 +0100
                Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-22 09:20 +0100
                Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-24 08:10 +0100
              Re: [PATCH v2 0/5] mm: support parallel free of memory Tim Chen <tim.c.chen@linux.intel.com> - 2017-03-21 16:20 +0100
          Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-16 08:00 +0100
          Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-16 08:40 +0100
            Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-16 15:00 +0100
              Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-16 15:20 +0100
    Re: [PATCH v2 0/5] mm: support parallel free of memory Vlastimil Babka <vbabka@suse.cz> - 2017-03-15 16:00 +0100
      Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-15 17:00 +0100
      Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-17 04:20 +0100
    Re: [PATCH v2 0/5] mm: support parallel free of memory Aaron Lu <aaron.lu@intel.com> - 2017-03-17 03:30 +0100

Page 1 of 2  [1] 2  Next page →


#1601158 — [PATCH v2 0/5] mm: support parallel free of memory

FromAaron Lu <aaron.lu@intel.com>
Date2017-03-15 10:10 +0100
Subject[PATCH v2 0/5] mm: support parallel free of memory
Message-ID<tld6q-5vn-13@gated-at.bofh.it>
For regular processes, the time taken in its exit() path to free its
used memory is not a problem. But there are heavy ones that consume
several Terabytes memory and the time taken to free its memory in its
exit() path could last more than ten minutes if THP is not used.

As Dave Hansen explained why do this in kernel:
"
One of the places we saw this happen was when an app crashed and was
exit()'ing under duress without cleaning up nicely.  The time that it
takes to unmap a few TB of 4k pages is pretty excessive.
"

To optimize this use case, a parallel free method is proposed here and
it is based on the current gather batch free(the following description
is taken from patch 2/5's changelog).

The current gather batch free works like this:
For each struct mmu_gather *tlb, there is a static buffer to store those
to-be-freed page pointers. The size is MMU_GATHER_BUNDLE, which is
defined to be 8. So if a tlb tear down doesn't free more than 8 pages,
that is all we need. If 8+ pages are to be freed, new pages will need
to be allocated to store those to-be-freed page pointers.

The structure used to describe the saved page pointers is called
struct mmu_gather_batch and tlb->local is of this type. tlb->local is
different than other struct mmu_gather_batch(es) in that the page
pointer array used by tlb->local points to the previouslly described
static buffer while the other struct mmu_gather_batch(es) page pointer
array points to the dynamically allocated pages.

These batches will form a singly linked list, starting from &tlb->local.

tlb->local.pages  => tlb->pages(8 pointers)
      \|/
      next => batch1->pages => about 510 pointers
                \|/
                next => batch2->pages => about 510 pointers
                          \|/
                          next => batch3->pages => about 510 pointers
                                    ... ...

The proposed parallel free did this: if the process has many pages to be
freed, accumulate them in these struct mmu_gather_batch(es) one after
another till 256K pages are accumulated. Then take this singly linked
list starting from tlb->local.next off struct mmu_gather *tlb and free
them in a worker thread. The main thread can return to continue zap
other pages(after freeing pages pointed by tlb->local.pages).

A test program that did a single malloc() of 320G memory is used to see
how useful the proposed parallel free solution is, the time calculated
is for the free() call. Test machine is a Haswell EX which has
4nodes/72cores/144threads with 512G memory. All tests are done with THP
disabled.

kernel                             time
v4.10                              10.8s  ±2.8%
this patch(with default setting)   5.795s ±5.8%

Patch 3/5 introduced a dedicated workqueue for the free workers and
here are more results when setting different values for max_active of
this workqueue:

max_active:   time
1             8.9s   ±0.5%
2             5.65s  ±5.5%
4             4.84s  ±0.16%
8             4.77s  ±0.97%
16            4.85s  ±0.77%
32            6.21s  ±0.46%

Comments are welcome and appreciated.

v2 changes: Nothing major, only minor ones.
 - rebased on top of v4.11-rc2-mmotm-2017-03-14-15-41;
 - use list_add_tail instead of list_add to add worker to tlb's worker
   list so that when doing flush, the first queued worker gets flushed
   first(based on the comsumption that the first queued worker has a
   better chance of finishing its job than those later queued workers);
 - use bool instead of int for variable free_batch_page in function
   tlb_flush_mmu_free_batches;
 - style change according to ./scripts/checkpatch;
 - reword some of the changelogs to make it more readable.

v1 is here:
https://lkml.org/lkml/2017/2/24/245

Aaron Lu (5):
  mm: add tlb_flush_mmu_free_batches
  mm: parallel free pages
  mm: use a dedicated workqueue for the free workers
  mm: add force_free_pages in zap_pte_range
  mm: add debugfs interface for parallel free tuning

 include/asm-generic/tlb.h |  15 ++---
 mm/memory.c               | 141 +++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 128 insertions(+), 28 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]


#1601159 — [PATCH v2 2/5] mm: parallel free pages

FromAaron Lu <aaron.lu@intel.com>
Date2017-03-15 10:10 +0100
Subject[PATCH v2 2/5] mm: parallel free pages
Message-ID<tld6r-5vn-35@gated-at.bofh.it>
In reply to#1601158
For regular processes, the time taken in its exit() path to free its
used memory is not a problem. But there are heavy ones that consume
several Terabytes memory and the time taken to free its memory could
last more than ten minutes.

To optimize this use case, a parallel free method is proposed and it is
based on the current gather batch free.

The current gather batch free works like this:
For each struct mmu_gather *tlb, there is a static buffer to store those
to-be-freed page pointers. The size is MMU_GATHER_BUNDLE, which is
defined to be 8. So if a tlb tear down doesn't free more than 8 pages,
that is all we need. If 8+ pages are to be freed, new pages will need
to be allocated to store those to-be-freed page pointers.

The structure used to describe the saved page pointers is called
struct mmu_gather_batch and tlb->local is of this type. tlb->local is
different than other struct mmu_gather_batch(es) in that the page
pointer array used by tlb->local points to the previouslly described
static buffer while the other struct mmu_gather_batch(es) page pointer
array points to the dynamically allocated pages.

These batches will form a singly linked list, starting from &tlb->local.

tlb->local.pages  => tlb->pages(8 pointers)
      \|/
      next => batch1->pages => about 510 pointers
                \|/
                next => batch2->pages => about 510 pointers
                          \|/
                          next => batch3->pages => about 510 pointers
                                    ... ...

The proposed parallel free did this: if the process has many pages to be
freed, accumulate them in these struct mmu_gather_batch(es) one after
another till 256K pages are accumulated. Then take this singly linked
list starting from tlb->local.next off struct mmu_gather *tlb and free
them in a worker thread. The main thread can return to continue zap
other pages(after freeing pages pointed by tlb->local.pages).

Note that since we may be accumulating as many as 256K pages now, the
soft lockup on !CONFIG_PREEMPT issue which is fixed by
commit 53a59fc67f97 ("mm: limit mmu_gather batching to fix soft lockups
on !CONFIG_PREEMPT") can reappear. For that matter, add cond_resched()
in tlb_flush_mmu_free_batches where many pages can be freed.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 include/asm-generic/tlb.h | 15 +++++++------
 mm/memory.c               | 57 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 4329bc6ef04b..7c2ac179cc47 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -78,13 +78,10 @@ struct mmu_gather_batch {
 #define MAX_GATHER_BATCH	\
 	((PAGE_SIZE - sizeof(struct mmu_gather_batch)) / sizeof(void *))
 
-/*
- * Limit the maximum number of mmu_gather batches to reduce a risk of soft
- * lockups for non-preemptible kernels on huge machines when a lot of memory
- * is zapped during unmapping.
- * 10K pages freed at once should be safe even without a preemption point.
- */
-#define MAX_GATHER_BATCH_COUNT	(10000UL/MAX_GATHER_BATCH)
+#define ASYNC_FREE_THRESHOLD (256*1024UL)
+#define MAX_GATHER_BATCH_COUNT	\
+	DIV_ROUND_UP(ASYNC_FREE_THRESHOLD, MAX_GATHER_BATCH)
+#define PAGE_FREE_NR_TO_YIELD (10000UL)
 
 /* struct mmu_gather is an opaque type used by the mm code for passing around
  * any data needed by arch specific code for tlb_remove_page.
@@ -108,6 +105,10 @@ struct mmu_gather {
 	struct page		*__pages[MMU_GATHER_BUNDLE];
 	unsigned int		batch_count;
 	int page_size;
+	/* how many pages we have gathered to be freed */
+	unsigned int            page_nr;
+	/* list for spawned workers that do the free jobs */
+	struct list_head        worker_list;
 };
 
 #define HAVE_GENERIC_MMU_GATHER
diff --git a/mm/memory.c b/mm/memory.c
index cdb2a53f251f..001c7720d773 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -228,6 +228,9 @@ void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long
 	tlb->local.max  = ARRAY_SIZE(tlb->__pages);
 	tlb->active     = &tlb->local;
 	tlb->batch_count = 0;
+	tlb->page_nr    = 0;
+
+	INIT_LIST_HEAD(&tlb->worker_list);
 
 #ifdef CONFIG_HAVE_RCU_TABLE_FREE
 	tlb->batch = NULL;
@@ -254,22 +257,65 @@ static void tlb_flush_mmu_free_batches(struct mmu_gather_batch *batch_start,
 				       bool free_batch_page)
 {
 	struct mmu_gather_batch *batch, *next;
+	int nr = 0;
 
 	for (batch = batch_start; batch; batch = next) {
 		next = batch->next;
 		if (batch->nr) {
 			free_pages_and_swap_cache(batch->pages, batch->nr);
+			nr += batch->nr;
 			batch->nr = 0;
 		}
-		if (free_batch_page)
+		if (free_batch_page) {
 			free_pages((unsigned long)batch, 0);
+			nr++;
+		}
+		if (nr >= PAGE_FREE_NR_TO_YIELD) {
+			cond_resched();
+			nr = 0;
+		}
 	}
 }
 
+struct batch_free_struct {
+	struct work_struct work;
+	struct mmu_gather_batch *batch_start;
+	struct list_head list;
+};
+
+static void batch_free_work(struct work_struct *work)
+{
+	struct batch_free_struct *batch_free = container_of(work,
+						struct batch_free_struct, work);
+	tlb_flush_mmu_free_batches(batch_free->batch_start, true);
+}
+
 static void tlb_flush_mmu_free(struct mmu_gather *tlb)
 {
+	struct batch_free_struct *batch_free = NULL;
+
+	if (tlb->page_nr >= ASYNC_FREE_THRESHOLD)
+		batch_free = kmalloc(sizeof(*batch_free),
+				     GFP_NOWAIT | __GFP_NOWARN);
+
+	if (batch_free) {
+		/*
+		 * Start a worker to free pages stored
+		 * in batches following tlb->local.
+		 */
+		batch_free->batch_start = tlb->local.next;
+		INIT_WORK(&batch_free->work, batch_free_work);
+		list_add_tail(&batch_free->list, &tlb->worker_list);
+		queue_work(system_unbound_wq, &batch_free->work);
+
+		tlb->batch_count = 0;
+		tlb->local.next = NULL;
+		/* fall through to free pages stored in tlb->local */
+	}
+
 	tlb_flush_mmu_free_batches(&tlb->local, false);
 	tlb->active = &tlb->local;
+	tlb->page_nr = 0;
 }
 
 void tlb_flush_mmu(struct mmu_gather *tlb)
@@ -284,11 +330,18 @@ void tlb_flush_mmu(struct mmu_gather *tlb)
  */
 void tlb_finish_mmu(struct mmu_gather *tlb, unsigned long start, unsigned long end)
 {
+	struct batch_free_struct *batch_free, *n;
+
 	tlb_flush_mmu(tlb);
 
 	/* keep the page table cache within bounds */
 	check_pgt_cache();
 
+	list_for_each_entry_safe(batch_free, n, &tlb->worker_list, list) {
+		flush_work(&batch_free->work);
+		kfree(batch_free);
+	}
+
 	tlb_flush_mmu_free_batches(tlb->local.next, true);
 	tlb->local.next = NULL;
 }
@@ -307,6 +360,8 @@ bool __tlb_remove_page_size(struct mmu_gather *tlb, struct page *page, int page_
 	VM_BUG_ON(!tlb->end);
 	VM_WARN_ON(tlb->page_size != page_size);
 
+	tlb->page_nr++;
+
 	batch = tlb->active;
 	/*
 	 * Add the page and check if we are full. If so
-- 
2.7.4

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


#1601191 — Re: [PATCH v2 2/5] mm: parallel free pages

From"Hillf Danton" <hillf.zj@alibaba-inc.com>
Date2017-03-15 10:50 +0100
SubjectRe: [PATCH v2 2/5] mm: parallel free pages
Message-ID<tldJ9-5Lz-35@gated-at.bofh.it>
In reply to#1601159
On March 15, 2017 5:00 PM Aaron Lu wrote: 
>  void tlb_finish_mmu(struct mmu_gather *tlb, unsigned long start, unsigned long end)
>  {
> +	struct batch_free_struct *batch_free, *n;
> +
s/*n/*next/

>  	tlb_flush_mmu(tlb);
> 
>  	/* keep the page table cache within bounds */
>  	check_pgt_cache();
> 
> +	list_for_each_entry_safe(batch_free, n, &tlb->worker_list, list) {
> +		flush_work(&batch_free->work);

Not sure, list_del before free?

> +		kfree(batch_free);
> +	}
> +
>  	tlb_flush_mmu_free_batches(tlb->local.next, true);
>  	tlb->local.next = NULL;
>  }

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


#1601345 — Re: [PATCH v2 2/5] mm: parallel free pages

FromAaron Lu <aaron.lu@intel.com>
Date2017-03-15 13:00 +0100
SubjectRe: [PATCH v2 2/5] mm: parallel free pages
Message-ID<tlfKW-76X-17@gated-at.bofh.it>
In reply to#1601191
On Wed, Mar 15, 2017 at 05:42:42PM +0800, Hillf Danton wrote:
> 
> On March 15, 2017 5:00 PM Aaron Lu wrote: 
> >  void tlb_finish_mmu(struct mmu_gather *tlb, unsigned long start, unsigned long end)
> >  {
> > +	struct batch_free_struct *batch_free, *n;
> > +
> s/*n/*next/
> 
> >  	tlb_flush_mmu(tlb);
> > 
> >  	/* keep the page table cache within bounds */
> >  	check_pgt_cache();
> > 
> > +	list_for_each_entry_safe(batch_free, n, &tlb->worker_list, list) {
> > +		flush_work(&batch_free->work);
> 
> Not sure, list_del before free?

I think this is a good idea, it makes code look saner.
I just did a search of list_for_each_entry_safe and found list_del is
usually(I didn't check every one of them) used before free.

So I'll add that in the next revision, probably some days later in case
there are other comments.

Thanks for your time to review the patch.

Regards,
Aaron
 
> > +		kfree(batch_free);
> > +	}
> > +
> >  	tlb_flush_mmu_free_batches(tlb->local.next, true);
> >  	tlb->local.next = NULL;
> >  }
> 
> --
> 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>

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


#1601443

FromMichal Hocko <mhocko@kernel.org>
Date2017-03-15 15:30 +0100
Message-ID<tli65-po-1@gated-at.bofh.it>
In reply to#1601158
On Wed 15-03-17 16:59:59, Aaron Lu wrote:
[...]
> The proposed parallel free did this: if the process has many pages to be
> freed, accumulate them in these struct mmu_gather_batch(es) one after
> another till 256K pages are accumulated. Then take this singly linked
> list starting from tlb->local.next off struct mmu_gather *tlb and free
> them in a worker thread. The main thread can return to continue zap
> other pages(after freeing pages pointed by tlb->local.pages).

I didn't have a look at the implementation yet but there are two
concerns that raise up from this description. Firstly how are we going
to tune the number of workers. I assume there will be some upper bound
(one of the patch subject mentions debugfs for tuning) and secondly
if we offload the page freeing to the worker then the original context
can consume much more cpu cycles than it was configured via cpu
controller. How are we going to handle that? Or is this considered
acceptable?
-- 
Michal Hocko
SUSE Labs

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


#1601504

FromAaron Lu <aaron.lu@intel.com>
Date2017-03-15 16:50 +0100
Message-ID<tljlx-1aZ-39@gated-at.bofh.it>
In reply to#1601443
On Wed, Mar 15, 2017 at 03:18:14PM +0100, Michal Hocko wrote:
> On Wed 15-03-17 16:59:59, Aaron Lu wrote:
> [...]
> > The proposed parallel free did this: if the process has many pages to be
> > freed, accumulate them in these struct mmu_gather_batch(es) one after
> > another till 256K pages are accumulated. Then take this singly linked
> > list starting from tlb->local.next off struct mmu_gather *tlb and free
> > them in a worker thread. The main thread can return to continue zap
> > other pages(after freeing pages pointed by tlb->local.pages).
> 
> I didn't have a look at the implementation yet but there are two
> concerns that raise up from this description. Firstly how are we going
> to tune the number of workers. I assume there will be some upper bound
> (one of the patch subject mentions debugfs for tuning) and secondly

The workers are put in a dedicated workqueue which is introduced in
patch 3/5 and the number of workers can be tuned through that workqueue's
sysfs interface: max_active.

> if we offload the page freeing to the worker then the original context
> can consume much more cpu cycles than it was configured via cpu
> controller. How are we going to handle that? Or is this considered
> acceptable?

I'll need to think about and take a look at this subject(not familiar
with cpu controller).

Thanks.

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


#1601530

FromMichal Hocko <mhocko@kernel.org>
Date2017-03-15 17:30 +0100
Message-ID<tljYd-1Jv-25@gated-at.bofh.it>
In reply to#1601504
On Wed 15-03-17 23:44:07, Aaron Lu wrote:
> On Wed, Mar 15, 2017 at 03:18:14PM +0100, Michal Hocko wrote:
> > On Wed 15-03-17 16:59:59, Aaron Lu wrote:
> > [...]
> > > The proposed parallel free did this: if the process has many pages to be
> > > freed, accumulate them in these struct mmu_gather_batch(es) one after
> > > another till 256K pages are accumulated. Then take this singly linked
> > > list starting from tlb->local.next off struct mmu_gather *tlb and free
> > > them in a worker thread. The main thread can return to continue zap
> > > other pages(after freeing pages pointed by tlb->local.pages).
> > 
> > I didn't have a look at the implementation yet but there are two
> > concerns that raise up from this description. Firstly how are we going
> > to tune the number of workers. I assume there will be some upper bound
> > (one of the patch subject mentions debugfs for tuning) and secondly
> 
> The workers are put in a dedicated workqueue which is introduced in
> patch 3/5 and the number of workers can be tuned through that workqueue's
> sysfs interface: max_active.

I suspect we cannot expect users to tune this. What do you consider a
reasonable default?

Moreover, and this is a more generic question, is this functionality
useful in general purpose workloads? After all the amount of the work to
be done is the same we just risk more lock contentions, unexpected CPU
usage etc. Which workloads will benefit from having exit path faster?
 
> > if we offload the page freeing to the worker then the original context
> > can consume much more cpu cycles than it was configured via cpu

I was not precise here. I meant to say more cpu cycles per time unit
that it was allowed.

> > controller. How are we going to handle that? Or is this considered
> > acceptable?
> 
> I'll need to think about and take a look at this subject(not familiar
> with cpu controller).

the main problem is that kworkers will not belong to the same cpu group
and so they will not be throttled properly.
-- 
Michal Hocko
SUSE Labs

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


#1601756

FromTim Chen <tim.c.chen@linux.intel.com>
Date2017-03-15 22:40 +0100
Message-ID<tloOe-55O-13@gated-at.bofh.it>
In reply to#1601530
On Wed, 2017-03-15 at 17:28 +0100, Michal Hocko wrote:
> On Wed 15-03-17 23:44:07, Aaron Lu wrote:
> > 
> > On Wed, Mar 15, 2017 at 03:18:14PM +0100, Michal Hocko wrote:
> > > 
> > > On Wed 15-03-17 16:59:59, Aaron Lu wrote:
> > > [...]
> > > > 
> > > > The proposed parallel free did this: if the process has many pages to be
> > > > freed, accumulate them in these struct mmu_gather_batch(es) one after
> > > > another till 256K pages are accumulated. Then take this singly linked
> > > > list starting from tlb->local.next off struct mmu_gather *tlb and free
> > > > them in a worker thread. The main thread can return to continue zap
> > > > other pages(after freeing pages pointed by tlb->local.pages).
> > > I didn't have a look at the implementation yet but there are two
> > > concerns that raise up from this description. Firstly how are we going
> > > to tune the number of workers. I assume there will be some upper bound
> > > (one of the patch subject mentions debugfs for tuning) and secondly
> > The workers are put in a dedicated workqueue which is introduced in
> > patch 3/5 and the number of workers can be tuned through that workqueue's
> > sysfs interface: max_active.
> I suspect we cannot expect users to tune this. What do you consider a
> reasonable default?

From Aaron's data, it seems like 4 is a reasonable value for max_active:

max_active:   time
1             8.9s   ±0.5%
2             5.65s  ±5.5%
4             4.84s  ±0.16%
8             4.77s  ±0.97%
16            4.85s  ±0.77%
32            6.21s  ±0.46%


> Moreover, and this is a more generic question, is this functionality
> useful in general purpose workloads? 

If we are running consecutive batch jobs, this optimization
should help start the next job sooner.

> After all the amount of the work to
> be done is the same we just risk more lock contentions, unexpected CPU
> usage etc. Which workloads will benefit from having exit path faster?
>  
> > 
> > > 
> > > if we offload the page freeing to the worker then the original context
> > > can consume much more cpu cycles than it was configured via cpu
> I was not precise here. I meant to say more cpu cycles per time unit
> that it was allowed.
> 
> > 
> > > 
> > > controller. How are we going to handle that? Or is this considered
> > > acceptable?
> > I'll need to think about and take a look at this subject(not familiar
> > with cpu controller).
> the main problem is that kworkers will not belong to the same cpu group
> and so they will not be throttled properly.

You do have a point that this page freeing activities should strive to
affect other threads not in the same cgroup minimally.

On the other hand, we also don't do this throttling of kworkers 
today (e.g. pdflush) according to the cgroup it is doing work for.


Thanks.

Tim

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


#1602112

FromMichal Hocko <mhocko@kernel.org>
Date2017-03-16 10:10 +0100
Message-ID<tlzzY-4tY-23@gated-at.bofh.it>
In reply to#1601756
On Wed 15-03-17 14:38:34, Tim Chen wrote:
> On Wed, 2017-03-15 at 17:28 +0100, Michal Hocko wrote:
> > On Wed 15-03-17 23:44:07, Aaron Lu wrote:
> > > 
> > > On Wed, Mar 15, 2017 at 03:18:14PM +0100, Michal Hocko wrote:
> > > > 
> > > > On Wed 15-03-17 16:59:59, Aaron Lu wrote:
> > > > [...]
> > > > > 
> > > > > The proposed parallel free did this: if the process has many pages to be
> > > > > freed, accumulate them in these struct mmu_gather_batch(es) one after
> > > > > another till 256K pages are accumulated. Then take this singly linked
> > > > > list starting from tlb->local.next off struct mmu_gather *tlb and free
> > > > > them in a worker thread. The main thread can return to continue zap
> > > > > other pages(after freeing pages pointed by tlb->local.pages).
> > > > I didn't have a look at the implementation yet but there are two
> > > > concerns that raise up from this description. Firstly how are we going
> > > > to tune the number of workers. I assume there will be some upper bound
> > > > (one of the patch subject mentions debugfs for tuning) and secondly
> > > The workers are put in a dedicated workqueue which is introduced in
> > > patch 3/5 and the number of workers can be tuned through that workqueue's
> > > sysfs interface: max_active.
> > I suspect we cannot expect users to tune this. What do you consider a
> > reasonable default?
> 
> From Aaron's data, it seems like 4 is a reasonable value for max_active:
> 
> max_active:   time
> 1             8.9s   ±0.5%
> 2             5.65s  ±5.5%
> 4             4.84s  ±0.16%
> 8             4.77s  ±0.97%
> 16            4.85s  ±0.77%
> 32            6.21s  ±0.46%

OK, but this will depend on the HW, right? Also now that I am looking at
those numbers more closely. This was about unmapping 320GB area and
using 4 times more CPUs you managed to half the run time. Is this really
worth it? Sure if those CPUs were idle then this is a clear win but if
the system is moderately busy then it doesn't look like a clear win to
me.

> > Moreover, and this is a more generic question, is this functionality
> > useful in general purpose workloads? 
> 
> If we are running consecutive batch jobs, this optimization
> should help start the next job sooner.

Is this sufficient justification to add a potentially hard to tune
optimization that can influence other workloads on the machine?

> > After all the amount of the work to
> > be done is the same we just risk more lock contentions, unexpected CPU
> > usage etc. Which workloads will benefit from having exit path faster?
> >  
> > > 
> > > > 
> > > > if we offload the page freeing to the worker then the original context
> > > > can consume much more cpu cycles than it was configured via cpu
> > I was not precise here. I meant to say more cpu cycles per time unit
> > that it was allowed.
> > 
> > > 
> > > > 
> > > > controller. How are we going to handle that? Or is this considered
> > > > acceptable?
> > > I'll need to think about and take a look at this subject(not familiar
> > > with cpu controller).
> > the main problem is that kworkers will not belong to the same cpu group
> > and so they will not be throttled properly.
> 
> You do have a point that this page freeing activities should strive to
> affect other threads not in the same cgroup minimally.
> 
> On the other hand, we also don't do this throttling of kworkers 
> today (e.g. pdflush) according to the cgroup it is doing work for.

Yes, I am not saying this a new problem. I just wanted to point out that
this is something to consider here. I believe this should be fixable.
Worker can attach to the same cgroup the initiator had for example
(assuming the cgroup core allows that which is something would have to
be checked).
-- 
Michal Hocko
SUSE Labs

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


#1602757

FromTim Chen <tim.c.chen@linux.intel.com>
Date2017-03-16 19:40 +0100
Message-ID<tlItA-2iT-7@gated-at.bofh.it>
In reply to#1602112
On Thu, 2017-03-16 at 10:07 +0100, Michal Hocko wrote:
> On Wed 15-03-17 14:38:34, Tim Chen wrote:
> > 
> > On Wed, 2017-03-15 at 17:28 +0100, Michal Hocko wrote:
> > > 
> > > On Wed 15-03-17 23:44:07, Aaron Lu wrote:
> > > > 
> > > > 
> > > > On Wed, Mar 15, 2017 at 03:18:14PM +0100, Michal Hocko wrote:
> > > > > 
> > > > > 
> > > > > On Wed 15-03-17 16:59:59, Aaron Lu wrote:
> > > > > [...]
> > > > > > 
> > > > > > 
> > > > > > The proposed parallel free did this: if the process has many pages to be
> > > > > > freed, accumulate them in these struct mmu_gather_batch(es) one after
> > > > > > another till 256K pages are accumulated. Then take this singly linked
> > > > > > list starting from tlb->local.next off struct mmu_gather *tlb and free
> > > > > > them in a worker thread. The main thread can return to continue zap
> > > > > > other pages(after freeing pages pointed by tlb->local.pages).
> > > > > I didn't have a look at the implementation yet but there are two
> > > > > concerns that raise up from this description. Firstly how are we going
> > > > > to tune the number of workers. I assume there will be some upper bound
> > > > > (one of the patch subject mentions debugfs for tuning) and secondly
> > > > The workers are put in a dedicated workqueue which is introduced in
> > > > patch 3/5 and the number of workers can be tuned through that workqueue's
> > > > sysfs interface: max_active.
> > > I suspect we cannot expect users to tune this. What do you consider a
> > > reasonable default?
> > From Aaron's data, it seems like 4 is a reasonable value for max_active:
> > 
> > max_active:   time
> > 1             8.9s   ±0.5%
> > 2             5.65s  ±5.5%
> > 4             4.84s  ±0.16%
> > 8             4.77s  ±0.97%
> > 16            4.85s  ±0.77%
> > 32            6.21s  ±0.46%
> OK, but this will depend on the HW, right? Also now that I am looking at
> those numbers more closely. This was about unmapping 320GB area and
> using 4 times more CPUs you managed to half the run time. Is this really
> worth it? Sure if those CPUs were idle then this is a clear win but if
> the system is moderately busy then it doesn't look like a clear win to
> me.

It looks like we can reduce the exit time in half by using only 2 workers
to disturb the system minimally.
Perhaps we can only do this expedited exit only when there are idle cpus around.
We can use the root sched domain's overload indicator for such a quick check.

Tim

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


#1603098

FromMinchan Kim <minchan@kernel.org>
Date2017-03-17 09:10 +0100
Message-ID<tlV7r-3m4-1@gated-at.bofh.it>
In reply to#1602757
On Fri, Mar 17, 2017 at 08:47:08AM +0100, Michal Hocko wrote:
> On Thu 16-03-17 11:36:21, Tim Chen wrote:
> [...]
> > Perhaps we can only do this expedited exit only when there are idle cpus around.
> > We can use the root sched domain's overload indicator for such a quick check.
> 
> This is not so easy, I am afraid. Those CPUs might be idle for a good
> reason (power saving etc.). You will never know by simply checking
> one metric. This is why doing these optimistic parallelization
> optimizations is far from trivial. This is not the first time somebody
> wants to do this.  People are trying to make THP migration faster
> doing the similar thing. I guess we really need a help from the
> scheduler to do this properly, though. I've been thinking about an API
> (e.g. try_to_run_in_backgroun) which would evaluate all these nasty
> details and either return with -EBUSY or kick the background thread to
> accomplish the work if the system is reasonably idle. I am not really

I agree with Michal's opinion.

In fact, I had prototyped zram parallel write(i.e., if there are many
CPU in the system, zram can compress a bio's pages in parallel via
multiple CPUs) and it seems to work well but my concern was out of
control about power, cpu load, wakeup latency and so on.

> sure whether such an API is viable though.  Peter, what do you think?

If scheduler can support such API(ie, return true and queue the job
if new job is scheduled into other CPU right now because there is
idle CPU in the system), it would be really great for things which
want to use multiple CPU power in parallel to complete the job asap.
Of course, it could sacrifice power but it's trade-off, IMHO.

Thanks.

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


#1603108

FromMichal Hocko <mhocko@kernel.org>
Date2017-03-17 09:30 +0100
Message-ID<tlV7r-3m4-3@gated-at.bofh.it>
In reply to#1602757
On Thu 16-03-17 11:36:21, Tim Chen wrote:
[...]
> Perhaps we can only do this expedited exit only when there are idle cpus around.
> We can use the root sched domain's overload indicator for such a quick check.

This is not so easy, I am afraid. Those CPUs might be idle for a good
reason (power saving etc.). You will never know by simply checking
one metric. This is why doing these optimistic parallelization
optimizations is far from trivial. This is not the first time somebody
wants to do this.  People are trying to make THP migration faster
doing the similar thing. I guess we really need a help from the
scheduler to do this properly, though. I've been thinking about an API
(e.g. try_to_run_in_backgroun) which would evaluate all these nasty
details and either return with -EBUSY or kick the background thread to
accomplish the work if the system is reasonably idle. I am not really
sure whether such an API is viable though.  Peter, what do you think?
-- 
Michal Hocko
SUSE Labs

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


#1603273

FromAaron Lu <aaron.lu@intel.com>
Date2017-03-17 13:40 +0100
Message-ID<tlZkJ-6pR-1@gated-at.bofh.it>
In reply to#1603108
On Fri, Mar 17, 2017 at 08:47:08AM +0100, Michal Hocko wrote:
> On Thu 16-03-17 11:36:21, Tim Chen wrote:
> [...]
> > Perhaps we can only do this expedited exit only when there are idle cpus around.
> > We can use the root sched domain's overload indicator for such a quick check.
> 
> This is not so easy, I am afraid. Those CPUs might be idle for a good
> reason (power saving etc.). You will never know by simply checking

Is it that those CPUs are deliberately put into idle mode to save power?
IIRC, idle injection driver could be used to do this and if so, the
injected idle task is a realtime one so the spawned kworker will not be
able to preempt(disturb) it.

> one metric. This is why doing these optimistic parallelization
> optimizations is far from trivial. This is not the first time somebody
> wants to do this.  People are trying to make THP migration faster
> doing the similar thing. I guess we really need a help from the
> scheduler to do this properly, though. I've been thinking about an API
> (e.g. try_to_run_in_backgroun) which would evaluate all these nasty
> details and either return with -EBUSY or kick the background thread to
> accomplish the work if the system is reasonably idle. I am not really
> sure whether such an API is viable though.  Peter, what do you think?

I would very much like to know what these nasty details are and what
'reasonably idle' actually means, I think they are useful to understand
the problem and define the API.

I totally agree that we shouldn't distrub the system by starting more
workers/threads to do spin work or to make a process utilizing more CPU
or other resources than allowed by its cgroup.

Thanks.

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


#1603291

FromMichal Hocko <mhocko@kernel.org>
Date2017-03-17 14:00 +0100
Message-ID<tlZE6-6AQ-15@gated-at.bofh.it>
In reply to#1603273
On Fri 17-03-17 20:33:15, Aaron Lu wrote:
> On Fri, Mar 17, 2017 at 08:47:08AM +0100, Michal Hocko wrote:
> > On Thu 16-03-17 11:36:21, Tim Chen wrote:
> > [...]
> > > Perhaps we can only do this expedited exit only when there are idle cpus around.
> > > We can use the root sched domain's overload indicator for such a quick check.
> > 
> > This is not so easy, I am afraid. Those CPUs might be idle for a good
> > reason (power saving etc.). You will never know by simply checking
> 
> Is it that those CPUs are deliberately put into idle mode to save power?

I am not a scheduler expert. All I know is that there is strong pressure
to make the schedule power aware and so some cpus are kept idle while
the workload is spread over other (currently active) cpus. And all I am
trying to tell is that this will be hard to guess without any assistance
from the scheduler. Especially when this should be long term
maintainable.

> IIRC, idle injection driver could be used to do this and if so, the
> injected idle task is a realtime one so the spawned kworker will not be
> able to preempt(disturb) it.
> 
> > one metric. This is why doing these optimistic parallelization
> > optimizations is far from trivial. This is not the first time somebody
> > wants to do this.  People are trying to make THP migration faster
> > doing the similar thing. I guess we really need a help from the
> > scheduler to do this properly, though. I've been thinking about an API
> > (e.g. try_to_run_in_backgroun) which would evaluate all these nasty
> > details and either return with -EBUSY or kick the background thread to
> > accomplish the work if the system is reasonably idle. I am not really
> > sure whether such an API is viable though.  Peter, what do you think?
> 
> I would very much like to know what these nasty details are and what
> 'reasonably idle' actually means, I think they are useful to understand
> the problem and define the API.

I would love to give you more specific information but I am not sure
myself. All I know is that the scheduler is the only place where we
have at least some idea about the recent load characteristics and some
policies on top. And that is why I _think_ we need to have an api and
which cooperates with the scheduler.
-- 
Michal Hocko
SUSE Labs

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


#1603298

FromPeter Zijlstra <peterz@infradead.org>
Date2017-03-17 14:20 +0100
Message-ID<tlZXr-6X4-5@gated-at.bofh.it>
In reply to#1603273
On Fri, Mar 17, 2017 at 08:33:15PM +0800, Aaron Lu wrote:
> On Fri, Mar 17, 2017 at 08:47:08AM +0100, Michal Hocko wrote:
> > On Thu 16-03-17 11:36:21, Tim Chen wrote:
> > [...]
> > > Perhaps we can only do this expedited exit only when there are idle cpus around.
> > > We can use the root sched domain's overload indicator for such a quick check.
> > 
> > This is not so easy, I am afraid. Those CPUs might be idle for a good
> > reason (power saving etc.). You will never know by simply checking
> 
> Is it that those CPUs are deliberately put into idle mode to save power?

No, forced idle injection is an abomination.

> > one metric. This is why doing these optimistic parallelization
> > optimizations is far from trivial. This is not the first time somebody
> > wants to do this.  People are trying to make THP migration faster
> > doing the similar thing. I guess we really need a help from the
> > scheduler to do this properly, though. I've been thinking about an API
> > (e.g. try_to_run_in_backgroun) which would evaluate all these nasty
> > details and either return with -EBUSY or kick the background thread to
> > accomplish the work if the system is reasonably idle. I am not really
> > sure whether such an API is viable though.  Peter, what do you think?
> 
> I would very much like to know what these nasty details are and what
> 'reasonably idle' actually means, I think they are useful to understand
> the problem and define the API.

A CPU being idle doesn't mean it'll be idle long enough to do your
additional work.

The CPU not being idle affects scheduling latency. It also increases
power usage and thermals.

If your workload wants peak single threaded throughput, making the other
CPUs do work will lower its turbo boost range for example.

An 'obvious' solution that doesn't work is an idle scheduler; its an
instant priority inversion if you take locks there. Not to mention you
loose any fwd progress guarantees for any work you put in.

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


#1603288

FromPeter Zijlstra <peterz@infradead.org>
Date2017-03-17 14:00 +0100
Message-ID<tlZE6-6AQ-21@gated-at.bofh.it>
In reply to#1603108
On Fri, Mar 17, 2017 at 08:47:08AM +0100, Michal Hocko wrote:
> On Thu 16-03-17 11:36:21, Tim Chen wrote:
> [...]
> > Perhaps we can only do this expedited exit only when there are idle cpus around.
> > We can use the root sched domain's overload indicator for such a quick check.
> 
> This is not so easy, I am afraid. Those CPUs might be idle for a good
> reason (power saving etc.). You will never know by simply checking
> one metric. This is why doing these optimistic parallelization
> optimizations is far from trivial. This is not the first time somebody
> wants to do this.  People are trying to make THP migration faster
> doing the similar thing. I guess we really need a help from the
> scheduler to do this properly, though. I've been thinking about an API
> (e.g. try_to_run_in_backgroun) which would evaluate all these nasty
> details and either return with -EBUSY or kick the background thread to
> accomplish the work if the system is reasonably idle. I am not really
> sure whether such an API is viable though. 

> Peter, what do you think?

Much pain lies this way.

Also, -enocontext.

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


#1603305

FromMichal Hocko <mhocko@kernel.org>
Date2017-03-17 14:30 +0100
Message-ID<tm077-72V-5@gated-at.bofh.it>
In reply to#1603288
On Fri 17-03-17 13:53:33, Peter Zijlstra wrote:
> On Fri, Mar 17, 2017 at 08:47:08AM +0100, Michal Hocko wrote:
> > On Thu 16-03-17 11:36:21, Tim Chen wrote:
> > [...]
> > > Perhaps we can only do this expedited exit only when there are idle cpus around.
> > > We can use the root sched domain's overload indicator for such a quick check.
> > 
> > This is not so easy, I am afraid. Those CPUs might be idle for a good
> > reason (power saving etc.). You will never know by simply checking
> > one metric. This is why doing these optimistic parallelization
> > optimizations is far from trivial. This is not the first time somebody
> > wants to do this.  People are trying to make THP migration faster
> > doing the similar thing. I guess we really need a help from the
> > scheduler to do this properly, though. I've been thinking about an API
> > (e.g. try_to_run_in_backgroun) which would evaluate all these nasty
> > details and either return with -EBUSY or kick the background thread to
> > accomplish the work if the system is reasonably idle. I am not really
> > sure whether such an API is viable though. 
> 
> > Peter, what do you think?
> 
> Much pain lies this way.

I somehow exptected this answer ;)
 
> Also, -enocontext.

Well, the context is that there are more users emerging which would like
to move some part of the heavy operation (e.g. munmap in exit or THP
migration) to the background thread because that operation can be split
and parallelized. kworker API is used for this purpose currently and I
believe that this is not the right approach because optimization for one
workload might be too disruptive on anybody else. On the other side
larger machines which would benefit from these optimizations are more
likely to have idle CPUs to (ab)use. So the idea was to provide an API
which would tell whether kicking a background worker(s) to accomplish
the task is feasible. The scheduler sounds like the best candidate to
ask this question to me. I might be wrong here of course but a
centralized API sounds like a better approach than ad-hoc solutions
developed for each particular usecase.  

-- 
Michal Hocko
SUSE Labs

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


#1605700

FromDave Hansen <dave.hansen@intel.com>
Date2017-03-21 16:10 +0100
Message-ID<tntA7-4ZS-53@gated-at.bofh.it>
In reply to#1602112
On 03/16/2017 02:07 AM, Michal Hocko wrote:
> On Wed 15-03-17 14:38:34, Tim Chen wrote:
>> max_active:   time
>> 1             8.9s   ±0.5%
>> 2             5.65s  ±5.5%
>> 4             4.84s  ±0.16%
>> 8             4.77s  ±0.97%
>> 16            4.85s  ±0.77%
>> 32            6.21s  ±0.46%
> 
> OK, but this will depend on the HW, right? Also now that I am looking at
> those numbers more closely. This was about unmapping 320GB area and
> using 4 times more CPUs you managed to half the run time. Is this really
> worth it? Sure if those CPUs were idle then this is a clear win but if
> the system is moderately busy then it doesn't look like a clear win to
> me.

This still suffers from zone lock contention.  It scales much better if
we are freeing memory from more than one zone.  We would expect any
other generic page allocator scalability improvements to really help
here, too.

Aaron, could you make sure to make sure that the memory being freed is
coming from multiple NUMA nodes?  It might also be interesting to boot
with a fake NUMA configuration with a *bunch* of nodes to see what the
best case looks like when zone lock contention isn't even in play where
one worker would be working on its own zone.

>>> Moreover, and this is a more generic question, is this functionality
>>> useful in general purpose workloads? 
>>
>> If we are running consecutive batch jobs, this optimization
>> should help start the next job sooner.
> 
> Is this sufficient justification to add a potentially hard to tune
> optimization that can influence other workloads on the machine?

The guys for whom a reboot is faster than a single exit() certainly
think so. :)

I have the feeling that we can find a pretty sane large process size to
be the floor where this feature gets activated.  I doubt the systems
that really care about noise from other workloads are often doing
multi-gigabyte mapping teardowns.

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


#1606271

FromAaron Lu <aaron.lu@intel.com>
Date2017-03-22 09:20 +0100
Message-ID<tnJES-7PW-13@gated-at.bofh.it>
In reply to#1605700
On Tue, Mar 21, 2017 at 07:54:37AM -0700, Dave Hansen wrote:
> On 03/16/2017 02:07 AM, Michal Hocko wrote:
> > On Wed 15-03-17 14:38:34, Tim Chen wrote:
> >> max_active:   time
> >> 1             8.9s   ±0.5%
> >> 2             5.65s  ±5.5%
> >> 4             4.84s  ±0.16%
> >> 8             4.77s  ±0.97%
> >> 16            4.85s  ±0.77%
> >> 32            6.21s  ±0.46%
> > 
> > OK, but this will depend on the HW, right? Also now that I am looking at
> > those numbers more closely. This was about unmapping 320GB area and
> > using 4 times more CPUs you managed to half the run time. Is this really
> > worth it? Sure if those CPUs were idle then this is a clear win but if
> > the system is moderately busy then it doesn't look like a clear win to
> > me.
> 
> This still suffers from zone lock contention.  It scales much better if
> we are freeing memory from more than one zone.  We would expect any
> other generic page allocator scalability improvements to really help
> here, too.
> 
> Aaron, could you make sure to make sure that the memory being freed is
> coming from multiple NUMA nodes?  It might also be interesting to boot

The test machine has 4 nodes and each has 128G memory.
With the test size of 320G, at least 3 nodes are involved.

But since the test is done on an idle system, I *guess* the allocated
memory is physically continuous. Then when they are freed in virtually
continuous order, it's likely that one after another physically continous
1G chunk are sent to the free kworkers. So roughly for the first
128 1G chunks, those workers will all be contending on the same zone.
(well, it shouldn't be 128 kworkers all runnable contending for the same
lock since early launched kworkers will have exited after finishing its
job before some later launched kworkers start).

> with a fake NUMA configuration with a *bunch* of nodes to see what the
> best case looks like when zone lock contention isn't even in play where
> one worker would be working on its own zone.

Good idea, will post results here once I finished the test.

Thanks.

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


#1608169

FromAaron Lu <aaron.lu@intel.com>
Date2017-03-24 08:10 +0100
Message-ID<torwd-6nm-5@gated-at.bofh.it>
In reply to#1605700

[Multipart message — attachments visible in raw view] — view raw

On Tue, Mar 21, 2017 at 07:54:37AM -0700, Dave Hansen wrote:
> On 03/16/2017 02:07 AM, Michal Hocko wrote:
> > On Wed 15-03-17 14:38:34, Tim Chen wrote:
> >> max_active:   time
> >> 1             8.9s   ±0.5%
> >> 2             5.65s  ±5.5%
> >> 4             4.84s  ±0.16%
> >> 8             4.77s  ±0.97%
> >> 16            4.85s  ±0.77%
> >> 32            6.21s  ±0.46%
> > 
> > OK, but this will depend on the HW, right? Also now that I am looking at
> > those numbers more closely. This was about unmapping 320GB area and
> > using 4 times more CPUs you managed to half the run time. Is this really
> > worth it? Sure if those CPUs were idle then this is a clear win but if
> > the system is moderately busy then it doesn't look like a clear win to
> > me.
> 
> This still suffers from zone lock contention.  It scales much better if
> we are freeing memory from more than one zone.  We would expect any
> other generic page allocator scalability improvements to really help
> here, too.
> 
> Aaron, could you make sure to make sure that the memory being freed is
> coming from multiple NUMA nodes?  It might also be interesting to boot
> with a fake NUMA configuration with a *bunch* of nodes to see what the
> best case looks like when zone lock contention isn't even in play where
> one worker would be working on its own zone.

This fake NUMA configuration thing is great for this purpose, I didn't
know we have this support in kernel.

So I added numa=fake=128 and also wrote a new test program(attached)
that mmap() 321G memory and made sure they are distributed equally in
107 nodes, i.e. 3G on each node. This is achieved by using mbind before
touching the memory on each node.

Then I enlarged the max_gather_batch_count to 1543 so that during zap,
3G memory is sent to a kworker for free instead of the default 1G. In
this way, each kworker should be working on a different node.

With this change, time to free the 321G memory is reduced to:

	3.23s ±13.7%  (about 70% decrease)

Lock contention is 1.81%:

        19.60%  [kernel.kallsyms]  [k] release_pages
        13.30%  [kernel.kallsyms]  [k] unmap_page_range
        13.18%  [kernel.kallsyms]  [k] free_pcppages_bulk
         8.34%  [kernel.kallsyms]  [k] __mod_zone_page_state
         7.75%  [kernel.kallsyms]  [k] page_remove_rmap
         7.37%  [kernel.kallsyms]  [k] free_hot_cold_page
         6.06%  [kernel.kallsyms]  [k] free_pages_and_swap_cache
         3.53%  [kernel.kallsyms]  [k] __list_del_entry_valid
         3.09%  [kernel.kallsyms]  [k] __list_add_valid
         1.81%  [kernel.kallsyms]  [k] native_queued_spin_lock_slowpath
         1.79%  [kernel.kallsyms]  [k] uncharge_list
         1.69%  [kernel.kallsyms]  [k] mem_cgroup_update_lru_size
         1.60%  [kernel.kallsyms]  [k] vm_normal_page
         1.46%  [kernel.kallsyms]  [k] __dec_node_state
         1.41%  [kernel.kallsyms]  [k] __mod_node_page_state
         1.20%  [kernel.kallsyms]  [k] __tlb_remove_page_size
         0.85%  [kernel.kallsyms]  [k] mem_cgroup_page_lruvec

From 'vmstat 1', the runnable process peaked at 6 during munmap():
procs -----------memory---------- ---swap-- -----io---- -system-- ------cpu-----
 r  b   swpd   free   buff  cache   si   so    bi    bo   in   cs us sy id wa st
 0  0      0 189114560      0 761292    0    0     0     0   70  146  0  0 100  0  0
 3  0      0 189099008      0 759932    0    0     0     0 2536  382  0  0 100  0  0
 6  0      0 274378848      0 759972    0    0     0     0 11332  249  0  3 97  0  0
 5  0      0 374426592      0 759972    0    0     0     0 13576  196  0  3 97  0  0
 4  0      0 474990144      0 759972    0    0     0     0 13250  227  0  3 97  0  0
 0  0      0 526039296      0 759972    0    0     0     0 6799  246  0  2 98  0  0
^C

This appears to be the best result from this approach.

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


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web