[PATCH v3 0/2] mm, thp: Fix unnecessarry resource consuming in swapin

[PATCH v3 0/2] mm, thp: Fix unnecessarry resource consuming in swapin

[Ebru Akagunduz]

This patch series fixes unnecessarry resource consuming
in khugepaged swapin and introduces a new function to
calculate value of specific vm event.

Ebru Akagunduz (2):
  mm, vmstat: calculate particular vm event
  mm, thp: avoid unnecessary swapin in khugepaged

 include/linux/vmstat.h |  6 ++++++
 mm/huge_memory.c       | 13 +++++++++++--
 mm/vmstat.c            | 12 ++++++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

-- 
1.9.1

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

[PATCH v3 1/2] mm, vmstat: calculate particular vm event

[Ebru Akagunduz]

Currently, vmstat can calculate specific vm event with all_vm_events()
however it allocates all vm events to stack. This patch introduces
a helper to sum value of a specific vm event over all cpu, without
loading all the events.

Signed-off-by: Ebru Akagunduz <ebru.akagunduz@gmail.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
Changes in v2:
 - this patch newly created in this version
 - create sum event function to
   calculate particular vm event (Kirill A. Shutemov)

Changes in v3:
 - add dummy definition of sum_vm_event
   when CONFIG_VM_EVENTS is not set
   (Kirill A. Shutemov)

 include/linux/vmstat.h |  6 ++++++
 mm/vmstat.c            | 12 ++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index 73fae8c..e5ec287 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -53,6 +53,8 @@ static inline void count_vm_events(enum vm_event_item item, long delta)
 
 extern void all_vm_events(unsigned long *);
 
+extern unsigned long sum_vm_event(enum vm_event_item item);
+
 extern void vm_events_fold_cpu(int cpu);
 
 #else
@@ -73,6 +75,10 @@ static inline void __count_vm_events(enum vm_event_item item, long delta)
 static inline void all_vm_events(unsigned long *ret)
 {
 }
+static inline unsigned long sum_vm_event(enum vm_event_item item)
+{
+	return 0;
+}
 static inline void vm_events_fold_cpu(int cpu)
 {
 }
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 5e43004..b76d664 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -34,6 +34,18 @@
 DEFINE_PER_CPU(struct vm_event_state, vm_event_states) = {{0}};
 EXPORT_PER_CPU_SYMBOL(vm_event_states);
 
+unsigned long sum_vm_event(enum vm_event_item item)
+{
+	int cpu;
+	unsigned long ret = 0;
+
+	get_online_cpus();
+	for_each_online_cpu(cpu)
+		ret += per_cpu(vm_event_states, cpu).event[item];
+	put_online_cpus();
+	return ret;
+}
+
 static void sum_vm_events(unsigned long *ret)
 {
 	int cpu;
-- 
1.9.1

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

Re: [PATCH v3 1/2] mm, vmstat: calculate particular vm event

[Rik van Riel]

[-- Attachment #1: Type: text/plain, Size: 510 bytes --]

On Mon, 2016-03-14 at 23:40 +0200, Ebru Akagunduz wrote:
> Currently, vmstat can calculate specific vm event with
> all_vm_events()
> however it allocates all vm events to stack. This patch introduces
> a helper to sum value of a specific vm event over all cpu, without
> loading all the events.
> 
> Signed-off-by: Ebru Akagunduz <ebru.akagunduz@gmail.com>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Reviewed-by: Rik van Riel <riel@redhat.com>

-- 
All Rights Reversed.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v3 1/2] mm, vmstat: calculate particular vm event

[Vlastimil Babka]

On 03/14/2016 10:40 PM, Ebru Akagunduz wrote:
> Currently, vmstat can calculate specific vm event with all_vm_events()
> however it allocates all vm events to stack. This patch introduces
> a helper to sum value of a specific vm event over all cpu, without
> loading all the events.
>
> Signed-off-by: Ebru Akagunduz <ebru.akagunduz@gmail.com>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Kirill was modest enough to not point this out, but this should IMHO 
have at least:

Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Otherwise:
Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks.

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

Re: [PATCH v3 1/2] mm, vmstat: calculate particular vm event

[Ebru Akagunduz]

On Wed, Mar 16, 2016 at 04:54:13PM +0100, Vlastimil Babka wrote:
> On 03/14/2016 10:40 PM, Ebru Akagunduz wrote:
> >Currently, vmstat can calculate specific vm event with all_vm_events()
> >however it allocates all vm events to stack. This patch introduces
> >a helper to sum value of a specific vm event over all cpu, without
> >loading all the events.
> >
> >Signed-off-by: Ebru Akagunduz <ebru.akagunduz@gmail.com>
> >Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> 
> Kirill was modest enough to not point this out, but this should IMHO
> have at least:
> 
> Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> 
> Otherwise:
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> 
Sure. I'll add Suggested-by in next version.

Thanks.

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

[PATCH v3 2/2] mm, thp: avoid unnecessary swapin in khugepaged

[Ebru Akagunduz]

Currently khugepaged makes swapin readahead to improve
THP collapse rate. This patch checks vm statistics
to avoid workload of swapin, if unnecessary. So that
when system under pressure, khugepaged won't consume
resources to swapin.

The patch was tested with a test program that allocates
800MB of memory, writes to it, and then sleeps. The system
was forced to swap out all. Afterwards, the test program
touches the area by writing, it skips a page in each
20 pages of the area. When waiting to swapin readahead
left part of the test, the memory forced to be busy
doing page reclaim. There was enough free memory during
test, khugepaged did not swapin readahead due to business.

Test results:

                        After swapped out
-------------------------------------------------------------------
              | Anonymous | AnonHugePages | Swap      | Fraction  |
-------------------------------------------------------------------
With patch    | 206608 kB |  204800 kB    | 593392 kB |    %99    |
-------------------------------------------------------------------
Without patch | 351308 kB | 350208 kB     | 448692 kB |    %99    |
-------------------------------------------------------------------

                        After swapped in (waiting 10 minutes)
-------------------------------------------------------------------
              | Anonymous | AnonHugePages | Swap      | Fraction  |
-------------------------------------------------------------------
With patch    | 551992 kB | 368640 kB     | 248008 kB |    %66    |
-------------------------------------------------------------------
Without patch | 586816 kB | 464896 kB     | 213184 kB |    %79    |
-------------------------------------------------------------------

Signed-off-by: Ebru Akagunduz <ebru.akagunduz@gmail.com>
---
Changes in v2:
 - Add reference to specify which patch fixed (Ebru Akagunduz)
 - Fix commit subject line (Ebru Akagunduz)

Changes in v3:
 - Remove default values of allocstall (Kirill A. Shutemov)

 mm/huge_memory.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 86e9666..67a398c 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -102,6 +102,7 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
  */
 static unsigned int khugepaged_max_ptes_none __read_mostly;
 static unsigned int khugepaged_max_ptes_swap __read_mostly;
+static unsigned long int allocstall;
 
 static int khugepaged(void *none);
 static int khugepaged_slab_init(void);
@@ -2438,7 +2439,7 @@ static void collapse_huge_page(struct mm_struct *mm,
 	struct page *new_page;
 	spinlock_t *pmd_ptl, *pte_ptl;
 	int isolated = 0, result = 0;
-	unsigned long hstart, hend;
+	unsigned long hstart, hend, swap, curr_allocstall;
 	struct mem_cgroup *memcg;
 	unsigned long mmun_start;	/* For mmu_notifiers */
 	unsigned long mmun_end;		/* For mmu_notifiers */
@@ -2493,7 +2494,14 @@ static void collapse_huge_page(struct mm_struct *mm,
 		goto out;
 	}
 
-	__collapse_huge_page_swapin(mm, vma, address, pmd);
+	swap = get_mm_counter(mm, MM_SWAPENTS);
+	curr_allocstall = sum_vm_event(ALLOCSTALL);
+	/*
+	 * When system under pressure, don't swapin readahead.
+	 * So that avoid unnecessary resource consuming.
+	 */
+	if (allocstall == curr_allocstall && swap != 0)
+		__collapse_huge_page_swapin(mm, vma, address, pmd);
 
 	anon_vma_lock_write(vma->anon_vma);
 
@@ -2790,6 +2798,7 @@ skip:
 			VM_BUG_ON(khugepaged_scan.address < hstart ||
 				  khugepaged_scan.address + HPAGE_PMD_SIZE >
 				  hend);
+			allocstall = sum_vm_event(ALLOCSTALL);
 			ret = khugepaged_scan_pmd(mm, vma,
 						  khugepaged_scan.address,
 						  hpage);
-- 
1.9.1

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

Re: [PATCH v3 2/2] mm, thp: avoid unnecessary swapin in khugepaged

[Vlastimil Babka]

On 03/14/2016 10:40 PM, Ebru Akagunduz wrote:
> Currently khugepaged makes swapin readahead to improve
> THP collapse rate. This patch checks vm statistics
> to avoid workload of swapin, if unnecessary. So that
> when system under pressure, khugepaged won't consume
> resources to swapin.
>
> The patch was tested with a test program that allocates
> 800MB of memory, writes to it, and then sleeps. The system
> was forced to swap out all. Afterwards, the test program
> touches the area by writing, it skips a page in each
> 20 pages of the area. When waiting to swapin readahead
> left part of the test, the memory forced to be busy
> doing page reclaim. There was enough free memory during
> test, khugepaged did not swapin readahead due to business.
>
> Test results:
>
>                          After swapped out
> -------------------------------------------------------------------
>                | Anonymous | AnonHugePages | Swap      | Fraction  |
> -------------------------------------------------------------------
> With patch    | 206608 kB |  204800 kB    | 593392 kB |    %99    |
> -------------------------------------------------------------------
> Without patch | 351308 kB | 350208 kB     | 448692 kB |    %99    |
> -------------------------------------------------------------------
>
>                          After swapped in (waiting 10 minutes)
> -------------------------------------------------------------------
>                | Anonymous | AnonHugePages | Swap      | Fraction  |
> -------------------------------------------------------------------
> With patch    | 551992 kB | 368640 kB     | 248008 kB |    %66    |
> -------------------------------------------------------------------
> Without patch | 586816 kB | 464896 kB     | 213184 kB |    %79    |
> -------------------------------------------------------------------
>
> Signed-off-by: Ebru Akagunduz <ebru.akagunduz@gmail.com>

Looks like a step in a good direction. Still might be worthwile to also 
wait for the swapin to complete, and actually collapse immediately, no?

> ---
> Changes in v2:
>   - Add reference to specify which patch fixed (Ebru Akagunduz)

The reference is again missing in v3.

>   - Fix commit subject line (Ebru Akagunduz)
>
> Changes in v3:
>   - Remove default values of allocstall (Kirill A. Shutemov)
>
>   mm/huge_memory.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 86e9666..67a398c 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -102,6 +102,7 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
>    */
>   static unsigned int khugepaged_max_ptes_none __read_mostly;
>   static unsigned int khugepaged_max_ptes_swap __read_mostly;
> +static unsigned long int allocstall;

"int" here is unnecessary

>
>   static int khugepaged(void *none);
>   static int khugepaged_slab_init(void);
> @@ -2438,7 +2439,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>   	struct page *new_page;
>   	spinlock_t *pmd_ptl, *pte_ptl;
>   	int isolated = 0, result = 0;
> -	unsigned long hstart, hend;
> +	unsigned long hstart, hend, swap, curr_allocstall;
>   	struct mem_cgroup *memcg;
>   	unsigned long mmun_start;	/* For mmu_notifiers */
>   	unsigned long mmun_end;		/* For mmu_notifiers */
> @@ -2493,7 +2494,14 @@ static void collapse_huge_page(struct mm_struct *mm,
>   		goto out;
>   	}
>
> -	__collapse_huge_page_swapin(mm, vma, address, pmd);
> +	swap = get_mm_counter(mm, MM_SWAPENTS);
> +	curr_allocstall = sum_vm_event(ALLOCSTALL);
> +	/*
> +	 * When system under pressure, don't swapin readahead.
> +	 * So that avoid unnecessary resource consuming.
> +	 */
> +	if (allocstall == curr_allocstall && swap != 0)
> +		__collapse_huge_page_swapin(mm, vma, address, pmd);
>
>   	anon_vma_lock_write(vma->anon_vma);
>
> @@ -2790,6 +2798,7 @@ skip:
>   			VM_BUG_ON(khugepaged_scan.address < hstart ||
>   				  khugepaged_scan.address + HPAGE_PMD_SIZE >
>   				  hend);
> +			allocstall = sum_vm_event(ALLOCSTALL);

Why here? Rik said in v2:

> Khugepaged stores the allocstall value when it goes to sleep,
> and checks it before calling (or not) __collapse_huge_page_swapin.

But that's not true, this is not "when it goes to sleep".
So AFAICS it only observes the allocstalls between starting to scan a 
single pmd, and trying to collapse the pmd. So the window is quite tiny 
especially compared to I/O speeds, and this will IMHO catch only really 
frequent stalls. Placing it really at "when it goes to sleep" sounds better.

>   			ret = khugepaged_scan_pmd(mm, vma,
>   						  khugepaged_scan.address,
>   						  hpage);
>

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

Re: [PATCH v3 2/2] mm, thp: avoid unnecessary swapin in khugepaged

[Ebru Akagunduz]

On Thu, Mar 17, 2016 at 12:07:44PM +0100, Vlastimil Babka wrote:
> On 03/14/2016 10:40 PM, Ebru Akagunduz wrote:
> >Currently khugepaged makes swapin readahead to improve
> >THP collapse rate. This patch checks vm statistics
> >to avoid workload of swapin, if unnecessary. So that
> >when system under pressure, khugepaged won't consume
> >resources to swapin.
> >
> >The patch was tested with a test program that allocates
> >800MB of memory, writes to it, and then sleeps. The system
> >was forced to swap out all. Afterwards, the test program
> >touches the area by writing, it skips a page in each
> >20 pages of the area. When waiting to swapin readahead
> >left part of the test, the memory forced to be busy
> >doing page reclaim. There was enough free memory during
> >test, khugepaged did not swapin readahead due to business.
> >
> >Test results:
> >
> >                         After swapped out
> >-------------------------------------------------------------------
> >               | Anonymous | AnonHugePages | Swap      | Fraction  |
> >-------------------------------------------------------------------
> >With patch    | 206608 kB |  204800 kB    | 593392 kB |    %99    |
> >-------------------------------------------------------------------
> >Without patch | 351308 kB | 350208 kB     | 448692 kB |    %99    |
> >-------------------------------------------------------------------
> >
> >                         After swapped in (waiting 10 minutes)
> >-------------------------------------------------------------------
> >               | Anonymous | AnonHugePages | Swap      | Fraction  |
> >-------------------------------------------------------------------
> >With patch    | 551992 kB | 368640 kB     | 248008 kB |    %66    |
> >-------------------------------------------------------------------
> >Without patch | 586816 kB | 464896 kB     | 213184 kB |    %79    |
> >-------------------------------------------------------------------
> >
> >Signed-off-by: Ebru Akagunduz <ebru.akagunduz@gmail.com>
> 
> Looks like a step in a good direction. Still might be worthwile to
> also wait for the swapin to complete, and actually collapse
> immediately, no?
> 
I'll send a patch to solve mmap_sem issues after getting this
patch series accepted.
> >---
> >Changes in v2:
> >  - Add reference to specify which patch fixed (Ebru Akagunduz)
> 
> The reference is again missing in v3.
> 
> >  - Fix commit subject line (Ebru Akagunduz)
> >
> >Changes in v3:
> >  - Remove default values of allocstall (Kirill A. Shutemov)
> >
> >  mm/huge_memory.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> >diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >index 86e9666..67a398c 100644
> >--- a/mm/huge_memory.c
> >+++ b/mm/huge_memory.c
> >@@ -102,6 +102,7 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
> >   */
> >  static unsigned int khugepaged_max_ptes_none __read_mostly;
> >  static unsigned int khugepaged_max_ptes_swap __read_mostly;
> >+static unsigned long int allocstall;
> 
> "int" here is unnecessary
> 
> >
> >  static int khugepaged(void *none);
> >  static int khugepaged_slab_init(void);
> >@@ -2438,7 +2439,7 @@ static void collapse_huge_page(struct mm_struct *mm,
> >  	struct page *new_page;
> >  	spinlock_t *pmd_ptl, *pte_ptl;
> >  	int isolated = 0, result = 0;
> >-	unsigned long hstart, hend;
> >+	unsigned long hstart, hend, swap, curr_allocstall;
> >  	struct mem_cgroup *memcg;
> >  	unsigned long mmun_start;	/* For mmu_notifiers */
> >  	unsigned long mmun_end;		/* For mmu_notifiers */
> >@@ -2493,7 +2494,14 @@ static void collapse_huge_page(struct mm_struct *mm,
> >  		goto out;
> >  	}
> >
> >-	__collapse_huge_page_swapin(mm, vma, address, pmd);
> >+	swap = get_mm_counter(mm, MM_SWAPENTS);
> >+	curr_allocstall = sum_vm_event(ALLOCSTALL);
> >+	/*
> >+	 * When system under pressure, don't swapin readahead.
> >+	 * So that avoid unnecessary resource consuming.
> >+	 */
> >+	if (allocstall == curr_allocstall && swap != 0)
> >+		__collapse_huge_page_swapin(mm, vma, address, pmd);
> >
> >  	anon_vma_lock_write(vma->anon_vma);
> >
> >@@ -2790,6 +2798,7 @@ skip:
> >  			VM_BUG_ON(khugepaged_scan.address < hstart ||
> >  				  khugepaged_scan.address + HPAGE_PMD_SIZE >
> >  				  hend);
> >+			allocstall = sum_vm_event(ALLOCSTALL);
> 
> Why here? Rik said in v2:
> 
> >Khugepaged stores the allocstall value when it goes to sleep,
> >and checks it before calling (or not) __collapse_huge_page_swapin.
> 
> But that's not true, this is not "when it goes to sleep".
> So AFAICS it only observes the allocstalls between starting to scan
> a single pmd, and trying to collapse the pmd. So the window is quite
> tiny especially compared to I/O speeds, and this will IMHO catch
> only really frequent stalls. Placing it really at "when it goes to
> sleep" sounds better.
> 
> >  			ret = khugepaged_scan_pmd(mm, vma,
> >  						  khugepaged_scan.address,
> >  						  hpage);
> >
> 

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