* [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT
@ 2019-06-27 11:54 Minchan Kim
2019-06-27 11:54 ` [PATCH v3 1/5] mm: introduce MADV_COLD Minchan Kim
` (6 more replies)
0 siblings, 7 replies; 27+ messages in thread
From: Minchan Kim @ 2019-06-27 11:54 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, LKML, linux-api, Michal Hocko, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov, Minchan Kim
This patch is part of previous series:
https://lore.kernel.org/lkml/20190531064313.193437-1-minchan@kernel.org/T/#u
Originally, it was created for external madvise hinting feature.
https://lkml.org/lkml/2019/5/31/463
Michal wanted to separte the discussion from external hinting interface
so this patchset includes only first part of my entire patchset
- introduce MADV_COLD and MADV_PAGEOUT hint to madvise.
However, I keep entire description for others for easier understanding
why this kinds of hint was born.
Thanks.
This patchset is against on next-20190530.
Below is description of previous entire patchset.
================= &< =====================
- Background
The Android terminology used for forking a new process and starting an app
from scratch is a cold start, while resuming an existing app is a hot start.
While we continually try to improve the performance of cold starts, hot
starts will always be significantly less power hungry as well as faster so
we are trying to make hot start more likely than cold start.
To increase hot start, Android userspace manages the order that apps should
be killed in a process called ActivityManagerService. ActivityManagerService
tracks every Android app or service that the user could be interacting with
at any time and translates that into a ranked list for lmkd(low memory
killer daemon). They are likely to be killed by lmkd if the system has to
reclaim memory. In that sense they are similar to entries in any other cache.
Those apps are kept alive for opportunistic performance improvements but
those performance improvements will vary based on the memory requirements of
individual workloads.
- Problem
Naturally, cached apps were dominant consumers of memory on the system.
However, they were not significant consumers of swap even though they are
good candidate for swap. Under investigation, swapping out only begins
once the low zone watermark is hit and kswapd wakes up, but the overall
allocation rate in the system might trip lmkd thresholds and cause a cached
process to be killed(we measured performance swapping out vs. zapping the
memory by killing a process. Unsurprisingly, zapping is 10x times faster
even though we use zram which is much faster than real storage) so kill
from lmkd will often satisfy the high zone watermark, resulting in very
few pages actually being moved to swap.
- Approach
The approach we chose was to use a new interface to allow userspace to
proactively reclaim entire processes by leveraging platform information.
This allowed us to bypass the inaccuracy of the kernel’s LRUs for pages
that are known to be cold from userspace and to avoid races with lmkd
by reclaiming apps as soon as they entered the cached state. Additionally,
it could provide many chances for platform to use much information to
optimize memory efficiency.
To achieve the goal, the patchset introduce two new options for madvise.
One is MADV_COLD which will deactivate activated pages and the other is
MADV_PAGEOUT which will reclaim private pages instantly. These new options
complement MADV_DONTNEED and MADV_FREE by adding non-destructive ways to
gain some free memory space. MADV_PAGEOUT is similar to MADV_DONTNEED in a way
that it hints the kernel that memory region is not currently needed and
should be reclaimed immediately; MADV_COLD is similar to MADV_FREE in a way
that it hints the kernel that memory region is not currently needed and
should be reclaimed when memory pressure rises.
Minchan Kim (5):
mm: introduce MADV_COLD
mm: change PAGEREF_RECLAIM_CLEAN with PAGE_REFRECLAIM
mm: account nr_isolated_xxx in [isolate|putback]_lru_page
mm: introduce MADV_PAGEOUT
mm: factor out pmd young/dirty bit handling and THP split
include/linux/huge_mm.h | 3 -
include/linux/swap.h | 2 +
include/uapi/asm-generic/mman-common.h | 2 +
mm/compaction.c | 2 -
mm/gup.c | 7 +-
mm/huge_memory.c | 74 -----
mm/internal.h | 2 +-
mm/khugepaged.c | 3 -
mm/madvise.c | 438 ++++++++++++++++++++++++-
mm/memory-failure.c | 3 -
mm/memory_hotplug.c | 4 -
mm/mempolicy.c | 6 +-
mm/migrate.c | 37 +--
mm/oom_kill.c | 2 +-
mm/swap.c | 42 +++
mm/vmscan.c | 86 ++++-
16 files changed, 566 insertions(+), 147 deletions(-)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 1/5] mm: introduce MADV_COLD
2019-06-27 11:54 [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Minchan Kim
@ 2019-06-27 11:54 ` Minchan Kim
2019-06-27 13:13 ` Dave Hansen
2019-06-27 11:54 ` [PATCH v3 2/5] mm: change PAGEREF_RECLAIM_CLEAN with PAGE_REFRECLAIM Minchan Kim
` (5 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Minchan Kim @ 2019-06-27 11:54 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, LKML, linux-api, Michal Hocko, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov, Minchan Kim
When a process expects no accesses to a certain memory range, it could
give a hint to kernel that the pages can be reclaimed when memory pressure
happens but data should be preserved for future use. This could reduce
workingset eviction so it ends up increasing performance.
This patch introduces the new MADV_COLD hint to madvise(2) syscall.
MADV_COLD can be used by a process to mark a memory range as not expected
to be used in the near future. The hint can help kernel in deciding which
pages to evict early during memory pressure.
It works for every LRU pages like MADV_[DONTNEED|FREE]. IOW, It moves
active file page -> inactive file LRU
active anon page -> inacdtive anon LRU
Unlike MADV_FREE, it doesn't move active anonymous pages to inactive
file LRU's head because MADV_COLD is a little bit different symantic.
MADV_FREE means it's okay to discard when the memory pressure because
the content of the page is *garbage* so freeing such pages is almost zero
overhead since we don't need to swap out and access afterward causes just
minor fault. Thus, it would make sense to put those freeable pages in
inactive file LRU to compete other used-once pages. It makes sense for
implmentaion point of view, too because it's not swapbacked memory any
longer until it would be re-dirtied. Even, it could give a bonus to make
them be reclaimed on swapless system. However, MADV_COLD doesn't mean
garbage so reclaiming them requires swap-out/in in the end so it's bigger
cost. Since we have designed VM LRU aging based on cost-model, anonymous
cold pages would be better to position inactive anon's LRU list, not file
LRU. Furthermore, it would help to avoid unnecessary scanning if system
doesn't have a swap device. Let's start simpler way without adding
complexity at this moment. However, keep in mind, too that it's a caveat
that workloads with a lot of pages cache are likely to ignore MADV_COLD
on anonymous memory because we rarely age anonymous LRU lists.
* man-page material
MADV_COLD (since Linux x.x)
Do not expect access in the near future so under memory pressure, pages
in the specified regions could be reclaimed more aggressively compared
to other pages in the system. The difference with MADV_DONTNEED is it
doesn't change the semantics of memory access in the specified regions.
Thus, it will keep the up-to-date contents of the region.
MADV_COLD cannot be applied to locked pages, Huge TLB pages, or VM_PFNMAP
pages.
* v2
* add up the warn with lots of page cache workload - mhocko
* add man page stuff - dave
* v1
* remove page_mapcount filter - hannes, mhocko
* remove idle page handling - joelaf
* RFCv2
* add more description - mhocko
* RFCv1
* renaming from MADV_COOL to MADV_COLD - hannes
* internal review
* use clear_page_youn in deactivate_page - joelaf
* Revise the description - surenb
* Renaming from MADV_WARM to MADV_COOL - surenb
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
include/linux/swap.h | 1 +
include/uapi/asm-generic/mman-common.h | 1 +
mm/internal.h | 2 +-
mm/madvise.c | 180 ++++++++++++++++++++++++-
mm/oom_kill.c | 2 +-
mm/swap.c | 42 ++++++
6 files changed, 224 insertions(+), 4 deletions(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index de2c67a33b7e..0ce997edb8bb 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -340,6 +340,7 @@ extern void lru_add_drain_cpu(int cpu);
extern void lru_add_drain_all(void);
extern void rotate_reclaimable_page(struct page *page);
extern void deactivate_file_page(struct page *page);
+extern void deactivate_page(struct page *page);
extern void mark_page_lazyfree(struct page *page);
extern void swap_setup(void);
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index ef4623f03156..d7b4231eea63 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -47,6 +47,7 @@
#define MADV_SEQUENTIAL 2 /* expect sequential page references */
#define MADV_WILLNEED 3 /* will need these pages */
#define MADV_DONTNEED 4 /* don't need these pages */
+#define MADV_COLD 5 /* deactivatie these pages */
/* common parameters: try to keep these consistent across architectures */
#define MADV_FREE 8 /* free pages only if memory pressure */
diff --git a/mm/internal.h b/mm/internal.h
index f53a14d67538..c61b215ff265 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -39,7 +39,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf);
void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
unsigned long floor, unsigned long ceiling);
-static inline bool can_madv_dontneed_vma(struct vm_area_struct *vma)
+static inline bool can_madv_lru_vma(struct vm_area_struct *vma)
{
return !(vma->vm_flags & (VM_LOCKED|VM_HUGETLB|VM_PFNMAP));
}
diff --git a/mm/madvise.c b/mm/madvise.c
index 628022e674a7..7abb8e54bc7a 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -40,6 +40,7 @@ static int madvise_need_mmap_write(int behavior)
case MADV_REMOVE:
case MADV_WILLNEED:
case MADV_DONTNEED:
+ case MADV_COLD:
case MADV_FREE:
return 0;
default:
@@ -307,6 +308,178 @@ static long madvise_willneed(struct vm_area_struct *vma,
return 0;
}
+static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
+ unsigned long end, struct mm_walk *walk)
+{
+ struct mmu_gather *tlb = walk->private;
+ struct mm_struct *mm = tlb->mm;
+ struct vm_area_struct *vma = walk->vma;
+ pte_t *orig_pte, *pte, ptent;
+ spinlock_t *ptl;
+ struct page *page;
+ unsigned long next;
+
+ next = pmd_addr_end(addr, end);
+ if (pmd_trans_huge(*pmd)) {
+ pmd_t orig_pmd;
+
+ tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
+ ptl = pmd_trans_huge_lock(pmd, vma);
+ if (!ptl)
+ return 0;
+
+ orig_pmd = *pmd;
+ if (is_huge_zero_pmd(orig_pmd))
+ goto huge_unlock;
+
+ if (unlikely(!pmd_present(orig_pmd))) {
+ VM_BUG_ON(thp_migration_supported() &&
+ !is_pmd_migration_entry(orig_pmd));
+ goto huge_unlock;
+ }
+
+ page = pmd_page(orig_pmd);
+ if (next - addr != HPAGE_PMD_SIZE) {
+ int err;
+
+ if (page_mapcount(page) != 1)
+ goto huge_unlock;
+
+ get_page(page);
+ spin_unlock(ptl);
+ lock_page(page);
+ err = split_huge_page(page);
+ unlock_page(page);
+ put_page(page);
+ if (!err)
+ goto regular_page;
+ return 0;
+ }
+
+ if (pmd_young(orig_pmd)) {
+ pmdp_invalidate(vma, addr, pmd);
+ orig_pmd = pmd_mkold(orig_pmd);
+
+ set_pmd_at(mm, addr, pmd, orig_pmd);
+ tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
+ }
+
+ test_and_clear_page_young(page);
+ deactivate_page(page);
+huge_unlock:
+ spin_unlock(ptl);
+ return 0;
+ }
+
+ if (pmd_trans_unstable(pmd))
+ return 0;
+
+regular_page:
+ tlb_change_page_size(tlb, PAGE_SIZE);
+ orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
+ flush_tlb_batched_pending(mm);
+ arch_enter_lazy_mmu_mode();
+ for (; addr < end; pte++, addr += PAGE_SIZE) {
+ ptent = *pte;
+
+ if (pte_none(ptent))
+ continue;
+
+ if (!pte_present(ptent))
+ continue;
+
+ page = vm_normal_page(vma, addr, ptent);
+ if (!page)
+ continue;
+
+ /*
+ * Creating a THP page is expensive so split it only if we
+ * are sure it's worth. Split it if we are only owner.
+ */
+ if (PageTransCompound(page)) {
+ if (page_mapcount(page) != 1)
+ break;
+ get_page(page);
+ if (!trylock_page(page)) {
+ put_page(page);
+ break;
+ }
+ pte_unmap_unlock(orig_pte, ptl);
+ if (split_huge_page(page)) {
+ unlock_page(page);
+ put_page(page);
+ pte_offset_map_lock(mm, pmd, addr, &ptl);
+ break;
+ }
+ unlock_page(page);
+ put_page(page);
+ pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
+ pte--;
+ addr -= PAGE_SIZE;
+ continue;
+ }
+
+ VM_BUG_ON_PAGE(PageTransCompound(page), page);
+
+ if (pte_young(ptent)) {
+ ptent = ptep_get_and_clear_full(mm, addr, pte,
+ tlb->fullmm);
+ ptent = pte_mkold(ptent);
+ set_pte_at(mm, addr, pte, ptent);
+ tlb_remove_tlb_entry(tlb, pte, addr);
+ }
+
+ /*
+ * We are deactivating a page for accelerating reclaiming.
+ * VM couldn't reclaim the page unless we clear PG_young.
+ * As a side effect, it makes confuse idle-page tracking
+ * because they will miss recent referenced history.
+ */
+ test_and_clear_page_young(page);
+ deactivate_page(page);
+ }
+
+ arch_enter_lazy_mmu_mode();
+ pte_unmap_unlock(orig_pte, ptl);
+ cond_resched();
+
+ return 0;
+}
+
+static void madvise_cold_page_range(struct mmu_gather *tlb,
+ struct vm_area_struct *vma,
+ unsigned long addr, unsigned long end)
+{
+ struct mm_walk cold_walk = {
+ .pmd_entry = madvise_cold_pte_range,
+ .mm = vma->vm_mm,
+ .private = tlb,
+ };
+
+ tlb_start_vma(tlb, vma);
+ walk_page_range(addr, end, &cold_walk);
+ tlb_end_vma(tlb, vma);
+}
+
+static long madvise_cold(struct vm_area_struct *vma,
+ struct vm_area_struct **prev,
+ unsigned long start_addr, unsigned long end_addr)
+{
+ struct mm_struct *mm = vma->vm_mm;
+ struct mmu_gather tlb;
+
+ *prev = vma;
+ if (!can_madv_lru_vma(vma))
+ return -EINVAL;
+
+ lru_add_drain();
+ tlb_gather_mmu(&tlb, mm, start_addr, end_addr);
+ madvise_cold_page_range(&tlb, vma, start_addr, end_addr);
+ tlb_finish_mmu(&tlb, start_addr, end_addr);
+
+ return 0;
+}
+
static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
unsigned long end, struct mm_walk *walk)
@@ -519,7 +692,7 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
int behavior)
{
*prev = vma;
- if (!can_madv_dontneed_vma(vma))
+ if (!can_madv_lru_vma(vma))
return -EINVAL;
if (!userfaultfd_remove(vma, start, end)) {
@@ -541,7 +714,7 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
*/
return -ENOMEM;
}
- if (!can_madv_dontneed_vma(vma))
+ if (!can_madv_lru_vma(vma))
return -EINVAL;
if (end > vma->vm_end) {
/*
@@ -695,6 +868,8 @@ madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
return madvise_remove(vma, prev, start, end);
case MADV_WILLNEED:
return madvise_willneed(vma, prev, start, end);
+ case MADV_COLD:
+ return madvise_cold(vma, prev, start, end);
case MADV_FREE:
case MADV_DONTNEED:
return madvise_dontneed_free(vma, prev, start, end, behavior);
@@ -716,6 +891,7 @@ madvise_behavior_valid(int behavior)
case MADV_WILLNEED:
case MADV_DONTNEED:
case MADV_FREE:
+ case MADV_COLD:
#ifdef CONFIG_KSM
case MADV_MERGEABLE:
case MADV_UNMERGEABLE:
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 6de5c354d6ca..2140a6f8db63 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -523,7 +523,7 @@ bool __oom_reap_task_mm(struct mm_struct *mm)
set_bit(MMF_UNSTABLE, &mm->flags);
for (vma = mm->mmap ; vma; vma = vma->vm_next) {
- if (!can_madv_dontneed_vma(vma))
+ if (!can_madv_lru_vma(vma))
continue;
/*
diff --git a/mm/swap.c b/mm/swap.c
index 607c48229a1d..a91859d061f3 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -47,6 +47,7 @@ int page_cluster;
static DEFINE_PER_CPU(struct pagevec, lru_add_pvec);
static DEFINE_PER_CPU(struct pagevec, lru_rotate_pvecs);
static DEFINE_PER_CPU(struct pagevec, lru_deactivate_file_pvecs);
+static DEFINE_PER_CPU(struct pagevec, lru_deactivate_pvecs);
static DEFINE_PER_CPU(struct pagevec, lru_lazyfree_pvecs);
#ifdef CONFIG_SMP
static DEFINE_PER_CPU(struct pagevec, activate_page_pvecs);
@@ -538,6 +539,22 @@ static void lru_deactivate_file_fn(struct page *page, struct lruvec *lruvec,
update_page_reclaim_stat(lruvec, file, 0);
}
+static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec,
+ void *arg)
+{
+ if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
+ int file = page_is_file_cache(page);
+ int lru = page_lru_base_type(page);
+
+ del_page_from_lru_list(page, lruvec, lru + LRU_ACTIVE);
+ ClearPageActive(page);
+ ClearPageReferenced(page);
+ add_page_to_lru_list(page, lruvec, lru);
+
+ __count_vm_events(PGDEACTIVATE, hpage_nr_pages(page));
+ update_page_reclaim_stat(lruvec, file, 0);
+ }
+}
static void lru_lazyfree_fn(struct page *page, struct lruvec *lruvec,
void *arg)
@@ -590,6 +607,10 @@ void lru_add_drain_cpu(int cpu)
if (pagevec_count(pvec))
pagevec_lru_move_fn(pvec, lru_deactivate_file_fn, NULL);
+ pvec = &per_cpu(lru_deactivate_pvecs, cpu);
+ if (pagevec_count(pvec))
+ pagevec_lru_move_fn(pvec, lru_deactivate_fn, NULL);
+
pvec = &per_cpu(lru_lazyfree_pvecs, cpu);
if (pagevec_count(pvec))
pagevec_lru_move_fn(pvec, lru_lazyfree_fn, NULL);
@@ -623,6 +644,26 @@ void deactivate_file_page(struct page *page)
}
}
+/*
+ * deactivate_page - deactivate a page
+ * @page: page to deactivate
+ *
+ * deactivate_page() moves @page to the inactive list if @page was on the active
+ * list and was not an unevictable page. This is done to accelerate the reclaim
+ * of @page.
+ */
+void deactivate_page(struct page *page)
+{
+ if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
+ struct pagevec *pvec = &get_cpu_var(lru_deactivate_pvecs);
+
+ get_page(page);
+ if (!pagevec_add(pvec, page) || PageCompound(page))
+ pagevec_lru_move_fn(pvec, lru_deactivate_fn, NULL);
+ put_cpu_var(lru_deactivate_pvecs);
+ }
+}
+
/**
* mark_page_lazyfree - make an anon page lazyfree
* @page: page to deactivate
@@ -687,6 +728,7 @@ void lru_add_drain_all(void)
if (pagevec_count(&per_cpu(lru_add_pvec, cpu)) ||
pagevec_count(&per_cpu(lru_rotate_pvecs, cpu)) ||
pagevec_count(&per_cpu(lru_deactivate_file_pvecs, cpu)) ||
+ pagevec_count(&per_cpu(lru_deactivate_pvecs, cpu)) ||
pagevec_count(&per_cpu(lru_lazyfree_pvecs, cpu)) ||
need_activate_page_drain(cpu)) {
INIT_WORK(work, lru_add_drain_per_cpu);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v3 2/5] mm: change PAGEREF_RECLAIM_CLEAN with PAGE_REFRECLAIM
2019-06-27 11:54 [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Minchan Kim
2019-06-27 11:54 ` [PATCH v3 1/5] mm: introduce MADV_COLD Minchan Kim
@ 2019-06-27 11:54 ` Minchan Kim
2019-06-27 11:54 ` [PATCH v3 3/5] mm: account nr_isolated_xxx in [isolate|putback]_lru_page Minchan Kim
` (4 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Minchan Kim @ 2019-06-27 11:54 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, LKML, linux-api, Michal Hocko, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov, Minchan Kim
The local variable references in shrink_page_list is PAGEREF_RECLAIM_CLEAN
as default. It is for preventing to reclaim dirty pages when CMA try to
migrate pages. Strictly speaking, we don't need it because CMA didn't allow
to write out by .may_writepage = 0 in reclaim_clean_pages_from_list.
Moreover, it has a problem to prevent anonymous pages's swap out even
though force_reclaim = true in shrink_page_list on upcoming patch.
So this patch makes references's default value to PAGEREF_RECLAIM and
rename force_reclaim with ignore_references to make it more clear.
This is a preparatory work for next patch.
* RFCv1
* use ignore_referecnes as parameter name - hannes
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
mm/vmscan.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 9e3292ee5c7c..49e9ee4d771d 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1117,7 +1117,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
struct scan_control *sc,
enum ttu_flags ttu_flags,
struct reclaim_stat *stat,
- bool force_reclaim)
+ bool ignore_references)
{
LIST_HEAD(ret_pages);
LIST_HEAD(free_pages);
@@ -1131,7 +1131,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
struct address_space *mapping;
struct page *page;
int may_enter_fs;
- enum page_references references = PAGEREF_RECLAIM_CLEAN;
+ enum page_references references = PAGEREF_RECLAIM;
bool dirty, writeback;
unsigned int nr_pages;
@@ -1262,7 +1262,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
}
}
- if (!force_reclaim)
+ if (!ignore_references)
references = page_check_references(page, sc);
switch (references) {
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v3 3/5] mm: account nr_isolated_xxx in [isolate|putback]_lru_page
2019-06-27 11:54 [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Minchan Kim
2019-06-27 11:54 ` [PATCH v3 1/5] mm: introduce MADV_COLD Minchan Kim
2019-06-27 11:54 ` [PATCH v3 2/5] mm: change PAGEREF_RECLAIM_CLEAN with PAGE_REFRECLAIM Minchan Kim
@ 2019-06-27 11:54 ` Minchan Kim
2019-07-09 9:38 ` Michal Hocko
2019-06-27 11:54 ` [PATCH v3 4/5] mm: introduce MADV_PAGEOUT Minchan Kim
` (3 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Minchan Kim @ 2019-06-27 11:54 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, LKML, linux-api, Michal Hocko, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov, Minchan Kim
The isolate counting is pecpu counter so it would be not huge gain
to work them by batch. Rather than complicating to make them batch,
let's make it more stright-foward via adding the counting logic
into [isolate|putback]_lru_page API.
* v1
* fix accounting bug - Hillf
Link: http://lkml.kernel.org/r/20190531165927.GA20067@cmpxchg.org
Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
mm/compaction.c | 2 --
mm/gup.c | 7 +------
mm/khugepaged.c | 3 ---
mm/memory-failure.c | 3 ---
mm/memory_hotplug.c | 4 ----
mm/mempolicy.c | 6 +-----
mm/migrate.c | 37 ++++++++-----------------------------
mm/vmscan.c | 22 ++++++++++++++++------
8 files changed, 26 insertions(+), 58 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index 9e1b9acb116b..c6591682deda 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -982,8 +982,6 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
/* Successfully isolated */
del_page_from_lru_list(page, lruvec, page_lru(page));
- inc_node_page_state(page,
- NR_ISOLATED_ANON + page_is_file_cache(page));
isolate_success:
list_add(&page->lru, &cc->migratepages);
diff --git a/mm/gup.c b/mm/gup.c
index 7dde2e3a1963..aec3a2b7e61b 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1473,13 +1473,8 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk,
drain_allow = false;
}
- if (!isolate_lru_page(head)) {
+ if (!isolate_lru_page(head))
list_add_tail(&head->lru, &cma_page_list);
- mod_node_page_state(page_pgdat(head),
- NR_ISOLATED_ANON +
- page_is_file_cache(head),
- hpage_nr_pages(head));
- }
}
}
}
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 0f7419938008..7da34e198ec5 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -503,7 +503,6 @@ void __khugepaged_exit(struct mm_struct *mm)
static void release_pte_page(struct page *page)
{
- dec_node_page_state(page, NR_ISOLATED_ANON + page_is_file_cache(page));
unlock_page(page);
putback_lru_page(page);
}
@@ -602,8 +601,6 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
result = SCAN_DEL_PAGE_LRU;
goto out;
}
- inc_node_page_state(page,
- NR_ISOLATED_ANON + page_is_file_cache(page));
VM_BUG_ON_PAGE(!PageLocked(page), page);
VM_BUG_ON_PAGE(PageLRU(page), page);
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 7e08cbf3ba49..3586e8226e4e 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1795,9 +1795,6 @@ static int __soft_offline_page(struct page *page, int flags)
* so use !__PageMovable instead for LRU page's mapping
* cannot have PAGE_MAPPING_MOVABLE.
*/
- if (!__PageMovable(page))
- inc_node_page_state(page, NR_ISOLATED_ANON +
- page_is_file_cache(page));
list_add(&page->lru, &pagelist);
ret = migrate_pages(&pagelist, new_page, NULL, MPOL_MF_MOVE_ALL,
MIGRATE_SYNC, MR_MEMORY_FAILURE);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index dfab21dc33dc..68577c677b46 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1384,10 +1384,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
ret = isolate_movable_page(page, ISOLATE_UNEVICTABLE);
if (!ret) { /* Success */
list_add_tail(&page->lru, &source);
- if (!__PageMovable(page))
- inc_node_page_state(page, NR_ISOLATED_ANON +
- page_is_file_cache(page));
-
} else {
pr_warn("failed to isolate pfn %lx\n", pfn);
dump_page(page, "isolation failed");
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 64562809bf3b..03081f3404ca 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -994,12 +994,8 @@ static int migrate_page_add(struct page *page, struct list_head *pagelist,
* Avoid migrating a page that is shared with others.
*/
if ((flags & MPOL_MF_MOVE_ALL) || page_mapcount(head) == 1) {
- if (!isolate_lru_page(head)) {
+ if (!isolate_lru_page(head))
list_add_tail(&head->lru, pagelist);
- mod_node_page_state(page_pgdat(head),
- NR_ISOLATED_ANON + page_is_file_cache(head),
- hpage_nr_pages(head));
- }
}
return 0;
diff --git a/mm/migrate.c b/mm/migrate.c
index 572b4bc85d76..5583324c01e7 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -190,8 +190,6 @@ void putback_movable_pages(struct list_head *l)
unlock_page(page);
put_page(page);
} else {
- mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
- page_is_file_cache(page), -hpage_nr_pages(page));
putback_lru_page(page);
}
}
@@ -1181,10 +1179,17 @@ static ICE_noinline int unmap_and_move(new_page_t get_new_page,
return -ENOMEM;
if (page_count(page) == 1) {
+ bool is_lru = !__PageMovable(page);
+
/* page was freed from under us. So we are done. */
ClearPageActive(page);
ClearPageUnevictable(page);
- if (unlikely(__PageMovable(page))) {
+ if (likely(is_lru))
+ mod_node_page_state(page_pgdat(page),
+ NR_ISOLATED_ANON +
+ page_is_file_cache(page),
+ -hpage_nr_pages(page));
+ else {
lock_page(page);
if (!PageMovable(page))
__ClearPageIsolated(page);
@@ -1210,15 +1215,6 @@ static ICE_noinline int unmap_and_move(new_page_t get_new_page,
* restored.
*/
list_del(&page->lru);
-
- /*
- * Compaction can migrate also non-LRU pages which are
- * not accounted to NR_ISOLATED_*. They can be recognized
- * as __PageMovable
- */
- if (likely(!__PageMovable(page)))
- mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
- page_is_file_cache(page), -hpage_nr_pages(page));
}
/*
@@ -1572,9 +1568,6 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr,
err = 0;
list_add_tail(&head->lru, pagelist);
- mod_node_page_state(page_pgdat(head),
- NR_ISOLATED_ANON + page_is_file_cache(head),
- hpage_nr_pages(head));
}
out_putpage:
/*
@@ -1890,8 +1883,6 @@ static struct page *alloc_misplaced_dst_page(struct page *page,
static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
{
- int page_lru;
-
VM_BUG_ON_PAGE(compound_order(page) && !PageTransHuge(page), page);
/* Avoid migrating to a node that is nearly full */
@@ -1913,10 +1904,6 @@ static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
return 0;
}
- page_lru = page_is_file_cache(page);
- mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON + page_lru,
- hpage_nr_pages(page));
-
/*
* Isolating the page has taken another reference, so the
* caller's reference can be safely dropped without the page
@@ -1971,8 +1958,6 @@ int migrate_misplaced_page(struct page *page, struct vm_area_struct *vma,
if (nr_remaining) {
if (!list_empty(&migratepages)) {
list_del(&page->lru);
- dec_node_page_state(page, NR_ISOLATED_ANON +
- page_is_file_cache(page));
putback_lru_page(page);
}
isolated = 0;
@@ -2002,7 +1987,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
pg_data_t *pgdat = NODE_DATA(node);
int isolated = 0;
struct page *new_page = NULL;
- int page_lru = page_is_file_cache(page);
unsigned long start = address & HPAGE_PMD_MASK;
new_page = alloc_pages_node(node,
@@ -2048,8 +2032,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
/* Retake the callers reference and putback on LRU */
get_page(page);
putback_lru_page(page);
- mod_node_page_state(page_pgdat(page),
- NR_ISOLATED_ANON + page_lru, -HPAGE_PMD_NR);
goto out_unlock;
}
@@ -2099,9 +2081,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
count_vm_events(PGMIGRATE_SUCCESS, HPAGE_PMD_NR);
count_vm_numa_events(NUMA_PAGE_MIGRATE, HPAGE_PMD_NR);
- mod_node_page_state(page_pgdat(page),
- NR_ISOLATED_ANON + page_lru,
- -HPAGE_PMD_NR);
return isolated;
out_fail:
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 49e9ee4d771d..223ce5da08f0 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1014,6 +1014,9 @@ int remove_mapping(struct address_space *mapping, struct page *page)
void putback_lru_page(struct page *page)
{
lru_cache_add(page);
+ mod_node_page_state(page_pgdat(page),
+ NR_ISOLATED_ANON + page_is_file_cache(page),
+ -hpage_nr_pages(page));
put_page(page); /* drop ref from isolate */
}
@@ -1479,6 +1482,9 @@ static unsigned long shrink_page_list(struct list_head *page_list,
*/
nr_reclaimed += nr_pages;
+ mod_node_page_state(pgdat, NR_ISOLATED_ANON +
+ page_is_file_cache(page),
+ -nr_pages);
/*
* Is there need to periodically free_page_list? It would
* appear not as the counts should be low
@@ -1554,7 +1560,6 @@ unsigned long reclaim_clean_pages_from_list(struct zone *zone,
ret = shrink_page_list(&clean_pages, zone->zone_pgdat, &sc,
TTU_IGNORE_ACCESS, &dummy_stat, true);
list_splice(&clean_pages, page_list);
- mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_FILE, -ret);
return ret;
}
@@ -1630,6 +1635,9 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode)
*/
ClearPageLRU(page);
ret = 0;
+ __mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
+ page_is_file_cache(page),
+ hpage_nr_pages(page));
}
return ret;
@@ -1761,6 +1769,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
total_scan, skipped, nr_taken, mode, lru);
update_lru_sizes(lruvec, lru, nr_zone_taken);
+
return nr_taken;
}
@@ -1809,6 +1818,9 @@ int isolate_lru_page(struct page *page)
ClearPageLRU(page);
del_page_from_lru_list(page, lruvec, lru);
ret = 0;
+ mod_node_page_state(pgdat, NR_ISOLATED_ANON +
+ page_is_file_cache(page),
+ hpage_nr_pages(page));
}
spin_unlock_irq(&pgdat->lru_lock);
}
@@ -1900,6 +1912,9 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec,
update_lru_size(lruvec, lru, page_zonenum(page), nr_pages);
list_move(&page->lru, &lruvec->lists[lru]);
+ __mod_node_page_state(pgdat, NR_ISOLATED_ANON +
+ page_is_file_cache(page),
+ -hpage_nr_pages(page));
if (put_page_testzero(page)) {
__ClearPageLRU(page);
__ClearPageActive(page);
@@ -1977,7 +1992,6 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
nr_taken = isolate_lru_pages(nr_to_scan, lruvec, &page_list,
&nr_scanned, sc, lru);
- __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken);
reclaim_stat->recent_scanned[file] += nr_taken;
item = current_is_kswapd() ? PGSCAN_KSWAPD : PGSCAN_DIRECT;
@@ -2003,8 +2017,6 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
move_pages_to_lru(lruvec, &page_list);
- __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, -nr_taken);
-
spin_unlock_irq(&pgdat->lru_lock);
mem_cgroup_uncharge_list(&page_list);
@@ -2063,7 +2075,6 @@ static void shrink_active_list(unsigned long nr_to_scan,
nr_taken = isolate_lru_pages(nr_to_scan, lruvec, &l_hold,
&nr_scanned, sc, lru);
- __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken);
reclaim_stat->recent_scanned[file] += nr_taken;
__count_vm_events(PGREFILL, nr_scanned);
@@ -2132,7 +2143,6 @@ static void shrink_active_list(unsigned long nr_to_scan,
__count_vm_events(PGDEACTIVATE, nr_deactivate);
__count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE, nr_deactivate);
- __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, -nr_taken);
spin_unlock_irq(&pgdat->lru_lock);
mem_cgroup_uncharge_list(&l_active);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v3 4/5] mm: introduce MADV_PAGEOUT
2019-06-27 11:54 [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Minchan Kim
` (2 preceding siblings ...)
2019-06-27 11:54 ` [PATCH v3 3/5] mm: account nr_isolated_xxx in [isolate|putback]_lru_page Minchan Kim
@ 2019-06-27 11:54 ` Minchan Kim
2019-07-09 9:55 ` Michal Hocko
2019-06-27 11:54 ` [PATCH v3 5/5] mm: factor out pmd young/dirty bit handling and THP split Minchan Kim
` (2 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Minchan Kim @ 2019-06-27 11:54 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, LKML, linux-api, Michal Hocko, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov, Minchan Kim
When a process expects no accesses to a certain memory range
for a long time, it could hint kernel that the pages can be
reclaimed instantly but data should be preserved for future use.
This could reduce workingset eviction so it ends up increasing
performance.
This patch introduces the new MADV_PAGEOUT hint to madvise(2)
syscall. MADV_PAGEOUT can be used by a process to mark a memory
range as not expected to be used for a long time so that kernel
reclaims *any LRU* pages instantly. The hint can help kernel in
deciding which pages to evict proactively.
- man-page material
MADV_PAGEOUT (since Linux x.x)
Do not expect access in the near future so pages in the specified
regions could be reclaimed instantly regardless of memory pressure.
Thus, access in the range after successful operation could cause
major page fault but never lose the up-to-date contents unlike
MADV_DONTNEED. It works for only private anonymous mappings and
non-anonymous mappings that belong to files that the calling process
could successfully open for writing; otherwise, it could be used for
sidechannel attack.
MADV_PAGEOUT cannot be applied to locked pages, Huge TLB pages, or
VM_PFNMAP pages.
* v2
* add comment about SWAP_CLUSTER_MAX - mhocko
* add permission check to prevent sidechannel attack - mhocko
* add man page stuff - dave
* v1
* change pte to old and rely on the other's reference - hannes
* remove page_mapcount to check shared page - mhocko
* RFC v2
* make reclaim_pages simple via factoring out isolate logic - hannes
* RFCv1
* rename from MADV_COLD to MADV_PAGEOUT - hannes
* bail out if process is being killed - Hillf
* fix reclaim_pages bugs - Hillf
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
include/linux/swap.h | 1 +
include/uapi/asm-generic/mman-common.h | 1 +
mm/madvise.c | 212 +++++++++++++++++++++++++
mm/vmscan.c | 58 +++++++
4 files changed, 272 insertions(+)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 0ce997edb8bb..063c0c1e112b 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -365,6 +365,7 @@ extern int vm_swappiness;
extern int remove_mapping(struct address_space *mapping, struct page *page);
extern unsigned long vm_total_pages;
+extern unsigned long reclaim_pages(struct list_head *page_list);
#ifdef CONFIG_NUMA
extern int node_reclaim_mode;
extern int sysctl_min_unmapped_ratio;
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index d7b4231eea63..f545e159b472 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -48,6 +48,7 @@
#define MADV_WILLNEED 3 /* will need these pages */
#define MADV_DONTNEED 4 /* don't need these pages */
#define MADV_COLD 5 /* deactivatie these pages */
+#define MADV_PAGEOUT 6 /* reclaim these pages */
/* common parameters: try to keep these consistent across architectures */
#define MADV_FREE 8 /* free pages only if memory pressure */
diff --git a/mm/madvise.c b/mm/madvise.c
index 7abb8e54bc7a..ee210473f639 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -11,6 +11,7 @@
#include <linux/syscalls.h>
#include <linux/mempolicy.h>
#include <linux/page-isolation.h>
+#include <linux/page_idle.h>
#include <linux/userfaultfd_k.h>
#include <linux/hugetlb.h>
#include <linux/falloc.h>
@@ -41,6 +42,7 @@ static int madvise_need_mmap_write(int behavior)
case MADV_WILLNEED:
case MADV_DONTNEED:
case MADV_COLD:
+ case MADV_PAGEOUT:
case MADV_FREE:
return 0;
default:
@@ -480,6 +482,213 @@ static long madvise_cold(struct vm_area_struct *vma,
return 0;
}
+static int madvise_pageout_pte_range(pmd_t *pmd, unsigned long addr,
+ unsigned long end, struct mm_walk *walk)
+{
+ struct mmu_gather *tlb = walk->private;
+ struct mm_struct *mm = tlb->mm;
+ struct vm_area_struct *vma = walk->vma;
+ pte_t *orig_pte, *pte, ptent;
+ spinlock_t *ptl;
+ LIST_HEAD(page_list);
+ struct page *page;
+ int isolated = 0;
+ unsigned long next;
+
+ if (fatal_signal_pending(current))
+ return -EINTR;
+
+ next = pmd_addr_end(addr, end);
+ if (pmd_trans_huge(*pmd)) {
+ pmd_t orig_pmd;
+
+ tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
+ ptl = pmd_trans_huge_lock(pmd, vma);
+ if (!ptl)
+ return 0;
+
+ orig_pmd = *pmd;
+ if (is_huge_zero_pmd(orig_pmd))
+ goto huge_unlock;
+
+ if (unlikely(!pmd_present(orig_pmd))) {
+ VM_BUG_ON(thp_migration_supported() &&
+ !is_pmd_migration_entry(orig_pmd));
+ goto huge_unlock;
+ }
+
+ page = pmd_page(orig_pmd);
+ if (next - addr != HPAGE_PMD_SIZE) {
+ int err;
+
+ if (page_mapcount(page) != 1)
+ goto huge_unlock;
+ get_page(page);
+ spin_unlock(ptl);
+ lock_page(page);
+ err = split_huge_page(page);
+ unlock_page(page);
+ put_page(page);
+ if (!err)
+ goto regular_page;
+ return 0;
+ }
+
+ if (isolate_lru_page(page))
+ goto huge_unlock;
+
+ if (pmd_young(orig_pmd)) {
+ pmdp_invalidate(vma, addr, pmd);
+ orig_pmd = pmd_mkold(orig_pmd);
+
+ set_pmd_at(mm, addr, pmd, orig_pmd);
+ tlb_remove_tlb_entry(tlb, pmd, addr);
+ }
+
+ ClearPageReferenced(page);
+ test_and_clear_page_young(page);
+ list_add(&page->lru, &page_list);
+huge_unlock:
+ spin_unlock(ptl);
+ reclaim_pages(&page_list);
+ return 0;
+ }
+
+ if (pmd_trans_unstable(pmd))
+ return 0;
+regular_page:
+ tlb_change_page_size(tlb, PAGE_SIZE);
+ orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
+ flush_tlb_batched_pending(mm);
+ arch_enter_lazy_mmu_mode();
+ for (; addr < end; pte++, addr += PAGE_SIZE) {
+ ptent = *pte;
+ if (!pte_present(ptent))
+ continue;
+
+ page = vm_normal_page(vma, addr, ptent);
+ if (!page)
+ continue;
+
+ /*
+ * creating a THP page is expensive so split it only if we
+ * are sure it's worth. Split it if we are only owner.
+ */
+ if (PageTransCompound(page)) {
+ if (page_mapcount(page) != 1)
+ break;
+ get_page(page);
+ if (!trylock_page(page)) {
+ put_page(page);
+ break;
+ }
+ pte_unmap_unlock(orig_pte, ptl);
+ if (split_huge_page(page)) {
+ unlock_page(page);
+ put_page(page);
+ pte_offset_map_lock(mm, pmd, addr, &ptl);
+ break;
+ }
+ unlock_page(page);
+ put_page(page);
+ pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
+ pte--;
+ addr -= PAGE_SIZE;
+ continue;
+ }
+
+ VM_BUG_ON_PAGE(PageTransCompound(page), page);
+
+ if (isolate_lru_page(page))
+ continue;
+
+ isolated++;
+ if (pte_young(ptent)) {
+ ptent = ptep_get_and_clear_full(mm, addr, pte,
+ tlb->fullmm);
+ ptent = pte_mkold(ptent);
+ set_pte_at(mm, addr, pte, ptent);
+ tlb_remove_tlb_entry(tlb, pte, addr);
+ }
+ ClearPageReferenced(page);
+ test_and_clear_page_young(page);
+ list_add(&page->lru, &page_list);
+ /*
+ * Prevent early OOM kill since it could isolate too many LRU
+ * pages concurrently.
+ */
+ if (isolated >= SWAP_CLUSTER_MAX) {
+ arch_leave_lazy_mmu_mode();
+ pte_unmap_unlock(orig_pte, ptl);
+ reclaim_pages(&page_list);
+ isolated = 0;
+ pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
+ arch_enter_lazy_mmu_mode();
+ orig_pte = pte;
+ }
+ }
+
+ arch_leave_lazy_mmu_mode();
+ pte_unmap_unlock(orig_pte, ptl);
+ reclaim_pages(&page_list);
+ cond_resched();
+
+ return 0;
+}
+
+static void madvise_pageout_page_range(struct mmu_gather *tlb,
+ struct vm_area_struct *vma,
+ unsigned long addr, unsigned long end)
+{
+ struct mm_walk pageout_walk = {
+ .pmd_entry = madvise_pageout_pte_range,
+ .mm = vma->vm_mm,
+ .private = tlb,
+ };
+
+ tlb_start_vma(tlb, vma);
+ walk_page_range(addr, end, &pageout_walk);
+ tlb_end_vma(tlb, vma);
+}
+
+static inline bool can_do_pageout(struct vm_area_struct *vma)
+{
+ if (vma_is_anonymous(vma))
+ return true;
+ if (!vma->vm_file)
+ return false;
+ /*
+ * paging out pagecache only for non-anonymous mappings that correspond
+ * to the files the calling process could (if tried) open for writing;
+ * otherwise we'd be including shared non-exclusive mappings, which
+ * opens a side channel.
+ */
+ return inode_owner_or_capable(file_inode(vma->vm_file)) ||
+ inode_permission(file_inode(vma->vm_file), MAY_WRITE) == 0;
+}
+
+static long madvise_pageout(struct vm_area_struct *vma,
+ struct vm_area_struct **prev,
+ unsigned long start_addr, unsigned long end_addr)
+{
+ struct mm_struct *mm = vma->vm_mm;
+ struct mmu_gather tlb;
+
+ *prev = vma;
+ if (!can_madv_lru_vma(vma))
+ return -EINVAL;
+
+ if (!can_do_pageout(vma))
+ return 0;
+
+ lru_add_drain();
+ tlb_gather_mmu(&tlb, mm, start_addr, end_addr);
+ madvise_pageout_page_range(&tlb, vma, start_addr, end_addr);
+ tlb_finish_mmu(&tlb, start_addr, end_addr);
+
+ return 0;
+}
+
static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
unsigned long end, struct mm_walk *walk)
@@ -870,6 +1079,8 @@ madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
return madvise_willneed(vma, prev, start, end);
case MADV_COLD:
return madvise_cold(vma, prev, start, end);
+ case MADV_PAGEOUT:
+ return madvise_pageout(vma, prev, start, end);
case MADV_FREE:
case MADV_DONTNEED:
return madvise_dontneed_free(vma, prev, start, end, behavior);
@@ -892,6 +1103,7 @@ madvise_behavior_valid(int behavior)
case MADV_DONTNEED:
case MADV_FREE:
case MADV_COLD:
+ case MADV_PAGEOUT:
#ifdef CONFIG_KSM
case MADV_MERGEABLE:
case MADV_UNMERGEABLE:
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 223ce5da08f0..b521770c4314 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2151,6 +2151,64 @@ static void shrink_active_list(unsigned long nr_to_scan,
nr_deactivate, nr_rotated, sc->priority, file);
}
+unsigned long reclaim_pages(struct list_head *page_list)
+{
+ int nid = -1;
+ unsigned long nr_reclaimed = 0;
+ LIST_HEAD(node_page_list);
+ struct reclaim_stat dummy_stat;
+ struct scan_control sc = {
+ .gfp_mask = GFP_KERNEL,
+ .priority = DEF_PRIORITY,
+ .may_writepage = 1,
+ .may_unmap = 1,
+ .may_swap = 1,
+ };
+
+ while (!list_empty(page_list)) {
+ struct page *page;
+
+ page = lru_to_page(page_list);
+ if (nid == -1) {
+ nid = page_to_nid(page);
+ INIT_LIST_HEAD(&node_page_list);
+ }
+
+ if (nid == page_to_nid(page)) {
+ list_move(&page->lru, &node_page_list);
+ continue;
+ }
+
+ nr_reclaimed += shrink_page_list(&node_page_list,
+ NODE_DATA(nid),
+ &sc, 0,
+ &dummy_stat, false);
+ while (!list_empty(&node_page_list)) {
+ struct page *page = lru_to_page(&node_page_list);
+
+ list_del(&page->lru);
+ putback_lru_page(page);
+ }
+
+ nid = -1;
+ }
+
+ if (!list_empty(&node_page_list)) {
+ nr_reclaimed += shrink_page_list(&node_page_list,
+ NODE_DATA(nid),
+ &sc, 0,
+ &dummy_stat, false);
+ while (!list_empty(&node_page_list)) {
+ struct page *page = lru_to_page(&node_page_list);
+
+ list_del(&page->lru);
+ putback_lru_page(page);
+ }
+ }
+
+ return nr_reclaimed;
+}
+
/*
* The inactive anon list should be small enough that the VM never has
* to do too much work.
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v3 5/5] mm: factor out pmd young/dirty bit handling and THP split
2019-06-27 11:54 [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Minchan Kim
` (3 preceding siblings ...)
2019-06-27 11:54 ` [PATCH v3 4/5] mm: introduce MADV_PAGEOUT Minchan Kim
@ 2019-06-27 11:54 ` Minchan Kim
2019-07-09 14:10 ` Michal Hocko
2019-06-27 18:06 ` [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Kirill A. Shutemov
2019-07-01 7:38 ` Minchan Kim
6 siblings, 1 reply; 27+ messages in thread
From: Minchan Kim @ 2019-06-27 11:54 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, LKML, linux-api, Michal Hocko, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov, Minchan Kim
Now, there are common part among MADV_COLD|PAGEOUT|FREE to reset
access/dirty bit resetting or split the THP page to handle part
of subpages in the THP page. This patch factor out the common part.
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
include/linux/huge_mm.h | 3 -
mm/huge_memory.c | 74 -------------
mm/madvise.c | 234 +++++++++++++++++++++++-----------------
3 files changed, 135 insertions(+), 176 deletions(-)
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 7cd5c150c21d..2667e1aa3ce5 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -29,9 +29,6 @@ extern struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
unsigned long addr,
pmd_t *pmd,
unsigned int flags);
-extern bool madvise_free_huge_pmd(struct mmu_gather *tlb,
- struct vm_area_struct *vma,
- pmd_t *pmd, unsigned long addr, unsigned long next);
extern int zap_huge_pmd(struct mmu_gather *tlb,
struct vm_area_struct *vma,
pmd_t *pmd, unsigned long addr);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 93f531b63a45..e4b9a06788f3 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1671,80 +1671,6 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf, pmd_t pmd)
return 0;
}
-/*
- * Return true if we do MADV_FREE successfully on entire pmd page.
- * Otherwise, return false.
- */
-bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
- pmd_t *pmd, unsigned long addr, unsigned long next)
-{
- spinlock_t *ptl;
- pmd_t orig_pmd;
- struct page *page;
- struct mm_struct *mm = tlb->mm;
- bool ret = false;
-
- tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
-
- ptl = pmd_trans_huge_lock(pmd, vma);
- if (!ptl)
- goto out_unlocked;
-
- orig_pmd = *pmd;
- if (is_huge_zero_pmd(orig_pmd))
- goto out;
-
- if (unlikely(!pmd_present(orig_pmd))) {
- VM_BUG_ON(thp_migration_supported() &&
- !is_pmd_migration_entry(orig_pmd));
- goto out;
- }
-
- page = pmd_page(orig_pmd);
- /*
- * If other processes are mapping this page, we couldn't discard
- * the page unless they all do MADV_FREE so let's skip the page.
- */
- if (page_mapcount(page) != 1)
- goto out;
-
- if (!trylock_page(page))
- goto out;
-
- /*
- * If user want to discard part-pages of THP, split it so MADV_FREE
- * will deactivate only them.
- */
- if (next - addr != HPAGE_PMD_SIZE) {
- get_page(page);
- spin_unlock(ptl);
- split_huge_page(page);
- unlock_page(page);
- put_page(page);
- goto out_unlocked;
- }
-
- if (PageDirty(page))
- ClearPageDirty(page);
- unlock_page(page);
-
- if (pmd_young(orig_pmd) || pmd_dirty(orig_pmd)) {
- pmdp_invalidate(vma, addr, pmd);
- orig_pmd = pmd_mkold(orig_pmd);
- orig_pmd = pmd_mkclean(orig_pmd);
-
- set_pmd_at(mm, addr, pmd, orig_pmd);
- tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
- }
-
- mark_page_lazyfree(page);
- ret = true;
-out:
- spin_unlock(ptl);
-out_unlocked:
- return ret;
-}
-
static inline void zap_deposited_table(struct mm_struct *mm, pmd_t *pmd)
{
pgtable_t pgtable;
diff --git a/mm/madvise.c b/mm/madvise.c
index ee210473f639..13b06dc8d402 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -310,6 +310,91 @@ static long madvise_willneed(struct vm_area_struct *vma,
return 0;
}
+enum madv_pmdp_reset_t {
+ MADV_PMDP_RESET, /* pmd was reset successfully */
+ MADV_PMDP_SPLIT, /* pmd was split */
+ MADV_PMDP_ERROR,
+};
+
+static enum madv_pmdp_reset_t madvise_pmdp_reset_or_split(struct mm_walk *walk,
+ pmd_t *pmd, spinlock_t *ptl,
+ unsigned long addr, unsigned long end,
+ bool young, bool dirty)
+{
+ pmd_t orig_pmd;
+ unsigned long next;
+ struct page *page;
+ struct mmu_gather *tlb = walk->private;
+ struct mm_struct *mm = walk->mm;
+ struct vm_area_struct *vma = walk->vma;
+ bool reset_young = false;
+ bool reset_dirty = false;
+ enum madv_pmdp_reset_t ret = MADV_PMDP_ERROR;
+
+ orig_pmd = *pmd;
+ if (is_huge_zero_pmd(orig_pmd))
+ return ret;
+
+ if (unlikely(!pmd_present(orig_pmd))) {
+ VM_BUG_ON(thp_migration_supported() &&
+ !is_pmd_migration_entry(orig_pmd));
+ return ret;
+ }
+
+ next = pmd_addr_end(addr, end);
+ page = pmd_page(orig_pmd);
+ if (next - addr != HPAGE_PMD_SIZE) {
+ /*
+ * THP collapsing is not cheap so only split the page is
+ * private to the this process.
+ */
+ if (page_mapcount(page) != 1)
+ return ret;
+ get_page(page);
+ spin_unlock(ptl);
+ lock_page(page);
+ if (!split_huge_page(page))
+ ret = MADV_PMDP_SPLIT;
+ unlock_page(page);
+ put_page(page);
+ return ret;
+ }
+
+ if (young && pmd_young(orig_pmd))
+ reset_young = true;
+ if (dirty && pmd_dirty(orig_pmd))
+ reset_dirty = true;
+
+ /*
+ * Other process could rely on the PG_dirty for data consistency,
+ * not pte_dirty so we could reset PG_dirty only when we are owner
+ * of the page.
+ */
+ if (reset_dirty) {
+ if (page_mapcount(page) != 1)
+ goto out;
+ if (!trylock_page(page))
+ goto out;
+ if (PageDirty(page))
+ ClearPageDirty(page);
+ unlock_page(page);
+ }
+
+ ret = MADV_PMDP_RESET;
+ if (reset_young || reset_dirty) {
+ tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
+ pmdp_invalidate(vma, addr, pmd);
+ if (reset_young)
+ orig_pmd = pmd_mkold(orig_pmd);
+ if (reset_dirty)
+ orig_pmd = pmd_mkclean(orig_pmd);
+ set_pmd_at(mm, addr, pmd, orig_pmd);
+ tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
+ }
+out:
+ return ret;
+}
+
static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
unsigned long end, struct mm_walk *walk)
{
@@ -319,64 +404,31 @@ static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
pte_t *orig_pte, *pte, ptent;
spinlock_t *ptl;
struct page *page;
- unsigned long next;
- next = pmd_addr_end(addr, end);
if (pmd_trans_huge(*pmd)) {
- pmd_t orig_pmd;
-
- tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
ptl = pmd_trans_huge_lock(pmd, vma);
if (!ptl)
return 0;
- orig_pmd = *pmd;
- if (is_huge_zero_pmd(orig_pmd))
- goto huge_unlock;
-
- if (unlikely(!pmd_present(orig_pmd))) {
- VM_BUG_ON(thp_migration_supported() &&
- !is_pmd_migration_entry(orig_pmd));
- goto huge_unlock;
- }
-
- page = pmd_page(orig_pmd);
- if (next - addr != HPAGE_PMD_SIZE) {
- int err;
-
- if (page_mapcount(page) != 1)
- goto huge_unlock;
-
- get_page(page);
+ switch (madvise_pmdp_reset_or_split(walk, pmd, ptl, addr, end,
+ true, false)) {
+ case MADV_PMDP_RESET:
spin_unlock(ptl);
- lock_page(page);
- err = split_huge_page(page);
- unlock_page(page);
- put_page(page);
- if (!err)
- goto regular_page;
- return 0;
- }
-
- if (pmd_young(orig_pmd)) {
- pmdp_invalidate(vma, addr, pmd);
- orig_pmd = pmd_mkold(orig_pmd);
-
- set_pmd_at(mm, addr, pmd, orig_pmd);
- tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
+ page = pmd_page(*pmd);
+ test_and_clear_page_young(page);
+ deactivate_page(page);
+ goto next;
+ case MADV_PMDP_ERROR:
+ spin_unlock(ptl);
+ goto next;
+ case MADV_PMDP_SPLIT:
+ ; /* go through */
}
-
- test_and_clear_page_young(page);
- deactivate_page(page);
-huge_unlock:
- spin_unlock(ptl);
- return 0;
}
if (pmd_trans_unstable(pmd))
return 0;
-regular_page:
tlb_change_page_size(tlb, PAGE_SIZE);
orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
flush_tlb_batched_pending(mm);
@@ -443,6 +495,7 @@ static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
arch_enter_lazy_mmu_mode();
pte_unmap_unlock(orig_pte, ptl);
+next:
cond_resched();
return 0;
@@ -493,70 +546,38 @@ static int madvise_pageout_pte_range(pmd_t *pmd, unsigned long addr,
LIST_HEAD(page_list);
struct page *page;
int isolated = 0;
- unsigned long next;
if (fatal_signal_pending(current))
return -EINTR;
- next = pmd_addr_end(addr, end);
if (pmd_trans_huge(*pmd)) {
- pmd_t orig_pmd;
-
- tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
ptl = pmd_trans_huge_lock(pmd, vma);
if (!ptl)
return 0;
- orig_pmd = *pmd;
- if (is_huge_zero_pmd(orig_pmd))
- goto huge_unlock;
-
- if (unlikely(!pmd_present(orig_pmd))) {
- VM_BUG_ON(thp_migration_supported() &&
- !is_pmd_migration_entry(orig_pmd));
- goto huge_unlock;
- }
-
- page = pmd_page(orig_pmd);
- if (next - addr != HPAGE_PMD_SIZE) {
- int err;
-
- if (page_mapcount(page) != 1)
- goto huge_unlock;
- get_page(page);
+ switch (madvise_pmdp_reset_or_split(walk, pmd, ptl, addr, end,
+ true, false)) {
+ case MADV_PMDP_RESET:
+ page = pmd_page(*pmd);
spin_unlock(ptl);
- lock_page(page);
- err = split_huge_page(page);
- unlock_page(page);
- put_page(page);
- if (!err)
- goto regular_page;
- return 0;
- }
-
- if (isolate_lru_page(page))
- goto huge_unlock;
-
- if (pmd_young(orig_pmd)) {
- pmdp_invalidate(vma, addr, pmd);
- orig_pmd = pmd_mkold(orig_pmd);
-
- set_pmd_at(mm, addr, pmd, orig_pmd);
- tlb_remove_tlb_entry(tlb, pmd, addr);
+ if (isolate_lru_page(page))
+ return 0;
+ ClearPageReferenced(page);
+ test_and_clear_page_young(page);
+ list_add(&page->lru, &page_list);
+ reclaim_pages(&page_list);
+ goto next;
+ case MADV_PMDP_ERROR:
+ spin_unlock(ptl);
+ goto next;
+ case MADV_PMDP_SPLIT:
+ ; /* go through */
}
-
- ClearPageReferenced(page);
- test_and_clear_page_young(page);
- list_add(&page->lru, &page_list);
-huge_unlock:
- spin_unlock(ptl);
- reclaim_pages(&page_list);
- return 0;
}
if (pmd_trans_unstable(pmd))
return 0;
-regular_page:
+
tlb_change_page_size(tlb, PAGE_SIZE);
orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
flush_tlb_batched_pending(mm);
@@ -631,6 +652,7 @@ static int madvise_pageout_pte_range(pmd_t *pmd, unsigned long addr,
arch_leave_lazy_mmu_mode();
pte_unmap_unlock(orig_pte, ptl);
reclaim_pages(&page_list);
+next:
cond_resched();
return 0;
@@ -700,12 +722,26 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
pte_t *orig_pte, *pte, ptent;
struct page *page;
int nr_swap = 0;
- unsigned long next;
- next = pmd_addr_end(addr, end);
- if (pmd_trans_huge(*pmd))
- if (madvise_free_huge_pmd(tlb, vma, pmd, addr, next))
+ if (pmd_trans_huge(*pmd)) {
+ ptl = pmd_trans_huge_lock(pmd, vma);
+ if (!ptl)
+ return 0;
+
+ switch (madvise_pmdp_reset_or_split(walk, pmd, ptl, addr, end,
+ true, true)) {
+ case MADV_PMDP_RESET:
+ page = pmd_page(*pmd);
+ spin_unlock(ptl);
+ mark_page_lazyfree(page);
goto next;
+ case MADV_PMDP_ERROR:
+ spin_unlock(ptl);
+ goto next;
+ case MADV_PMDP_SPLIT:
+ ; /* go through */
+ }
+ }
if (pmd_trans_unstable(pmd))
return 0;
@@ -817,8 +853,8 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
}
arch_leave_lazy_mmu_mode();
pte_unmap_unlock(orig_pte, ptl);
- cond_resched();
next:
+ cond_resched();
return 0;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v3 1/5] mm: introduce MADV_COLD
2019-06-27 11:54 ` [PATCH v3 1/5] mm: introduce MADV_COLD Minchan Kim
@ 2019-06-27 13:13 ` Dave Hansen
2019-06-27 14:02 ` Michal Hocko
2019-06-27 23:46 ` Minchan Kim
0 siblings, 2 replies; 27+ messages in thread
From: Dave Hansen @ 2019-06-27 13:13 UTC (permalink / raw)
To: Minchan Kim, Andrew Morton
Cc: linux-mm, LKML, linux-api, Michal Hocko, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Kirill A . Shutemov
On 6/27/19 4:54 AM, Minchan Kim wrote:
> This patch introduces the new MADV_COLD hint to madvise(2) syscall.
> MADV_COLD can be used by a process to mark a memory range as not expected
> to be used in the near future. The hint can help kernel in deciding which
> pages to evict early during memory pressure.
>
> It works for every LRU pages like MADV_[DONTNEED|FREE]. IOW, It moves
>
> active file page -> inactive file LRU
> active anon page -> inacdtive anon LRU
Is the LRU behavior part of the interface or the implementation?
I ask because we've got something in between tossing something down the
LRU and swapping it: page migration. Specifically, on a system with
slower memory media (like persistent memory) we just migrate a page
instead of discarding it at reclaim:
> https://lore.kernel.org/linux-mm/20190321200157.29678-4-keith.busch@intel.com/
So let's say I have a page I want to evict from DRAM to the next slower
tier of memory. Do I use MADV_COLD or MADV_PAGEOUT? If the LRU
behavior is part of the interface itself, then MADV_COLD doesn't work.
Do you think we'll need a third MADV_ flag for our automatic migration
behavior? MADV_REALLYCOLD? MADV_MIGRATEOUT?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 1/5] mm: introduce MADV_COLD
2019-06-27 13:13 ` Dave Hansen
@ 2019-06-27 14:02 ` Michal Hocko
2019-06-27 14:36 ` Dave Hansen
2019-06-27 23:46 ` Minchan Kim
1 sibling, 1 reply; 27+ messages in thread
From: Michal Hocko @ 2019-06-27 14:02 UTC (permalink / raw)
To: Dave Hansen
Cc: Minchan Kim, Andrew Morton, linux-mm, LKML, linux-api,
Johannes Weiner, Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Kirill A . Shutemov
On Thu 27-06-19 06:13:36, Dave Hansen wrote:
> On 6/27/19 4:54 AM, Minchan Kim wrote:
> > This patch introduces the new MADV_COLD hint to madvise(2) syscall.
> > MADV_COLD can be used by a process to mark a memory range as not expected
> > to be used in the near future. The hint can help kernel in deciding which
> > pages to evict early during memory pressure.
> >
> > It works for every LRU pages like MADV_[DONTNEED|FREE]. IOW, It moves
> >
> > active file page -> inactive file LRU
> > active anon page -> inacdtive anon LRU
>
> Is the LRU behavior part of the interface or the implementation?
>
> I ask because we've got something in between tossing something down the
> LRU and swapping it: page migration. Specifically, on a system with
> slower memory media (like persistent memory) we just migrate a page
> instead of discarding it at reclaim:
But we already do have interfaces for migrating the memory
(move_pages(2)). Why should this interface duplicate that interface?
I believe the only purpose of these two new madvise modes is to provide
a non-destructive MADV_{DONTNEED,FREE} alteternatives. In other words,
pageout vs. age interface.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 1/5] mm: introduce MADV_COLD
2019-06-27 14:02 ` Michal Hocko
@ 2019-06-27 14:36 ` Dave Hansen
2019-06-27 14:53 ` Michal Hocko
0 siblings, 1 reply; 27+ messages in thread
From: Dave Hansen @ 2019-06-27 14:36 UTC (permalink / raw)
To: Michal Hocko
Cc: Minchan Kim, Andrew Morton, linux-mm, LKML, linux-api,
Johannes Weiner, Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Kirill A . Shutemov
On 6/27/19 7:02 AM, Michal Hocko wrote:
>> Is the LRU behavior part of the interface or the implementation?
>>
>> I ask because we've got something in between tossing something down the
>> LRU and swapping it: page migration. Specifically, on a system with
>> slower memory media (like persistent memory) we just migrate a page
>> instead of discarding it at reclaim:
> But we already do have interfaces for migrating the memory
> (move_pages(2)). Why should this interface duplicate that interface?
> I believe the only purpose of these two new madvise modes is to provide
> a non-destructive MADV_{DONTNEED,FREE} alteternatives. In other words,
> pageout vs. age interface.
The existing interface's problem for this case is that it has to know
exact locations where the memory is and where it should go. For
instance, if you have two sockets, you very likely want to demote DRAM
to the persistent memory DIMM sitting next to it and not go
cross-socket. To do _that_, you need to know where the existing
allocation lies so you can find the appropriate destination node.
That's not a problem for existing NUMA-enlightened apps, but it is for
everything else.
For MADV_COLD, if we defined it like this, I think we could use it for
both purposes (demotion and LRU movement):
Pages in the specified regions will be treated as less-recently-
accessed compared to pages in the system with similar access
frequencies. In contrast to MADV_DONTNEED, the contents of the
region are preserved.
It would be nice not to talk about reclaim at all since we're not
promising reclaim per se.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 1/5] mm: introduce MADV_COLD
2019-06-27 14:36 ` Dave Hansen
@ 2019-06-27 14:53 ` Michal Hocko
2019-06-27 23:56 ` Minchan Kim
0 siblings, 1 reply; 27+ messages in thread
From: Michal Hocko @ 2019-06-27 14:53 UTC (permalink / raw)
To: Dave Hansen
Cc: Minchan Kim, Andrew Morton, linux-mm, LKML, linux-api,
Johannes Weiner, Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Kirill A . Shutemov
On Thu 27-06-19 07:36:50, Dave Hansen wrote:
[...]
> For MADV_COLD, if we defined it like this, I think we could use it for
> both purposes (demotion and LRU movement):
>
> Pages in the specified regions will be treated as less-recently-
> accessed compared to pages in the system with similar access
> frequencies. In contrast to MADV_DONTNEED, the contents of the
you meant s@MADV_DONTNEED@MADV_FREE@ I suppose
> region are preserved.
>
> It would be nice not to talk about reclaim at all since we're not
> promising reclaim per se.
Well, I guess this is just an implementation detail. MADV_FREE is really
only about aging. It is up to the kernel what to do during the reclaim
and the advice doesn't and shouldn't make any difference here.
Now MADV_PAGEOUT would be more tricky in that direction because it
defines an immediate action to page out the range. I do understand your
argument about NUMA unaware applications which might want to get
something like MADV_DEMOTE which would move a page to a secondary memory
(whatever that is) but I think this is asking for its own madvise.
MADV_PAGEOUT has a quite simple semnatic - move to the backing storage -
and I would rather not make it more complex.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT
2019-06-27 11:54 [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Minchan Kim
` (4 preceding siblings ...)
2019-06-27 11:54 ` [PATCH v3 5/5] mm: factor out pmd young/dirty bit handling and THP split Minchan Kim
@ 2019-06-27 18:06 ` Kirill A. Shutemov
2019-06-27 23:12 ` Minchan Kim
2019-07-01 7:38 ` Minchan Kim
6 siblings, 1 reply; 27+ messages in thread
From: Kirill A. Shutemov @ 2019-06-27 18:06 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, linux-mm, LKML, linux-api, Michal Hocko,
Johannes Weiner, Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Thu, Jun 27, 2019 at 08:54:00PM +0900, Minchan Kim wrote:
> - Problem
>
> Naturally, cached apps were dominant consumers of memory on the system.
> However, they were not significant consumers of swap even though they are
> good candidate for swap. Under investigation, swapping out only begins
> once the low zone watermark is hit and kswapd wakes up, but the overall
> allocation rate in the system might trip lmkd thresholds and cause a cached
> process to be killed(we measured performance swapping out vs. zapping the
> memory by killing a process. Unsurprisingly, zapping is 10x times faster
> even though we use zram which is much faster than real storage) so kill
> from lmkd will often satisfy the high zone watermark, resulting in very
> few pages actually being moved to swap.
Maybe we should look if we do The Right Thing™ at system-wide level before
introducing new API? How changing swappiness affects your workloads? What
is swappiness value in your setup?
--
Kirill A. Shutemov
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT
2019-06-27 18:06 ` [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Kirill A. Shutemov
@ 2019-06-27 23:12 ` Minchan Kim
0 siblings, 0 replies; 27+ messages in thread
From: Minchan Kim @ 2019-06-27 23:12 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Andrew Morton, linux-mm, LKML, linux-api, Michal Hocko,
Johannes Weiner, Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Thu, Jun 27, 2019 at 09:06:01PM +0300, Kirill A. Shutemov wrote:
> On Thu, Jun 27, 2019 at 08:54:00PM +0900, Minchan Kim wrote:
> > - Problem
> >
> > Naturally, cached apps were dominant consumers of memory on the system.
> > However, they were not significant consumers of swap even though they are
> > good candidate for swap. Under investigation, swapping out only begins
> > once the low zone watermark is hit and kswapd wakes up, but the overall
> > allocation rate in the system might trip lmkd thresholds and cause a cached
> > process to be killed(we measured performance swapping out vs. zapping the
> > memory by killing a process. Unsurprisingly, zapping is 10x times faster
> > even though we use zram which is much faster than real storage) so kill
> > from lmkd will often satisfy the high zone watermark, resulting in very
> > few pages actually being moved to swap.
>
> Maybe we should look if we do The Right Thing™ at system-wide level before
> introducing new API? How changing swappiness affects your workloads? What
> is swappiness value in your setup?
It was 100. Even, I tried 150 and 200 with simple hack of swappiness.
However, it caused too excessive swpout.
Anyway, systen-level tune is generally good but if process has hint, that
should work better and that's why advise API is.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 1/5] mm: introduce MADV_COLD
2019-06-27 13:13 ` Dave Hansen
2019-06-27 14:02 ` Michal Hocko
@ 2019-06-27 23:46 ` Minchan Kim
1 sibling, 0 replies; 27+ messages in thread
From: Minchan Kim @ 2019-06-27 23:46 UTC (permalink / raw)
To: Dave Hansen
Cc: Andrew Morton, linux-mm, LKML, linux-api, Michal Hocko,
Johannes Weiner, Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Kirill A . Shutemov
On Thu, Jun 27, 2019 at 06:13:36AM -0700, Dave Hansen wrote:
> On 6/27/19 4:54 AM, Minchan Kim wrote:
> > This patch introduces the new MADV_COLD hint to madvise(2) syscall.
> > MADV_COLD can be used by a process to mark a memory range as not expected
> > to be used in the near future. The hint can help kernel in deciding which
> > pages to evict early during memory pressure.
> >
> > It works for every LRU pages like MADV_[DONTNEED|FREE]. IOW, It moves
> >
> > active file page -> inactive file LRU
> > active anon page -> inacdtive anon LRU
>
> Is the LRU behavior part of the interface or the implementation?
It's a just implementation. What user should expect with this API is they just
informs to the kernel "this memory in the regions wouldn't access in the near
future" so how kernel will handle memory in there is up to the kernel.
>
> I ask because we've got something in between tossing something down the
> LRU and swapping it: page migration. Specifically, on a system with
> slower memory media (like persistent memory) we just migrate a page
> instead of discarding it at reclaim:
>
> > https://lore.kernel.org/linux-mm/20190321200157.29678-4-keith.busch@intel.com/
>
> So let's say I have a page I want to evict from DRAM to the next slower
> tier of memory. Do I use MADV_COLD or MADV_PAGEOUT? If the LRU
> behavior is part of the interface itself, then MADV_COLD doesn't work.
IMHO, if it's one of storage in the memory hierarchy, that shouldn't be transparent
for the user? What I meant is VM moves inactive pages to the persistent memory
before the reclaiming. IOW, VM would have one more level LRU or extened inactive
LRU to cover the persistent memory.
>
> Do you think we'll need a third MADV_ flag for our automatic migration
> behavior? MADV_REALLYCOLD? MADV_MIGRATEOUT?
I believe it depends on how we abstract the persistent memory of cache hierarchy.
If we abstract it as diffrent storage with DRAM, manybe, that should be part of
other syscall like like move_pages.
If we abstract it as part of DRAM, that should be part of additional LRU
or extended inactive LRU.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 1/5] mm: introduce MADV_COLD
2019-06-27 14:53 ` Michal Hocko
@ 2019-06-27 23:56 ` Minchan Kim
2019-07-01 7:35 ` Minchan Kim
0 siblings, 1 reply; 27+ messages in thread
From: Minchan Kim @ 2019-06-27 23:56 UTC (permalink / raw)
To: Michal Hocko
Cc: Dave Hansen, Andrew Morton, linux-mm, LKML, linux-api,
Johannes Weiner, Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Kirill A . Shutemov
On Thu, Jun 27, 2019 at 04:53:02PM +0200, Michal Hocko wrote:
> On Thu 27-06-19 07:36:50, Dave Hansen wrote:
> [...]
> > For MADV_COLD, if we defined it like this, I think we could use it for
> > both purposes (demotion and LRU movement):
> >
> > Pages in the specified regions will be treated as less-recently-
> > accessed compared to pages in the system with similar access
> > frequencies. In contrast to MADV_DONTNEED, the contents of the
>
> you meant s@MADV_DONTNEED@MADV_FREE@ I suppose
Right, MADV_FREE is more proper because it's aging related.
>
> > region are preserved.
> >
> > It would be nice not to talk about reclaim at all since we're not
> > promising reclaim per se.
Your suggestion doesn't expose any implementation detail and could meet your
needs later. I'm okay. I will change it if others are not against of it.
Thanks, Dave.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 1/5] mm: introduce MADV_COLD
2019-06-27 23:56 ` Minchan Kim
@ 2019-07-01 7:35 ` Minchan Kim
2019-07-09 9:19 ` Michal Hocko
0 siblings, 1 reply; 27+ messages in thread
From: Minchan Kim @ 2019-07-01 7:35 UTC (permalink / raw)
To: Michal Hocko
Cc: Dave Hansen, Andrew Morton, linux-mm, LKML, linux-api,
Johannes Weiner, Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Kirill A . Shutemov
On Fri, Jun 28, 2019 at 08:56:18AM +0900, Minchan Kim wrote:
> On Thu, Jun 27, 2019 at 04:53:02PM +0200, Michal Hocko wrote:
> > On Thu 27-06-19 07:36:50, Dave Hansen wrote:
> > [...]
> > > For MADV_COLD, if we defined it like this, I think we could use it for
> > > both purposes (demotion and LRU movement):
> > >
> > > Pages in the specified regions will be treated as less-recently-
> > > accessed compared to pages in the system with similar access
> > > frequencies. In contrast to MADV_DONTNEED, the contents of the
> >
> > you meant s@MADV_DONTNEED@MADV_FREE@ I suppose
>
> Right, MADV_FREE is more proper because it's aging related.
>
> >
> > > region are preserved.
> > >
> > > It would be nice not to talk about reclaim at all since we're not
> > > promising reclaim per se.
>
> Your suggestion doesn't expose any implementation detail and could meet your
> needs later. I'm okay. I will change it if others are not against of it.
>
> Thanks, Dave.
From 39df9f94e6204b8893f3f3feb692745657392657 Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan@kernel.org>
Date: Fri, 24 May 2019 13:47:54 +0900
Subject: [PATCH v3 1/5] mm: introduce MADV_COLD
When a process expects no accesses to a certain memory range, it could
give a hint to kernel that the pages can be reclaimed when memory pressure
happens but data should be preserved for future use. This could reduce
workingset eviction so it ends up increasing performance.
This patch introduces the new MADV_COLD hint to madvise(2) syscall.
MADV_COLD can be used by a process to mark a memory range as not expected
to be used in the near future. The hint can help kernel in deciding which
pages to evict early during memory pressure.
It works for every LRU pages like MADV_[DONTNEED|FREE]. IOW, It moves
active file page -> inactive file LRU
active anon page -> inacdtive anon LRU
Unlike MADV_FREE, it doesn't move active anonymous pages to inactive
file LRU's head because MADV_COLD is a little bit different symantic.
MADV_FREE means it's okay to discard when the memory pressure because
the content of the page is *garbage* so freeing such pages is almost zero
overhead since we don't need to swap out and access afterward causes just
minor fault. Thus, it would make sense to put those freeable pages in
inactive file LRU to compete other used-once pages. It makes sense for
implmentaion point of view, too because it's not swapbacked memory any
longer until it would be re-dirtied. Even, it could give a bonus to make
them be reclaimed on swapless system. However, MADV_COLD doesn't mean
garbage so reclaiming them requires swap-out/in in the end so it's bigger
cost. Since we have designed VM LRU aging based on cost-model, anonymous
cold pages would be better to position inactive anon's LRU list, not file
LRU. Furthermore, it would help to avoid unnecessary scanning if system
doesn't have a swap device. Let's start simpler way without adding
complexity at this moment. However, keep in mind, too that it's a caveat
that workloads with a lot of pages cache are likely to ignore MADV_COLD
on anonymous memory because we rarely age anonymous LRU lists.
* man-page material
MADV_COLD (since Linux x.x)
Pages in the specified regions will be treated as less-recently-accessed
compared to pages in the system with similar access frequencies.
In contrast to MADV_FREE, the contents of the region are preserved
regardless of subsequent writes to pages.
MADV_COLD cannot be applied to locked pages, Huge TLB pages, or VM_PFNMAP
pages.
* v2
* add up the warn with lots of page cache workload - mhocko
* add man page stuff - dave
* v1
* remove page_mapcount filter - hannes, mhocko
* remove idle page handling - joelaf
* RFCv2
* add more description - mhocko
* RFCv1
* renaming from MADV_COOL to MADV_COLD - hannes
* internal review
* use clear_page_youn in deactivate_page - joelaf
* Revise the description - surenb
* Renaming from MADV_WARM to MADV_COOL - surenb
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
include/linux/swap.h | 1 +
include/uapi/asm-generic/mman-common.h | 1 +
mm/internal.h | 2 +-
mm/madvise.c | 180 ++++++++++++++++++++++++-
mm/oom_kill.c | 2 +-
mm/swap.c | 42 ++++++
6 files changed, 224 insertions(+), 4 deletions(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index de2c67a33b7e..0ce997edb8bb 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -340,6 +340,7 @@ extern void lru_add_drain_cpu(int cpu);
extern void lru_add_drain_all(void);
extern void rotate_reclaimable_page(struct page *page);
extern void deactivate_file_page(struct page *page);
+extern void deactivate_page(struct page *page);
extern void mark_page_lazyfree(struct page *page);
extern void swap_setup(void);
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index ef4623f03156..d7b4231eea63 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -47,6 +47,7 @@
#define MADV_SEQUENTIAL 2 /* expect sequential page references */
#define MADV_WILLNEED 3 /* will need these pages */
#define MADV_DONTNEED 4 /* don't need these pages */
+#define MADV_COLD 5 /* deactivatie these pages */
/* common parameters: try to keep these consistent across architectures */
#define MADV_FREE 8 /* free pages only if memory pressure */
diff --git a/mm/internal.h b/mm/internal.h
index f53a14d67538..c61b215ff265 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -39,7 +39,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf);
void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
unsigned long floor, unsigned long ceiling);
-static inline bool can_madv_dontneed_vma(struct vm_area_struct *vma)
+static inline bool can_madv_lru_vma(struct vm_area_struct *vma)
{
return !(vma->vm_flags & (VM_LOCKED|VM_HUGETLB|VM_PFNMAP));
}
diff --git a/mm/madvise.c b/mm/madvise.c
index 628022e674a7..7abb8e54bc7a 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -40,6 +40,7 @@ static int madvise_need_mmap_write(int behavior)
case MADV_REMOVE:
case MADV_WILLNEED:
case MADV_DONTNEED:
+ case MADV_COLD:
case MADV_FREE:
return 0;
default:
@@ -307,6 +308,178 @@ static long madvise_willneed(struct vm_area_struct *vma,
return 0;
}
+static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
+ unsigned long end, struct mm_walk *walk)
+{
+ struct mmu_gather *tlb = walk->private;
+ struct mm_struct *mm = tlb->mm;
+ struct vm_area_struct *vma = walk->vma;
+ pte_t *orig_pte, *pte, ptent;
+ spinlock_t *ptl;
+ struct page *page;
+ unsigned long next;
+
+ next = pmd_addr_end(addr, end);
+ if (pmd_trans_huge(*pmd)) {
+ pmd_t orig_pmd;
+
+ tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
+ ptl = pmd_trans_huge_lock(pmd, vma);
+ if (!ptl)
+ return 0;
+
+ orig_pmd = *pmd;
+ if (is_huge_zero_pmd(orig_pmd))
+ goto huge_unlock;
+
+ if (unlikely(!pmd_present(orig_pmd))) {
+ VM_BUG_ON(thp_migration_supported() &&
+ !is_pmd_migration_entry(orig_pmd));
+ goto huge_unlock;
+ }
+
+ page = pmd_page(orig_pmd);
+ if (next - addr != HPAGE_PMD_SIZE) {
+ int err;
+
+ if (page_mapcount(page) != 1)
+ goto huge_unlock;
+
+ get_page(page);
+ spin_unlock(ptl);
+ lock_page(page);
+ err = split_huge_page(page);
+ unlock_page(page);
+ put_page(page);
+ if (!err)
+ goto regular_page;
+ return 0;
+ }
+
+ if (pmd_young(orig_pmd)) {
+ pmdp_invalidate(vma, addr, pmd);
+ orig_pmd = pmd_mkold(orig_pmd);
+
+ set_pmd_at(mm, addr, pmd, orig_pmd);
+ tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
+ }
+
+ test_and_clear_page_young(page);
+ deactivate_page(page);
+huge_unlock:
+ spin_unlock(ptl);
+ return 0;
+ }
+
+ if (pmd_trans_unstable(pmd))
+ return 0;
+
+regular_page:
+ tlb_change_page_size(tlb, PAGE_SIZE);
+ orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
+ flush_tlb_batched_pending(mm);
+ arch_enter_lazy_mmu_mode();
+ for (; addr < end; pte++, addr += PAGE_SIZE) {
+ ptent = *pte;
+
+ if (pte_none(ptent))
+ continue;
+
+ if (!pte_present(ptent))
+ continue;
+
+ page = vm_normal_page(vma, addr, ptent);
+ if (!page)
+ continue;
+
+ /*
+ * Creating a THP page is expensive so split it only if we
+ * are sure it's worth. Split it if we are only owner.
+ */
+ if (PageTransCompound(page)) {
+ if (page_mapcount(page) != 1)
+ break;
+ get_page(page);
+ if (!trylock_page(page)) {
+ put_page(page);
+ break;
+ }
+ pte_unmap_unlock(orig_pte, ptl);
+ if (split_huge_page(page)) {
+ unlock_page(page);
+ put_page(page);
+ pte_offset_map_lock(mm, pmd, addr, &ptl);
+ break;
+ }
+ unlock_page(page);
+ put_page(page);
+ pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
+ pte--;
+ addr -= PAGE_SIZE;
+ continue;
+ }
+
+ VM_BUG_ON_PAGE(PageTransCompound(page), page);
+
+ if (pte_young(ptent)) {
+ ptent = ptep_get_and_clear_full(mm, addr, pte,
+ tlb->fullmm);
+ ptent = pte_mkold(ptent);
+ set_pte_at(mm, addr, pte, ptent);
+ tlb_remove_tlb_entry(tlb, pte, addr);
+ }
+
+ /*
+ * We are deactivating a page for accelerating reclaiming.
+ * VM couldn't reclaim the page unless we clear PG_young.
+ * As a side effect, it makes confuse idle-page tracking
+ * because they will miss recent referenced history.
+ */
+ test_and_clear_page_young(page);
+ deactivate_page(page);
+ }
+
+ arch_enter_lazy_mmu_mode();
+ pte_unmap_unlock(orig_pte, ptl);
+ cond_resched();
+
+ return 0;
+}
+
+static void madvise_cold_page_range(struct mmu_gather *tlb,
+ struct vm_area_struct *vma,
+ unsigned long addr, unsigned long end)
+{
+ struct mm_walk cold_walk = {
+ .pmd_entry = madvise_cold_pte_range,
+ .mm = vma->vm_mm,
+ .private = tlb,
+ };
+
+ tlb_start_vma(tlb, vma);
+ walk_page_range(addr, end, &cold_walk);
+ tlb_end_vma(tlb, vma);
+}
+
+static long madvise_cold(struct vm_area_struct *vma,
+ struct vm_area_struct **prev,
+ unsigned long start_addr, unsigned long end_addr)
+{
+ struct mm_struct *mm = vma->vm_mm;
+ struct mmu_gather tlb;
+
+ *prev = vma;
+ if (!can_madv_lru_vma(vma))
+ return -EINVAL;
+
+ lru_add_drain();
+ tlb_gather_mmu(&tlb, mm, start_addr, end_addr);
+ madvise_cold_page_range(&tlb, vma, start_addr, end_addr);
+ tlb_finish_mmu(&tlb, start_addr, end_addr);
+
+ return 0;
+}
+
static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
unsigned long end, struct mm_walk *walk)
@@ -519,7 +692,7 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
int behavior)
{
*prev = vma;
- if (!can_madv_dontneed_vma(vma))
+ if (!can_madv_lru_vma(vma))
return -EINVAL;
if (!userfaultfd_remove(vma, start, end)) {
@@ -541,7 +714,7 @@ static long madvise_dontneed_free(struct vm_area_struct *vma,
*/
return -ENOMEM;
}
- if (!can_madv_dontneed_vma(vma))
+ if (!can_madv_lru_vma(vma))
return -EINVAL;
if (end > vma->vm_end) {
/*
@@ -695,6 +868,8 @@ madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
return madvise_remove(vma, prev, start, end);
case MADV_WILLNEED:
return madvise_willneed(vma, prev, start, end);
+ case MADV_COLD:
+ return madvise_cold(vma, prev, start, end);
case MADV_FREE:
case MADV_DONTNEED:
return madvise_dontneed_free(vma, prev, start, end, behavior);
@@ -716,6 +891,7 @@ madvise_behavior_valid(int behavior)
case MADV_WILLNEED:
case MADV_DONTNEED:
case MADV_FREE:
+ case MADV_COLD:
#ifdef CONFIG_KSM
case MADV_MERGEABLE:
case MADV_UNMERGEABLE:
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 6de5c354d6ca..2140a6f8db63 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -523,7 +523,7 @@ bool __oom_reap_task_mm(struct mm_struct *mm)
set_bit(MMF_UNSTABLE, &mm->flags);
for (vma = mm->mmap ; vma; vma = vma->vm_next) {
- if (!can_madv_dontneed_vma(vma))
+ if (!can_madv_lru_vma(vma))
continue;
/*
diff --git a/mm/swap.c b/mm/swap.c
index 607c48229a1d..a91859d061f3 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -47,6 +47,7 @@ int page_cluster;
static DEFINE_PER_CPU(struct pagevec, lru_add_pvec);
static DEFINE_PER_CPU(struct pagevec, lru_rotate_pvecs);
static DEFINE_PER_CPU(struct pagevec, lru_deactivate_file_pvecs);
+static DEFINE_PER_CPU(struct pagevec, lru_deactivate_pvecs);
static DEFINE_PER_CPU(struct pagevec, lru_lazyfree_pvecs);
#ifdef CONFIG_SMP
static DEFINE_PER_CPU(struct pagevec, activate_page_pvecs);
@@ -538,6 +539,22 @@ static void lru_deactivate_file_fn(struct page *page, struct lruvec *lruvec,
update_page_reclaim_stat(lruvec, file, 0);
}
+static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec,
+ void *arg)
+{
+ if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
+ int file = page_is_file_cache(page);
+ int lru = page_lru_base_type(page);
+
+ del_page_from_lru_list(page, lruvec, lru + LRU_ACTIVE);
+ ClearPageActive(page);
+ ClearPageReferenced(page);
+ add_page_to_lru_list(page, lruvec, lru);
+
+ __count_vm_events(PGDEACTIVATE, hpage_nr_pages(page));
+ update_page_reclaim_stat(lruvec, file, 0);
+ }
+}
static void lru_lazyfree_fn(struct page *page, struct lruvec *lruvec,
void *arg)
@@ -590,6 +607,10 @@ void lru_add_drain_cpu(int cpu)
if (pagevec_count(pvec))
pagevec_lru_move_fn(pvec, lru_deactivate_file_fn, NULL);
+ pvec = &per_cpu(lru_deactivate_pvecs, cpu);
+ if (pagevec_count(pvec))
+ pagevec_lru_move_fn(pvec, lru_deactivate_fn, NULL);
+
pvec = &per_cpu(lru_lazyfree_pvecs, cpu);
if (pagevec_count(pvec))
pagevec_lru_move_fn(pvec, lru_lazyfree_fn, NULL);
@@ -623,6 +644,26 @@ void deactivate_file_page(struct page *page)
}
}
+/*
+ * deactivate_page - deactivate a page
+ * @page: page to deactivate
+ *
+ * deactivate_page() moves @page to the inactive list if @page was on the active
+ * list and was not an unevictable page. This is done to accelerate the reclaim
+ * of @page.
+ */
+void deactivate_page(struct page *page)
+{
+ if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
+ struct pagevec *pvec = &get_cpu_var(lru_deactivate_pvecs);
+
+ get_page(page);
+ if (!pagevec_add(pvec, page) || PageCompound(page))
+ pagevec_lru_move_fn(pvec, lru_deactivate_fn, NULL);
+ put_cpu_var(lru_deactivate_pvecs);
+ }
+}
+
/**
* mark_page_lazyfree - make an anon page lazyfree
* @page: page to deactivate
@@ -687,6 +728,7 @@ void lru_add_drain_all(void)
if (pagevec_count(&per_cpu(lru_add_pvec, cpu)) ||
pagevec_count(&per_cpu(lru_rotate_pvecs, cpu)) ||
pagevec_count(&per_cpu(lru_deactivate_file_pvecs, cpu)) ||
+ pagevec_count(&per_cpu(lru_deactivate_pvecs, cpu)) ||
pagevec_count(&per_cpu(lru_lazyfree_pvecs, cpu)) ||
need_activate_page_drain(cpu)) {
INIT_WORK(work, lru_add_drain_per_cpu);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT
2019-06-27 11:54 [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Minchan Kim
` (5 preceding siblings ...)
2019-06-27 18:06 ` [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Kirill A. Shutemov
@ 2019-07-01 7:38 ` Minchan Kim
2019-07-01 10:22 ` Michal Hocko
6 siblings, 1 reply; 27+ messages in thread
From: Minchan Kim @ 2019-07-01 7:38 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, LKML, linux-api, Michal Hocko, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
Hi Folks,
Do you guys have comments? I think it would be long enough to be
pending. If there is no further comments, I want to ask to merge.
Thanks.
On Thu, Jun 27, 2019 at 08:54:00PM +0900, Minchan Kim wrote:
> This patch is part of previous series:
> https://lore.kernel.org/lkml/20190531064313.193437-1-minchan@kernel.org/T/#u
> Originally, it was created for external madvise hinting feature.
>
> https://lkml.org/lkml/2019/5/31/463
> Michal wanted to separte the discussion from external hinting interface
> so this patchset includes only first part of my entire patchset
>
> - introduce MADV_COLD and MADV_PAGEOUT hint to madvise.
>
> However, I keep entire description for others for easier understanding
> why this kinds of hint was born.
>
> Thanks.
>
> This patchset is against on next-20190530.
>
> Below is description of previous entire patchset.
> ================= &< =====================
>
> - Background
>
> The Android terminology used for forking a new process and starting an app
> from scratch is a cold start, while resuming an existing app is a hot start.
> While we continually try to improve the performance of cold starts, hot
> starts will always be significantly less power hungry as well as faster so
> we are trying to make hot start more likely than cold start.
>
> To increase hot start, Android userspace manages the order that apps should
> be killed in a process called ActivityManagerService. ActivityManagerService
> tracks every Android app or service that the user could be interacting with
> at any time and translates that into a ranked list for lmkd(low memory
> killer daemon). They are likely to be killed by lmkd if the system has to
> reclaim memory. In that sense they are similar to entries in any other cache.
> Those apps are kept alive for opportunistic performance improvements but
> those performance improvements will vary based on the memory requirements of
> individual workloads.
>
> - Problem
>
> Naturally, cached apps were dominant consumers of memory on the system.
> However, they were not significant consumers of swap even though they are
> good candidate for swap. Under investigation, swapping out only begins
> once the low zone watermark is hit and kswapd wakes up, but the overall
> allocation rate in the system might trip lmkd thresholds and cause a cached
> process to be killed(we measured performance swapping out vs. zapping the
> memory by killing a process. Unsurprisingly, zapping is 10x times faster
> even though we use zram which is much faster than real storage) so kill
> from lmkd will often satisfy the high zone watermark, resulting in very
> few pages actually being moved to swap.
>
> - Approach
>
> The approach we chose was to use a new interface to allow userspace to
> proactively reclaim entire processes by leveraging platform information.
> This allowed us to bypass the inaccuracy of the kernel’s LRUs for pages
> that are known to be cold from userspace and to avoid races with lmkd
> by reclaiming apps as soon as they entered the cached state. Additionally,
> it could provide many chances for platform to use much information to
> optimize memory efficiency.
>
> To achieve the goal, the patchset introduce two new options for madvise.
> One is MADV_COLD which will deactivate activated pages and the other is
> MADV_PAGEOUT which will reclaim private pages instantly. These new options
> complement MADV_DONTNEED and MADV_FREE by adding non-destructive ways to
> gain some free memory space. MADV_PAGEOUT is similar to MADV_DONTNEED in a way
> that it hints the kernel that memory region is not currently needed and
> should be reclaimed immediately; MADV_COLD is similar to MADV_FREE in a way
> that it hints the kernel that memory region is not currently needed and
> should be reclaimed when memory pressure rises.
>
> Minchan Kim (5):
> mm: introduce MADV_COLD
> mm: change PAGEREF_RECLAIM_CLEAN with PAGE_REFRECLAIM
> mm: account nr_isolated_xxx in [isolate|putback]_lru_page
> mm: introduce MADV_PAGEOUT
> mm: factor out pmd young/dirty bit handling and THP split
>
> include/linux/huge_mm.h | 3 -
> include/linux/swap.h | 2 +
> include/uapi/asm-generic/mman-common.h | 2 +
> mm/compaction.c | 2 -
> mm/gup.c | 7 +-
> mm/huge_memory.c | 74 -----
> mm/internal.h | 2 +-
> mm/khugepaged.c | 3 -
> mm/madvise.c | 438 ++++++++++++++++++++++++-
> mm/memory-failure.c | 3 -
> mm/memory_hotplug.c | 4 -
> mm/mempolicy.c | 6 +-
> mm/migrate.c | 37 +--
> mm/oom_kill.c | 2 +-
> mm/swap.c | 42 +++
> mm/vmscan.c | 86 ++++-
> 16 files changed, 566 insertions(+), 147 deletions(-)
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT
2019-07-01 7:38 ` Minchan Kim
@ 2019-07-01 10:22 ` Michal Hocko
0 siblings, 0 replies; 27+ messages in thread
From: Michal Hocko @ 2019-07-01 10:22 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Mon 01-07-19 16:38:48, Minchan Kim wrote:
>
> Hi Folks,
>
> Do you guys have comments? I think it would be long enough to be
> pending. If there is no further comments, I want to ask to merge.
This is definitely on my todo list for this week. But please be patient.
It's been _one_ work day since you posted this last version so I do not
think this is stalling for too long. Sure the current version is
probably not too much different from the previous but I didn't get to
review it in the depth yet. All the code duplication doesn't make it
much easier but I understand your reasoning that sharing more code is
not really straightforward.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 1/5] mm: introduce MADV_COLD
2019-07-01 7:35 ` Minchan Kim
@ 2019-07-09 9:19 ` Michal Hocko
0 siblings, 0 replies; 27+ messages in thread
From: Michal Hocko @ 2019-07-09 9:19 UTC (permalink / raw)
To: Minchan Kim
Cc: Dave Hansen, Andrew Morton, linux-mm, LKML, linux-api,
Johannes Weiner, Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Kirill A . Shutemov
On Mon 01-07-19 16:35:00, Minchan Kim wrote:
> >From 39df9f94e6204b8893f3f3feb692745657392657 Mon Sep 17 00:00:00 2001
> From: Minchan Kim <minchan@kernel.org>
> Date: Fri, 24 May 2019 13:47:54 +0900
> Subject: [PATCH v3 1/5] mm: introduce MADV_COLD
>
> When a process expects no accesses to a certain memory range, it could
> give a hint to kernel that the pages can be reclaimed when memory pressure
> happens but data should be preserved for future use. This could reduce
> workingset eviction so it ends up increasing performance.
>
> This patch introduces the new MADV_COLD hint to madvise(2) syscall.
> MADV_COLD can be used by a process to mark a memory range as not expected
> to be used in the near future. The hint can help kernel in deciding which
> pages to evict early during memory pressure.
>
> It works for every LRU pages like MADV_[DONTNEED|FREE]. IOW, It moves
>
> active file page -> inactive file LRU
> active anon page -> inacdtive anon LRU
>
> Unlike MADV_FREE, it doesn't move active anonymous pages to inactive
> file LRU's head because MADV_COLD is a little bit different symantic.
> MADV_FREE means it's okay to discard when the memory pressure because
> the content of the page is *garbage* so freeing such pages is almost zero
> overhead since we don't need to swap out and access afterward causes just
> minor fault. Thus, it would make sense to put those freeable pages in
> inactive file LRU to compete other used-once pages. It makes sense for
> implmentaion point of view, too because it's not swapbacked memory any
> longer until it would be re-dirtied. Even, it could give a bonus to make
> them be reclaimed on swapless system. However, MADV_COLD doesn't mean
> garbage so reclaiming them requires swap-out/in in the end so it's bigger
> cost. Since we have designed VM LRU aging based on cost-model, anonymous
> cold pages would be better to position inactive anon's LRU list, not file
> LRU. Furthermore, it would help to avoid unnecessary scanning if system
> doesn't have a swap device. Let's start simpler way without adding
> complexity at this moment. However, keep in mind, too that it's a caveat
> that workloads with a lot of pages cache are likely to ignore MADV_COLD
> on anonymous memory because we rarely age anonymous LRU lists.
>
> * man-page material
>
> MADV_COLD (since Linux x.x)
>
> Pages in the specified regions will be treated as less-recently-accessed
> compared to pages in the system with similar access frequencies.
> In contrast to MADV_FREE, the contents of the region are preserved
> regardless of subsequent writes to pages.
>
> MADV_COLD cannot be applied to locked pages, Huge TLB pages, or VM_PFNMAP
> pages.
>
> * v2
> * add up the warn with lots of page cache workload - mhocko
> * add man page stuff - dave
>
> * v1
> * remove page_mapcount filter - hannes, mhocko
> * remove idle page handling - joelaf
>
> * RFCv2
> * add more description - mhocko
>
> * RFCv1
> * renaming from MADV_COOL to MADV_COLD - hannes
>
> * internal review
> * use clear_page_youn in deactivate_page - joelaf
> * Revise the description - surenb
> * Renaming from MADV_WARM to MADV_COOL - surenb
>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
OK, looks reasonable to me. THP part still gives me a head spin but it
is consistent with madv_free part so I will trust that all weird corner
cases are already caught there.
Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 3/5] mm: account nr_isolated_xxx in [isolate|putback]_lru_page
2019-06-27 11:54 ` [PATCH v3 3/5] mm: account nr_isolated_xxx in [isolate|putback]_lru_page Minchan Kim
@ 2019-07-09 9:38 ` Michal Hocko
0 siblings, 0 replies; 27+ messages in thread
From: Michal Hocko @ 2019-07-09 9:38 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Thu 27-06-19 20:54:03, Minchan Kim wrote:
> The isolate counting is pecpu counter so it would be not huge gain
> to work them by batch. Rather than complicating to make them batch,
> let's make it more stright-foward via adding the counting logic
> into [isolate|putback]_lru_page API.
>
> * v1
> * fix accounting bug - Hillf
>
> Link: http://lkml.kernel.org/r/20190531165927.GA20067@cmpxchg.org
> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
I like that the NR_ISOLATED_$FOO handling gets out of any code except
for vmscan and migration. This is definitely an improvement.
I haven't spotted any imbalance so I hope I haven't really missed any
path.
Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!
> ---
> mm/compaction.c | 2 --
> mm/gup.c | 7 +------
> mm/khugepaged.c | 3 ---
> mm/memory-failure.c | 3 ---
> mm/memory_hotplug.c | 4 ----
> mm/mempolicy.c | 6 +-----
> mm/migrate.c | 37 ++++++++-----------------------------
> mm/vmscan.c | 22 ++++++++++++++++------
> 8 files changed, 26 insertions(+), 58 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 9e1b9acb116b..c6591682deda 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -982,8 +982,6 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>
> /* Successfully isolated */
> del_page_from_lru_list(page, lruvec, page_lru(page));
> - inc_node_page_state(page,
> - NR_ISOLATED_ANON + page_is_file_cache(page));
>
> isolate_success:
> list_add(&page->lru, &cc->migratepages);
> diff --git a/mm/gup.c b/mm/gup.c
> index 7dde2e3a1963..aec3a2b7e61b 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1473,13 +1473,8 @@ static long check_and_migrate_cma_pages(struct task_struct *tsk,
> drain_allow = false;
> }
>
> - if (!isolate_lru_page(head)) {
> + if (!isolate_lru_page(head))
> list_add_tail(&head->lru, &cma_page_list);
> - mod_node_page_state(page_pgdat(head),
> - NR_ISOLATED_ANON +
> - page_is_file_cache(head),
> - hpage_nr_pages(head));
> - }
> }
> }
> }
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 0f7419938008..7da34e198ec5 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -503,7 +503,6 @@ void __khugepaged_exit(struct mm_struct *mm)
>
> static void release_pte_page(struct page *page)
> {
> - dec_node_page_state(page, NR_ISOLATED_ANON + page_is_file_cache(page));
> unlock_page(page);
> putback_lru_page(page);
> }
> @@ -602,8 +601,6 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> result = SCAN_DEL_PAGE_LRU;
> goto out;
> }
> - inc_node_page_state(page,
> - NR_ISOLATED_ANON + page_is_file_cache(page));
> VM_BUG_ON_PAGE(!PageLocked(page), page);
> VM_BUG_ON_PAGE(PageLRU(page), page);
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 7e08cbf3ba49..3586e8226e4e 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1795,9 +1795,6 @@ static int __soft_offline_page(struct page *page, int flags)
> * so use !__PageMovable instead for LRU page's mapping
> * cannot have PAGE_MAPPING_MOVABLE.
> */
> - if (!__PageMovable(page))
> - inc_node_page_state(page, NR_ISOLATED_ANON +
> - page_is_file_cache(page));
> list_add(&page->lru, &pagelist);
> ret = migrate_pages(&pagelist, new_page, NULL, MPOL_MF_MOVE_ALL,
> MIGRATE_SYNC, MR_MEMORY_FAILURE);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index dfab21dc33dc..68577c677b46 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1384,10 +1384,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
> ret = isolate_movable_page(page, ISOLATE_UNEVICTABLE);
> if (!ret) { /* Success */
> list_add_tail(&page->lru, &source);
> - if (!__PageMovable(page))
> - inc_node_page_state(page, NR_ISOLATED_ANON +
> - page_is_file_cache(page));
> -
> } else {
> pr_warn("failed to isolate pfn %lx\n", pfn);
> dump_page(page, "isolation failed");
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 64562809bf3b..03081f3404ca 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -994,12 +994,8 @@ static int migrate_page_add(struct page *page, struct list_head *pagelist,
> * Avoid migrating a page that is shared with others.
> */
> if ((flags & MPOL_MF_MOVE_ALL) || page_mapcount(head) == 1) {
> - if (!isolate_lru_page(head)) {
> + if (!isolate_lru_page(head))
> list_add_tail(&head->lru, pagelist);
> - mod_node_page_state(page_pgdat(head),
> - NR_ISOLATED_ANON + page_is_file_cache(head),
> - hpage_nr_pages(head));
> - }
> }
>
> return 0;
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 572b4bc85d76..5583324c01e7 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -190,8 +190,6 @@ void putback_movable_pages(struct list_head *l)
> unlock_page(page);
> put_page(page);
> } else {
> - mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
> - page_is_file_cache(page), -hpage_nr_pages(page));
> putback_lru_page(page);
> }
> }
> @@ -1181,10 +1179,17 @@ static ICE_noinline int unmap_and_move(new_page_t get_new_page,
> return -ENOMEM;
>
> if (page_count(page) == 1) {
> + bool is_lru = !__PageMovable(page);
> +
> /* page was freed from under us. So we are done. */
> ClearPageActive(page);
> ClearPageUnevictable(page);
> - if (unlikely(__PageMovable(page))) {
> + if (likely(is_lru))
> + mod_node_page_state(page_pgdat(page),
> + NR_ISOLATED_ANON +
> + page_is_file_cache(page),
> + -hpage_nr_pages(page));
> + else {
> lock_page(page);
> if (!PageMovable(page))
> __ClearPageIsolated(page);
> @@ -1210,15 +1215,6 @@ static ICE_noinline int unmap_and_move(new_page_t get_new_page,
> * restored.
> */
> list_del(&page->lru);
> -
> - /*
> - * Compaction can migrate also non-LRU pages which are
> - * not accounted to NR_ISOLATED_*. They can be recognized
> - * as __PageMovable
> - */
> - if (likely(!__PageMovable(page)))
> - mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
> - page_is_file_cache(page), -hpage_nr_pages(page));
> }
>
> /*
> @@ -1572,9 +1568,6 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr,
>
> err = 0;
> list_add_tail(&head->lru, pagelist);
> - mod_node_page_state(page_pgdat(head),
> - NR_ISOLATED_ANON + page_is_file_cache(head),
> - hpage_nr_pages(head));
> }
> out_putpage:
> /*
> @@ -1890,8 +1883,6 @@ static struct page *alloc_misplaced_dst_page(struct page *page,
>
> static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
> {
> - int page_lru;
> -
> VM_BUG_ON_PAGE(compound_order(page) && !PageTransHuge(page), page);
>
> /* Avoid migrating to a node that is nearly full */
> @@ -1913,10 +1904,6 @@ static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
> return 0;
> }
>
> - page_lru = page_is_file_cache(page);
> - mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON + page_lru,
> - hpage_nr_pages(page));
> -
> /*
> * Isolating the page has taken another reference, so the
> * caller's reference can be safely dropped without the page
> @@ -1971,8 +1958,6 @@ int migrate_misplaced_page(struct page *page, struct vm_area_struct *vma,
> if (nr_remaining) {
> if (!list_empty(&migratepages)) {
> list_del(&page->lru);
> - dec_node_page_state(page, NR_ISOLATED_ANON +
> - page_is_file_cache(page));
> putback_lru_page(page);
> }
> isolated = 0;
> @@ -2002,7 +1987,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
> pg_data_t *pgdat = NODE_DATA(node);
> int isolated = 0;
> struct page *new_page = NULL;
> - int page_lru = page_is_file_cache(page);
> unsigned long start = address & HPAGE_PMD_MASK;
>
> new_page = alloc_pages_node(node,
> @@ -2048,8 +2032,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
> /* Retake the callers reference and putback on LRU */
> get_page(page);
> putback_lru_page(page);
> - mod_node_page_state(page_pgdat(page),
> - NR_ISOLATED_ANON + page_lru, -HPAGE_PMD_NR);
>
> goto out_unlock;
> }
> @@ -2099,9 +2081,6 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
> count_vm_events(PGMIGRATE_SUCCESS, HPAGE_PMD_NR);
> count_vm_numa_events(NUMA_PAGE_MIGRATE, HPAGE_PMD_NR);
>
> - mod_node_page_state(page_pgdat(page),
> - NR_ISOLATED_ANON + page_lru,
> - -HPAGE_PMD_NR);
> return isolated;
>
> out_fail:
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 49e9ee4d771d..223ce5da08f0 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1014,6 +1014,9 @@ int remove_mapping(struct address_space *mapping, struct page *page)
> void putback_lru_page(struct page *page)
> {
> lru_cache_add(page);
> + mod_node_page_state(page_pgdat(page),
> + NR_ISOLATED_ANON + page_is_file_cache(page),
> + -hpage_nr_pages(page));
> put_page(page); /* drop ref from isolate */
> }
>
> @@ -1479,6 +1482,9 @@ static unsigned long shrink_page_list(struct list_head *page_list,
> */
> nr_reclaimed += nr_pages;
>
> + mod_node_page_state(pgdat, NR_ISOLATED_ANON +
> + page_is_file_cache(page),
> + -nr_pages);
> /*
> * Is there need to periodically free_page_list? It would
> * appear not as the counts should be low
> @@ -1554,7 +1560,6 @@ unsigned long reclaim_clean_pages_from_list(struct zone *zone,
> ret = shrink_page_list(&clean_pages, zone->zone_pgdat, &sc,
> TTU_IGNORE_ACCESS, &dummy_stat, true);
> list_splice(&clean_pages, page_list);
> - mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_FILE, -ret);
> return ret;
> }
>
> @@ -1630,6 +1635,9 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode)
> */
> ClearPageLRU(page);
> ret = 0;
> + __mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON +
> + page_is_file_cache(page),
> + hpage_nr_pages(page));
> }
>
> return ret;
> @@ -1761,6 +1769,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
> trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
> total_scan, skipped, nr_taken, mode, lru);
> update_lru_sizes(lruvec, lru, nr_zone_taken);
> +
> return nr_taken;
> }
>
> @@ -1809,6 +1818,9 @@ int isolate_lru_page(struct page *page)
> ClearPageLRU(page);
> del_page_from_lru_list(page, lruvec, lru);
> ret = 0;
> + mod_node_page_state(pgdat, NR_ISOLATED_ANON +
> + page_is_file_cache(page),
> + hpage_nr_pages(page));
> }
> spin_unlock_irq(&pgdat->lru_lock);
> }
> @@ -1900,6 +1912,9 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec,
> update_lru_size(lruvec, lru, page_zonenum(page), nr_pages);
> list_move(&page->lru, &lruvec->lists[lru]);
>
> + __mod_node_page_state(pgdat, NR_ISOLATED_ANON +
> + page_is_file_cache(page),
> + -hpage_nr_pages(page));
> if (put_page_testzero(page)) {
> __ClearPageLRU(page);
> __ClearPageActive(page);
> @@ -1977,7 +1992,6 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
> nr_taken = isolate_lru_pages(nr_to_scan, lruvec, &page_list,
> &nr_scanned, sc, lru);
>
> - __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken);
> reclaim_stat->recent_scanned[file] += nr_taken;
>
> item = current_is_kswapd() ? PGSCAN_KSWAPD : PGSCAN_DIRECT;
> @@ -2003,8 +2017,6 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
>
> move_pages_to_lru(lruvec, &page_list);
>
> - __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, -nr_taken);
> -
> spin_unlock_irq(&pgdat->lru_lock);
>
> mem_cgroup_uncharge_list(&page_list);
> @@ -2063,7 +2075,6 @@ static void shrink_active_list(unsigned long nr_to_scan,
> nr_taken = isolate_lru_pages(nr_to_scan, lruvec, &l_hold,
> &nr_scanned, sc, lru);
>
> - __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken);
> reclaim_stat->recent_scanned[file] += nr_taken;
>
> __count_vm_events(PGREFILL, nr_scanned);
> @@ -2132,7 +2143,6 @@ static void shrink_active_list(unsigned long nr_to_scan,
> __count_vm_events(PGDEACTIVATE, nr_deactivate);
> __count_memcg_events(lruvec_memcg(lruvec), PGDEACTIVATE, nr_deactivate);
>
> - __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, -nr_taken);
> spin_unlock_irq(&pgdat->lru_lock);
>
> mem_cgroup_uncharge_list(&l_active);
> --
> 2.22.0.410.gd8fdbe21b5-goog
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 4/5] mm: introduce MADV_PAGEOUT
2019-06-27 11:54 ` [PATCH v3 4/5] mm: introduce MADV_PAGEOUT Minchan Kim
@ 2019-07-09 9:55 ` Michal Hocko
2019-07-10 10:48 ` Minchan Kim
0 siblings, 1 reply; 27+ messages in thread
From: Michal Hocko @ 2019-07-09 9:55 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Thu 27-06-19 20:54:04, Minchan Kim wrote:
> When a process expects no accesses to a certain memory range
> for a long time, it could hint kernel that the pages can be
> reclaimed instantly but data should be preserved for future use.
> This could reduce workingset eviction so it ends up increasing
> performance.
>
> This patch introduces the new MADV_PAGEOUT hint to madvise(2)
> syscall. MADV_PAGEOUT can be used by a process to mark a memory
> range as not expected to be used for a long time so that kernel
> reclaims *any LRU* pages instantly. The hint can help kernel in
> deciding which pages to evict proactively.
>
> - man-page material
>
> MADV_PAGEOUT (since Linux x.x)
>
> Do not expect access in the near future so pages in the specified
> regions could be reclaimed instantly regardless of memory pressure.
> Thus, access in the range after successful operation could cause
> major page fault but never lose the up-to-date contents unlike
> MADV_DONTNEED.
> It works for only private anonymous mappings and
> non-anonymous mappings that belong to files that the calling process
> could successfully open for writing; otherwise, it could be used for
> sidechannel attack.
I would rephrase this way:
"
Pages belonging to a shared mapping are only processed if a write access
is allowed for the calling process.
"
I wouldn't really mention side channel attacks for a man page. You can
mention can_do_mincore check and the side channel prevention in the
changelog that is not aimed for the man page.
> MADV_PAGEOUT cannot be applied to locked pages, Huge TLB pages, or
> VM_PFNMAP pages.
>
> * v2
> * add comment about SWAP_CLUSTER_MAX - mhocko
> * add permission check to prevent sidechannel attack - mhocko
> * add man page stuff - dave
>
> * v1
> * change pte to old and rely on the other's reference - hannes
> * remove page_mapcount to check shared page - mhocko
>
> * RFC v2
> * make reclaim_pages simple via factoring out isolate logic - hannes
>
> * RFCv1
> * rename from MADV_COLD to MADV_PAGEOUT - hannes
> * bail out if process is being killed - Hillf
> * fix reclaim_pages bugs - Hillf
>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
I am still not convinced about the SWAP_CLUSTER_MAX batching and the
udnerlying OOM argument. Is one pmd worth of pages really an OOM risk?
Sure you can have many invocations in parallel and that would add on
but the same might happen with SWAP_CLUSTER_MAX. So I would just remove
the batching for now and think of it only if we really see this being a
problem for real. Unless you feel really strong about this, of course.
Anyway the patch looks ok to me otherwise.
Acked-by: Michal Hocko <mhocko@suse.co>
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 5/5] mm: factor out pmd young/dirty bit handling and THP split
2019-06-27 11:54 ` [PATCH v3 5/5] mm: factor out pmd young/dirty bit handling and THP split Minchan Kim
@ 2019-07-09 14:10 ` Michal Hocko
2019-07-10 10:56 ` Minchan Kim
0 siblings, 1 reply; 27+ messages in thread
From: Michal Hocko @ 2019-07-09 14:10 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Thu 27-06-19 20:54:05, Minchan Kim wrote:
> Now, there are common part among MADV_COLD|PAGEOUT|FREE to reset
> access/dirty bit resetting or split the THP page to handle part
> of subpages in the THP page. This patch factor out the common part.
While this reduces the code duplication to some degree I suspect it only
goes half way. I haven't tried that myself due to lack of time but I
believe this has a potential to reduce even more. All those madvise
calls are doing the same thing essentially. What page tables and apply
an operation on ptes and/or a page that is mapped. And that suggests
that the specific operation should be good with defining two - pte and
page operations on each entry. All the rest should be a common code.
That being said, I do not feel strongly about this patch. The rest of
the series should be good enough even without it and I wouldn't delay it
just by discussing a potential of the cleanup.
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
> include/linux/huge_mm.h | 3 -
> mm/huge_memory.c | 74 -------------
> mm/madvise.c | 234 +++++++++++++++++++++++-----------------
> 3 files changed, 135 insertions(+), 176 deletions(-)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 7cd5c150c21d..2667e1aa3ce5 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -29,9 +29,6 @@ extern struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
> unsigned long addr,
> pmd_t *pmd,
> unsigned int flags);
> -extern bool madvise_free_huge_pmd(struct mmu_gather *tlb,
> - struct vm_area_struct *vma,
> - pmd_t *pmd, unsigned long addr, unsigned long next);
> extern int zap_huge_pmd(struct mmu_gather *tlb,
> struct vm_area_struct *vma,
> pmd_t *pmd, unsigned long addr);
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 93f531b63a45..e4b9a06788f3 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1671,80 +1671,6 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf, pmd_t pmd)
> return 0;
> }
>
> -/*
> - * Return true if we do MADV_FREE successfully on entire pmd page.
> - * Otherwise, return false.
> - */
> -bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> - pmd_t *pmd, unsigned long addr, unsigned long next)
> -{
> - spinlock_t *ptl;
> - pmd_t orig_pmd;
> - struct page *page;
> - struct mm_struct *mm = tlb->mm;
> - bool ret = false;
> -
> - tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
> -
> - ptl = pmd_trans_huge_lock(pmd, vma);
> - if (!ptl)
> - goto out_unlocked;
> -
> - orig_pmd = *pmd;
> - if (is_huge_zero_pmd(orig_pmd))
> - goto out;
> -
> - if (unlikely(!pmd_present(orig_pmd))) {
> - VM_BUG_ON(thp_migration_supported() &&
> - !is_pmd_migration_entry(orig_pmd));
> - goto out;
> - }
> -
> - page = pmd_page(orig_pmd);
> - /*
> - * If other processes are mapping this page, we couldn't discard
> - * the page unless they all do MADV_FREE so let's skip the page.
> - */
> - if (page_mapcount(page) != 1)
> - goto out;
> -
> - if (!trylock_page(page))
> - goto out;
> -
> - /*
> - * If user want to discard part-pages of THP, split it so MADV_FREE
> - * will deactivate only them.
> - */
> - if (next - addr != HPAGE_PMD_SIZE) {
> - get_page(page);
> - spin_unlock(ptl);
> - split_huge_page(page);
> - unlock_page(page);
> - put_page(page);
> - goto out_unlocked;
> - }
> -
> - if (PageDirty(page))
> - ClearPageDirty(page);
> - unlock_page(page);
> -
> - if (pmd_young(orig_pmd) || pmd_dirty(orig_pmd)) {
> - pmdp_invalidate(vma, addr, pmd);
> - orig_pmd = pmd_mkold(orig_pmd);
> - orig_pmd = pmd_mkclean(orig_pmd);
> -
> - set_pmd_at(mm, addr, pmd, orig_pmd);
> - tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> - }
> -
> - mark_page_lazyfree(page);
> - ret = true;
> -out:
> - spin_unlock(ptl);
> -out_unlocked:
> - return ret;
> -}
> -
> static inline void zap_deposited_table(struct mm_struct *mm, pmd_t *pmd)
> {
> pgtable_t pgtable;
> diff --git a/mm/madvise.c b/mm/madvise.c
> index ee210473f639..13b06dc8d402 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -310,6 +310,91 @@ static long madvise_willneed(struct vm_area_struct *vma,
> return 0;
> }
>
> +enum madv_pmdp_reset_t {
> + MADV_PMDP_RESET, /* pmd was reset successfully */
> + MADV_PMDP_SPLIT, /* pmd was split */
> + MADV_PMDP_ERROR,
> +};
> +
> +static enum madv_pmdp_reset_t madvise_pmdp_reset_or_split(struct mm_walk *walk,
> + pmd_t *pmd, spinlock_t *ptl,
> + unsigned long addr, unsigned long end,
> + bool young, bool dirty)
> +{
> + pmd_t orig_pmd;
> + unsigned long next;
> + struct page *page;
> + struct mmu_gather *tlb = walk->private;
> + struct mm_struct *mm = walk->mm;
> + struct vm_area_struct *vma = walk->vma;
> + bool reset_young = false;
> + bool reset_dirty = false;
> + enum madv_pmdp_reset_t ret = MADV_PMDP_ERROR;
> +
> + orig_pmd = *pmd;
> + if (is_huge_zero_pmd(orig_pmd))
> + return ret;
> +
> + if (unlikely(!pmd_present(orig_pmd))) {
> + VM_BUG_ON(thp_migration_supported() &&
> + !is_pmd_migration_entry(orig_pmd));
> + return ret;
> + }
> +
> + next = pmd_addr_end(addr, end);
> + page = pmd_page(orig_pmd);
> + if (next - addr != HPAGE_PMD_SIZE) {
> + /*
> + * THP collapsing is not cheap so only split the page is
> + * private to the this process.
> + */
> + if (page_mapcount(page) != 1)
> + return ret;
> + get_page(page);
> + spin_unlock(ptl);
> + lock_page(page);
> + if (!split_huge_page(page))
> + ret = MADV_PMDP_SPLIT;
> + unlock_page(page);
> + put_page(page);
> + return ret;
> + }
> +
> + if (young && pmd_young(orig_pmd))
> + reset_young = true;
> + if (dirty && pmd_dirty(orig_pmd))
> + reset_dirty = true;
> +
> + /*
> + * Other process could rely on the PG_dirty for data consistency,
> + * not pte_dirty so we could reset PG_dirty only when we are owner
> + * of the page.
> + */
> + if (reset_dirty) {
> + if (page_mapcount(page) != 1)
> + goto out;
> + if (!trylock_page(page))
> + goto out;
> + if (PageDirty(page))
> + ClearPageDirty(page);
> + unlock_page(page);
> + }
> +
> + ret = MADV_PMDP_RESET;
> + if (reset_young || reset_dirty) {
> + tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
> + pmdp_invalidate(vma, addr, pmd);
> + if (reset_young)
> + orig_pmd = pmd_mkold(orig_pmd);
> + if (reset_dirty)
> + orig_pmd = pmd_mkclean(orig_pmd);
> + set_pmd_at(mm, addr, pmd, orig_pmd);
> + tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> + }
> +out:
> + return ret;
> +}
> +
> static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
> unsigned long end, struct mm_walk *walk)
> {
> @@ -319,64 +404,31 @@ static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
> pte_t *orig_pte, *pte, ptent;
> spinlock_t *ptl;
> struct page *page;
> - unsigned long next;
>
> - next = pmd_addr_end(addr, end);
> if (pmd_trans_huge(*pmd)) {
> - pmd_t orig_pmd;
> -
> - tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
> ptl = pmd_trans_huge_lock(pmd, vma);
> if (!ptl)
> return 0;
>
> - orig_pmd = *pmd;
> - if (is_huge_zero_pmd(orig_pmd))
> - goto huge_unlock;
> -
> - if (unlikely(!pmd_present(orig_pmd))) {
> - VM_BUG_ON(thp_migration_supported() &&
> - !is_pmd_migration_entry(orig_pmd));
> - goto huge_unlock;
> - }
> -
> - page = pmd_page(orig_pmd);
> - if (next - addr != HPAGE_PMD_SIZE) {
> - int err;
> -
> - if (page_mapcount(page) != 1)
> - goto huge_unlock;
> -
> - get_page(page);
> + switch (madvise_pmdp_reset_or_split(walk, pmd, ptl, addr, end,
> + true, false)) {
> + case MADV_PMDP_RESET:
> spin_unlock(ptl);
> - lock_page(page);
> - err = split_huge_page(page);
> - unlock_page(page);
> - put_page(page);
> - if (!err)
> - goto regular_page;
> - return 0;
> - }
> -
> - if (pmd_young(orig_pmd)) {
> - pmdp_invalidate(vma, addr, pmd);
> - orig_pmd = pmd_mkold(orig_pmd);
> -
> - set_pmd_at(mm, addr, pmd, orig_pmd);
> - tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> + page = pmd_page(*pmd);
> + test_and_clear_page_young(page);
> + deactivate_page(page);
> + goto next;
> + case MADV_PMDP_ERROR:
> + spin_unlock(ptl);
> + goto next;
> + case MADV_PMDP_SPLIT:
> + ; /* go through */
> }
> -
> - test_and_clear_page_young(page);
> - deactivate_page(page);
> -huge_unlock:
> - spin_unlock(ptl);
> - return 0;
> }
>
> if (pmd_trans_unstable(pmd))
> return 0;
>
> -regular_page:
> tlb_change_page_size(tlb, PAGE_SIZE);
> orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> flush_tlb_batched_pending(mm);
> @@ -443,6 +495,7 @@ static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
>
> arch_enter_lazy_mmu_mode();
> pte_unmap_unlock(orig_pte, ptl);
> +next:
> cond_resched();
>
> return 0;
> @@ -493,70 +546,38 @@ static int madvise_pageout_pte_range(pmd_t *pmd, unsigned long addr,
> LIST_HEAD(page_list);
> struct page *page;
> int isolated = 0;
> - unsigned long next;
>
> if (fatal_signal_pending(current))
> return -EINTR;
>
> - next = pmd_addr_end(addr, end);
> if (pmd_trans_huge(*pmd)) {
> - pmd_t orig_pmd;
> -
> - tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
> ptl = pmd_trans_huge_lock(pmd, vma);
> if (!ptl)
> return 0;
>
> - orig_pmd = *pmd;
> - if (is_huge_zero_pmd(orig_pmd))
> - goto huge_unlock;
> -
> - if (unlikely(!pmd_present(orig_pmd))) {
> - VM_BUG_ON(thp_migration_supported() &&
> - !is_pmd_migration_entry(orig_pmd));
> - goto huge_unlock;
> - }
> -
> - page = pmd_page(orig_pmd);
> - if (next - addr != HPAGE_PMD_SIZE) {
> - int err;
> -
> - if (page_mapcount(page) != 1)
> - goto huge_unlock;
> - get_page(page);
> + switch (madvise_pmdp_reset_or_split(walk, pmd, ptl, addr, end,
> + true, false)) {
> + case MADV_PMDP_RESET:
> + page = pmd_page(*pmd);
> spin_unlock(ptl);
> - lock_page(page);
> - err = split_huge_page(page);
> - unlock_page(page);
> - put_page(page);
> - if (!err)
> - goto regular_page;
> - return 0;
> - }
> -
> - if (isolate_lru_page(page))
> - goto huge_unlock;
> -
> - if (pmd_young(orig_pmd)) {
> - pmdp_invalidate(vma, addr, pmd);
> - orig_pmd = pmd_mkold(orig_pmd);
> -
> - set_pmd_at(mm, addr, pmd, orig_pmd);
> - tlb_remove_tlb_entry(tlb, pmd, addr);
> + if (isolate_lru_page(page))
> + return 0;
> + ClearPageReferenced(page);
> + test_and_clear_page_young(page);
> + list_add(&page->lru, &page_list);
> + reclaim_pages(&page_list);
> + goto next;
> + case MADV_PMDP_ERROR:
> + spin_unlock(ptl);
> + goto next;
> + case MADV_PMDP_SPLIT:
> + ; /* go through */
> }
> -
> - ClearPageReferenced(page);
> - test_and_clear_page_young(page);
> - list_add(&page->lru, &page_list);
> -huge_unlock:
> - spin_unlock(ptl);
> - reclaim_pages(&page_list);
> - return 0;
> }
>
> if (pmd_trans_unstable(pmd))
> return 0;
> -regular_page:
> +
> tlb_change_page_size(tlb, PAGE_SIZE);
> orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> flush_tlb_batched_pending(mm);
> @@ -631,6 +652,7 @@ static int madvise_pageout_pte_range(pmd_t *pmd, unsigned long addr,
> arch_leave_lazy_mmu_mode();
> pte_unmap_unlock(orig_pte, ptl);
> reclaim_pages(&page_list);
> +next:
> cond_resched();
>
> return 0;
> @@ -700,12 +722,26 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
> pte_t *orig_pte, *pte, ptent;
> struct page *page;
> int nr_swap = 0;
> - unsigned long next;
>
> - next = pmd_addr_end(addr, end);
> - if (pmd_trans_huge(*pmd))
> - if (madvise_free_huge_pmd(tlb, vma, pmd, addr, next))
> + if (pmd_trans_huge(*pmd)) {
> + ptl = pmd_trans_huge_lock(pmd, vma);
> + if (!ptl)
> + return 0;
> +
> + switch (madvise_pmdp_reset_or_split(walk, pmd, ptl, addr, end,
> + true, true)) {
> + case MADV_PMDP_RESET:
> + page = pmd_page(*pmd);
> + spin_unlock(ptl);
> + mark_page_lazyfree(page);
> goto next;
> + case MADV_PMDP_ERROR:
> + spin_unlock(ptl);
> + goto next;
> + case MADV_PMDP_SPLIT:
> + ; /* go through */
> + }
> + }
>
> if (pmd_trans_unstable(pmd))
> return 0;
> @@ -817,8 +853,8 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
> }
> arch_leave_lazy_mmu_mode();
> pte_unmap_unlock(orig_pte, ptl);
> - cond_resched();
> next:
> + cond_resched();
> return 0;
> }
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 4/5] mm: introduce MADV_PAGEOUT
2019-07-09 9:55 ` Michal Hocko
@ 2019-07-10 10:48 ` Minchan Kim
2019-07-10 11:16 ` Michal Hocko
0 siblings, 1 reply; 27+ messages in thread
From: Minchan Kim @ 2019-07-10 10:48 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Tue, Jul 09, 2019 at 11:55:19AM +0200, Michal Hocko wrote:
> On Thu 27-06-19 20:54:04, Minchan Kim wrote:
> > When a process expects no accesses to a certain memory range
> > for a long time, it could hint kernel that the pages can be
> > reclaimed instantly but data should be preserved for future use.
> > This could reduce workingset eviction so it ends up increasing
> > performance.
> >
> > This patch introduces the new MADV_PAGEOUT hint to madvise(2)
> > syscall. MADV_PAGEOUT can be used by a process to mark a memory
> > range as not expected to be used for a long time so that kernel
> > reclaims *any LRU* pages instantly. The hint can help kernel in
> > deciding which pages to evict proactively.
> >
> > - man-page material
> >
> > MADV_PAGEOUT (since Linux x.x)
> >
> > Do not expect access in the near future so pages in the specified
> > regions could be reclaimed instantly regardless of memory pressure.
> > Thus, access in the range after successful operation could cause
> > major page fault but never lose the up-to-date contents unlike
> > MADV_DONTNEED.
>
> > It works for only private anonymous mappings and
> > non-anonymous mappings that belong to files that the calling process
> > could successfully open for writing; otherwise, it could be used for
> > sidechannel attack.
>
> I would rephrase this way:
> "
> Pages belonging to a shared mapping are only processed if a write access
> is allowed for the calling process.
> "
>
> I wouldn't really mention side channel attacks for a man page. You can
> mention can_do_mincore check and the side channel prevention in the
> changelog that is not aimed for the man page.
Agree. I will rephrase with one you suggested.
Thanks for the suggestion.
>
> > MADV_PAGEOUT cannot be applied to locked pages, Huge TLB pages, or
> > VM_PFNMAP pages.
> >
> > * v2
> > * add comment about SWAP_CLUSTER_MAX - mhocko
> > * add permission check to prevent sidechannel attack - mhocko
> > * add man page stuff - dave
> >
> > * v1
> > * change pte to old and rely on the other's reference - hannes
> > * remove page_mapcount to check shared page - mhocko
> >
> > * RFC v2
> > * make reclaim_pages simple via factoring out isolate logic - hannes
> >
> > * RFCv1
> > * rename from MADV_COLD to MADV_PAGEOUT - hannes
> > * bail out if process is being killed - Hillf
> > * fix reclaim_pages bugs - Hillf
> >
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
>
>
> I am still not convinced about the SWAP_CLUSTER_MAX batching and the
> udnerlying OOM argument. Is one pmd worth of pages really an OOM risk?
> Sure you can have many invocations in parallel and that would add on
> but the same might happen with SWAP_CLUSTER_MAX. So I would just remove
> the batching for now and think of it only if we really see this being a
> problem for real. Unless you feel really strong about this, of course.
I don't have the number to support SWAP_CLUSTER_MAX batching for hinting
operations. However, I wanted to be consistent with other LRU batching
logic so that it could affect altogether if someone try to increase
SWAP_CLUSTER_MAX which is more efficienty for batching operation, later.
(AFAIK, someone tried it a few years ago but rollback soon, I couldn't
rebemeber what was the reason at that time, anyway).
>
> Anyway the patch looks ok to me otherwise.
>
> Acked-by: Michal Hocko <mhocko@suse.co>
Thanks!
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 5/5] mm: factor out pmd young/dirty bit handling and THP split
2019-07-09 14:10 ` Michal Hocko
@ 2019-07-10 10:56 ` Minchan Kim
0 siblings, 0 replies; 27+ messages in thread
From: Minchan Kim @ 2019-07-10 10:56 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Tue, Jul 09, 2019 at 04:10:19PM +0200, Michal Hocko wrote:
> On Thu 27-06-19 20:54:05, Minchan Kim wrote:
> > Now, there are common part among MADV_COLD|PAGEOUT|FREE to reset
> > access/dirty bit resetting or split the THP page to handle part
> > of subpages in the THP page. This patch factor out the common part.
>
> While this reduces the code duplication to some degree I suspect it only
> goes half way. I haven't tried that myself due to lack of time but I
> believe this has a potential to reduce even more. All those madvise
> calls are doing the same thing essentially. What page tables and apply
> an operation on ptes and/or a page that is mapped. And that suggests
> that the specific operation should be good with defining two - pte and
> page operations on each entry. All the rest should be a common code.
>
> That being said, I do not feel strongly about this patch. The rest of
> the series should be good enough even without it and I wouldn't delay it
> just by discussing a potential of the cleanup.
I totally agree with you. For several cycles, some people asked me to
factor common part out. I understand them why they wanted it. However,
when I tried it, it's not trivial to clean it out due to subtle
difference of them. If I couldn't make it clean at this moment, I want to
keep them without factoing out since it's more readable, at least.
I will drop this patch next submit unless someone pop with better idea.
>
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
> > include/linux/huge_mm.h | 3 -
> > mm/huge_memory.c | 74 -------------
> > mm/madvise.c | 234 +++++++++++++++++++++++-----------------
> > 3 files changed, 135 insertions(+), 176 deletions(-)
> >
> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> > index 7cd5c150c21d..2667e1aa3ce5 100644
> > --- a/include/linux/huge_mm.h
> > +++ b/include/linux/huge_mm.h
> > @@ -29,9 +29,6 @@ extern struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
> > unsigned long addr,
> > pmd_t *pmd,
> > unsigned int flags);
> > -extern bool madvise_free_huge_pmd(struct mmu_gather *tlb,
> > - struct vm_area_struct *vma,
> > - pmd_t *pmd, unsigned long addr, unsigned long next);
> > extern int zap_huge_pmd(struct mmu_gather *tlb,
> > struct vm_area_struct *vma,
> > pmd_t *pmd, unsigned long addr);
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 93f531b63a45..e4b9a06788f3 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -1671,80 +1671,6 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf, pmd_t pmd)
> > return 0;
> > }
> >
> > -/*
> > - * Return true if we do MADV_FREE successfully on entire pmd page.
> > - * Otherwise, return false.
> > - */
> > -bool madvise_free_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> > - pmd_t *pmd, unsigned long addr, unsigned long next)
> > -{
> > - spinlock_t *ptl;
> > - pmd_t orig_pmd;
> > - struct page *page;
> > - struct mm_struct *mm = tlb->mm;
> > - bool ret = false;
> > -
> > - tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
> > -
> > - ptl = pmd_trans_huge_lock(pmd, vma);
> > - if (!ptl)
> > - goto out_unlocked;
> > -
> > - orig_pmd = *pmd;
> > - if (is_huge_zero_pmd(orig_pmd))
> > - goto out;
> > -
> > - if (unlikely(!pmd_present(orig_pmd))) {
> > - VM_BUG_ON(thp_migration_supported() &&
> > - !is_pmd_migration_entry(orig_pmd));
> > - goto out;
> > - }
> > -
> > - page = pmd_page(orig_pmd);
> > - /*
> > - * If other processes are mapping this page, we couldn't discard
> > - * the page unless they all do MADV_FREE so let's skip the page.
> > - */
> > - if (page_mapcount(page) != 1)
> > - goto out;
> > -
> > - if (!trylock_page(page))
> > - goto out;
> > -
> > - /*
> > - * If user want to discard part-pages of THP, split it so MADV_FREE
> > - * will deactivate only them.
> > - */
> > - if (next - addr != HPAGE_PMD_SIZE) {
> > - get_page(page);
> > - spin_unlock(ptl);
> > - split_huge_page(page);
> > - unlock_page(page);
> > - put_page(page);
> > - goto out_unlocked;
> > - }
> > -
> > - if (PageDirty(page))
> > - ClearPageDirty(page);
> > - unlock_page(page);
> > -
> > - if (pmd_young(orig_pmd) || pmd_dirty(orig_pmd)) {
> > - pmdp_invalidate(vma, addr, pmd);
> > - orig_pmd = pmd_mkold(orig_pmd);
> > - orig_pmd = pmd_mkclean(orig_pmd);
> > -
> > - set_pmd_at(mm, addr, pmd, orig_pmd);
> > - tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> > - }
> > -
> > - mark_page_lazyfree(page);
> > - ret = true;
> > -out:
> > - spin_unlock(ptl);
> > -out_unlocked:
> > - return ret;
> > -}
> > -
> > static inline void zap_deposited_table(struct mm_struct *mm, pmd_t *pmd)
> > {
> > pgtable_t pgtable;
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index ee210473f639..13b06dc8d402 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -310,6 +310,91 @@ static long madvise_willneed(struct vm_area_struct *vma,
> > return 0;
> > }
> >
> > +enum madv_pmdp_reset_t {
> > + MADV_PMDP_RESET, /* pmd was reset successfully */
> > + MADV_PMDP_SPLIT, /* pmd was split */
> > + MADV_PMDP_ERROR,
> > +};
> > +
> > +static enum madv_pmdp_reset_t madvise_pmdp_reset_or_split(struct mm_walk *walk,
> > + pmd_t *pmd, spinlock_t *ptl,
> > + unsigned long addr, unsigned long end,
> > + bool young, bool dirty)
> > +{
> > + pmd_t orig_pmd;
> > + unsigned long next;
> > + struct page *page;
> > + struct mmu_gather *tlb = walk->private;
> > + struct mm_struct *mm = walk->mm;
> > + struct vm_area_struct *vma = walk->vma;
> > + bool reset_young = false;
> > + bool reset_dirty = false;
> > + enum madv_pmdp_reset_t ret = MADV_PMDP_ERROR;
> > +
> > + orig_pmd = *pmd;
> > + if (is_huge_zero_pmd(orig_pmd))
> > + return ret;
> > +
> > + if (unlikely(!pmd_present(orig_pmd))) {
> > + VM_BUG_ON(thp_migration_supported() &&
> > + !is_pmd_migration_entry(orig_pmd));
> > + return ret;
> > + }
> > +
> > + next = pmd_addr_end(addr, end);
> > + page = pmd_page(orig_pmd);
> > + if (next - addr != HPAGE_PMD_SIZE) {
> > + /*
> > + * THP collapsing is not cheap so only split the page is
> > + * private to the this process.
> > + */
> > + if (page_mapcount(page) != 1)
> > + return ret;
> > + get_page(page);
> > + spin_unlock(ptl);
> > + lock_page(page);
> > + if (!split_huge_page(page))
> > + ret = MADV_PMDP_SPLIT;
> > + unlock_page(page);
> > + put_page(page);
> > + return ret;
> > + }
> > +
> > + if (young && pmd_young(orig_pmd))
> > + reset_young = true;
> > + if (dirty && pmd_dirty(orig_pmd))
> > + reset_dirty = true;
> > +
> > + /*
> > + * Other process could rely on the PG_dirty for data consistency,
> > + * not pte_dirty so we could reset PG_dirty only when we are owner
> > + * of the page.
> > + */
> > + if (reset_dirty) {
> > + if (page_mapcount(page) != 1)
> > + goto out;
> > + if (!trylock_page(page))
> > + goto out;
> > + if (PageDirty(page))
> > + ClearPageDirty(page);
> > + unlock_page(page);
> > + }
> > +
> > + ret = MADV_PMDP_RESET;
> > + if (reset_young || reset_dirty) {
> > + tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
> > + pmdp_invalidate(vma, addr, pmd);
> > + if (reset_young)
> > + orig_pmd = pmd_mkold(orig_pmd);
> > + if (reset_dirty)
> > + orig_pmd = pmd_mkclean(orig_pmd);
> > + set_pmd_at(mm, addr, pmd, orig_pmd);
> > + tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> > + }
> > +out:
> > + return ret;
> > +}
> > +
> > static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
> > unsigned long end, struct mm_walk *walk)
> > {
> > @@ -319,64 +404,31 @@ static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
> > pte_t *orig_pte, *pte, ptent;
> > spinlock_t *ptl;
> > struct page *page;
> > - unsigned long next;
> >
> > - next = pmd_addr_end(addr, end);
> > if (pmd_trans_huge(*pmd)) {
> > - pmd_t orig_pmd;
> > -
> > - tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
> > ptl = pmd_trans_huge_lock(pmd, vma);
> > if (!ptl)
> > return 0;
> >
> > - orig_pmd = *pmd;
> > - if (is_huge_zero_pmd(orig_pmd))
> > - goto huge_unlock;
> > -
> > - if (unlikely(!pmd_present(orig_pmd))) {
> > - VM_BUG_ON(thp_migration_supported() &&
> > - !is_pmd_migration_entry(orig_pmd));
> > - goto huge_unlock;
> > - }
> > -
> > - page = pmd_page(orig_pmd);
> > - if (next - addr != HPAGE_PMD_SIZE) {
> > - int err;
> > -
> > - if (page_mapcount(page) != 1)
> > - goto huge_unlock;
> > -
> > - get_page(page);
> > + switch (madvise_pmdp_reset_or_split(walk, pmd, ptl, addr, end,
> > + true, false)) {
> > + case MADV_PMDP_RESET:
> > spin_unlock(ptl);
> > - lock_page(page);
> > - err = split_huge_page(page);
> > - unlock_page(page);
> > - put_page(page);
> > - if (!err)
> > - goto regular_page;
> > - return 0;
> > - }
> > -
> > - if (pmd_young(orig_pmd)) {
> > - pmdp_invalidate(vma, addr, pmd);
> > - orig_pmd = pmd_mkold(orig_pmd);
> > -
> > - set_pmd_at(mm, addr, pmd, orig_pmd);
> > - tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> > + page = pmd_page(*pmd);
> > + test_and_clear_page_young(page);
> > + deactivate_page(page);
> > + goto next;
> > + case MADV_PMDP_ERROR:
> > + spin_unlock(ptl);
> > + goto next;
> > + case MADV_PMDP_SPLIT:
> > + ; /* go through */
> > }
> > -
> > - test_and_clear_page_young(page);
> > - deactivate_page(page);
> > -huge_unlock:
> > - spin_unlock(ptl);
> > - return 0;
> > }
> >
> > if (pmd_trans_unstable(pmd))
> > return 0;
> >
> > -regular_page:
> > tlb_change_page_size(tlb, PAGE_SIZE);
> > orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> > flush_tlb_batched_pending(mm);
> > @@ -443,6 +495,7 @@ static int madvise_cold_pte_range(pmd_t *pmd, unsigned long addr,
> >
> > arch_enter_lazy_mmu_mode();
> > pte_unmap_unlock(orig_pte, ptl);
> > +next:
> > cond_resched();
> >
> > return 0;
> > @@ -493,70 +546,38 @@ static int madvise_pageout_pte_range(pmd_t *pmd, unsigned long addr,
> > LIST_HEAD(page_list);
> > struct page *page;
> > int isolated = 0;
> > - unsigned long next;
> >
> > if (fatal_signal_pending(current))
> > return -EINTR;
> >
> > - next = pmd_addr_end(addr, end);
> > if (pmd_trans_huge(*pmd)) {
> > - pmd_t orig_pmd;
> > -
> > - tlb_change_page_size(tlb, HPAGE_PMD_SIZE);
> > ptl = pmd_trans_huge_lock(pmd, vma);
> > if (!ptl)
> > return 0;
> >
> > - orig_pmd = *pmd;
> > - if (is_huge_zero_pmd(orig_pmd))
> > - goto huge_unlock;
> > -
> > - if (unlikely(!pmd_present(orig_pmd))) {
> > - VM_BUG_ON(thp_migration_supported() &&
> > - !is_pmd_migration_entry(orig_pmd));
> > - goto huge_unlock;
> > - }
> > -
> > - page = pmd_page(orig_pmd);
> > - if (next - addr != HPAGE_PMD_SIZE) {
> > - int err;
> > -
> > - if (page_mapcount(page) != 1)
> > - goto huge_unlock;
> > - get_page(page);
> > + switch (madvise_pmdp_reset_or_split(walk, pmd, ptl, addr, end,
> > + true, false)) {
> > + case MADV_PMDP_RESET:
> > + page = pmd_page(*pmd);
> > spin_unlock(ptl);
> > - lock_page(page);
> > - err = split_huge_page(page);
> > - unlock_page(page);
> > - put_page(page);
> > - if (!err)
> > - goto regular_page;
> > - return 0;
> > - }
> > -
> > - if (isolate_lru_page(page))
> > - goto huge_unlock;
> > -
> > - if (pmd_young(orig_pmd)) {
> > - pmdp_invalidate(vma, addr, pmd);
> > - orig_pmd = pmd_mkold(orig_pmd);
> > -
> > - set_pmd_at(mm, addr, pmd, orig_pmd);
> > - tlb_remove_tlb_entry(tlb, pmd, addr);
> > + if (isolate_lru_page(page))
> > + return 0;
> > + ClearPageReferenced(page);
> > + test_and_clear_page_young(page);
> > + list_add(&page->lru, &page_list);
> > + reclaim_pages(&page_list);
> > + goto next;
> > + case MADV_PMDP_ERROR:
> > + spin_unlock(ptl);
> > + goto next;
> > + case MADV_PMDP_SPLIT:
> > + ; /* go through */
> > }
> > -
> > - ClearPageReferenced(page);
> > - test_and_clear_page_young(page);
> > - list_add(&page->lru, &page_list);
> > -huge_unlock:
> > - spin_unlock(ptl);
> > - reclaim_pages(&page_list);
> > - return 0;
> > }
> >
> > if (pmd_trans_unstable(pmd))
> > return 0;
> > -regular_page:
> > +
> > tlb_change_page_size(tlb, PAGE_SIZE);
> > orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> > flush_tlb_batched_pending(mm);
> > @@ -631,6 +652,7 @@ static int madvise_pageout_pte_range(pmd_t *pmd, unsigned long addr,
> > arch_leave_lazy_mmu_mode();
> > pte_unmap_unlock(orig_pte, ptl);
> > reclaim_pages(&page_list);
> > +next:
> > cond_resched();
> >
> > return 0;
> > @@ -700,12 +722,26 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
> > pte_t *orig_pte, *pte, ptent;
> > struct page *page;
> > int nr_swap = 0;
> > - unsigned long next;
> >
> > - next = pmd_addr_end(addr, end);
> > - if (pmd_trans_huge(*pmd))
> > - if (madvise_free_huge_pmd(tlb, vma, pmd, addr, next))
> > + if (pmd_trans_huge(*pmd)) {
> > + ptl = pmd_trans_huge_lock(pmd, vma);
> > + if (!ptl)
> > + return 0;
> > +
> > + switch (madvise_pmdp_reset_or_split(walk, pmd, ptl, addr, end,
> > + true, true)) {
> > + case MADV_PMDP_RESET:
> > + page = pmd_page(*pmd);
> > + spin_unlock(ptl);
> > + mark_page_lazyfree(page);
> > goto next;
> > + case MADV_PMDP_ERROR:
> > + spin_unlock(ptl);
> > + goto next;
> > + case MADV_PMDP_SPLIT:
> > + ; /* go through */
> > + }
> > + }
> >
> > if (pmd_trans_unstable(pmd))
> > return 0;
> > @@ -817,8 +853,8 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
> > }
> > arch_leave_lazy_mmu_mode();
> > pte_unmap_unlock(orig_pte, ptl);
> > - cond_resched();
> > next:
> > + cond_resched();
> > return 0;
> > }
> >
> > --
> > 2.22.0.410.gd8fdbe21b5-goog
>
> --
> Michal Hocko
> SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 4/5] mm: introduce MADV_PAGEOUT
2019-07-10 10:48 ` Minchan Kim
@ 2019-07-10 11:16 ` Michal Hocko
2019-07-10 11:53 ` Minchan Kim
0 siblings, 1 reply; 27+ messages in thread
From: Michal Hocko @ 2019-07-10 11:16 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Wed 10-07-19 19:48:09, Minchan Kim wrote:
> On Tue, Jul 09, 2019 at 11:55:19AM +0200, Michal Hocko wrote:
[...]
> > I am still not convinced about the SWAP_CLUSTER_MAX batching and the
> > udnerlying OOM argument. Is one pmd worth of pages really an OOM risk?
> > Sure you can have many invocations in parallel and that would add on
> > but the same might happen with SWAP_CLUSTER_MAX. So I would just remove
> > the batching for now and think of it only if we really see this being a
> > problem for real. Unless you feel really strong about this, of course.
>
> I don't have the number to support SWAP_CLUSTER_MAX batching for hinting
> operations. However, I wanted to be consistent with other LRU batching
> logic so that it could affect altogether if someone try to increase
> SWAP_CLUSTER_MAX which is more efficienty for batching operation, later.
> (AFAIK, someone tried it a few years ago but rollback soon, I couldn't
> rebemeber what was the reason at that time, anyway).
Then please drop this part. It makes the code more complex while any
benefit is not demonstrated.
> > Anyway the patch looks ok to me otherwise.
> >
> > Acked-by: Michal Hocko <mhocko@suse.co>
>
> Thanks!
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 4/5] mm: introduce MADV_PAGEOUT
2019-07-10 11:16 ` Michal Hocko
@ 2019-07-10 11:53 ` Minchan Kim
2019-07-10 19:47 ` Michal Hocko
0 siblings, 1 reply; 27+ messages in thread
From: Minchan Kim @ 2019-07-10 11:53 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Wed, Jul 10, 2019 at 01:16:22PM +0200, Michal Hocko wrote:
> On Wed 10-07-19 19:48:09, Minchan Kim wrote:
> > On Tue, Jul 09, 2019 at 11:55:19AM +0200, Michal Hocko wrote:
> [...]
> > > I am still not convinced about the SWAP_CLUSTER_MAX batching and the
> > > udnerlying OOM argument. Is one pmd worth of pages really an OOM risk?
> > > Sure you can have many invocations in parallel and that would add on
> > > but the same might happen with SWAP_CLUSTER_MAX. So I would just remove
> > > the batching for now and think of it only if we really see this being a
> > > problem for real. Unless you feel really strong about this, of course.
> >
> > I don't have the number to support SWAP_CLUSTER_MAX batching for hinting
> > operations. However, I wanted to be consistent with other LRU batching
> > logic so that it could affect altogether if someone try to increase
> > SWAP_CLUSTER_MAX which is more efficienty for batching operation, later.
> > (AFAIK, someone tried it a few years ago but rollback soon, I couldn't
> > rebemeber what was the reason at that time, anyway).
>
> Then please drop this part. It makes the code more complex while any
> benefit is not demonstrated.
The history says the benefit.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/patch/?id=d37dd5dcb955dd8c2cdd4eaef1f15d1b7ecbc379
With the history, rather than proving it's worth for upcoming new code,
need to try to prove no harmful any longer if we want to remove(or not
consistent with other reclaim path). It's not the goal of this patch.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 4/5] mm: introduce MADV_PAGEOUT
2019-07-10 11:53 ` Minchan Kim
@ 2019-07-10 19:47 ` Michal Hocko
2019-07-11 0:25 ` Minchan Kim
0 siblings, 1 reply; 27+ messages in thread
From: Michal Hocko @ 2019-07-10 19:47 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Wed 10-07-19 20:53:56, Minchan Kim wrote:
> On Wed, Jul 10, 2019 at 01:16:22PM +0200, Michal Hocko wrote:
> > On Wed 10-07-19 19:48:09, Minchan Kim wrote:
> > > On Tue, Jul 09, 2019 at 11:55:19AM +0200, Michal Hocko wrote:
> > [...]
> > > > I am still not convinced about the SWAP_CLUSTER_MAX batching and the
> > > > udnerlying OOM argument. Is one pmd worth of pages really an OOM risk?
> > > > Sure you can have many invocations in parallel and that would add on
> > > > but the same might happen with SWAP_CLUSTER_MAX. So I would just remove
> > > > the batching for now and think of it only if we really see this being a
> > > > problem for real. Unless you feel really strong about this, of course.
> > >
> > > I don't have the number to support SWAP_CLUSTER_MAX batching for hinting
> > > operations. However, I wanted to be consistent with other LRU batching
> > > logic so that it could affect altogether if someone try to increase
> > > SWAP_CLUSTER_MAX which is more efficienty for batching operation, later.
> > > (AFAIK, someone tried it a few years ago but rollback soon, I couldn't
> > > rebemeber what was the reason at that time, anyway).
> >
> > Then please drop this part. It makes the code more complex while any
> > benefit is not demonstrated.
>
> The history says the benefit.
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/patch/?id=d37dd5dcb955dd8c2cdd4eaef1f15d1b7ecbc379
Limiting the number of isolated pages is fine. All I am saying is that
SWAP_CLUSTER_MAX is an arbitrary number same as 512 pages for one PMD as
a unit of work. Both can lead to the same effect if there are too many
parallel tasks doing the same thing.
I do not want you to change that in the reclaim path. All I am asking
for is to add a bathing without any actual data to back that because
that makes the code more complex without any gains.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 4/5] mm: introduce MADV_PAGEOUT
2019-07-10 19:47 ` Michal Hocko
@ 2019-07-11 0:25 ` Minchan Kim
0 siblings, 0 replies; 27+ messages in thread
From: Minchan Kim @ 2019-07-11 0:25 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
Tim Murray, Joel Fernandes, Suren Baghdasaryan,
Daniel Colascione, Shakeel Butt, Sonny Rao, oleksandr, hdanton,
lizeb, Dave Hansen, Kirill A . Shutemov
On Wed, Jul 10, 2019 at 09:47:19PM +0200, Michal Hocko wrote:
> On Wed 10-07-19 20:53:56, Minchan Kim wrote:
> > On Wed, Jul 10, 2019 at 01:16:22PM +0200, Michal Hocko wrote:
> > > On Wed 10-07-19 19:48:09, Minchan Kim wrote:
> > > > On Tue, Jul 09, 2019 at 11:55:19AM +0200, Michal Hocko wrote:
> > > [...]
> > > > > I am still not convinced about the SWAP_CLUSTER_MAX batching and the
> > > > > udnerlying OOM argument. Is one pmd worth of pages really an OOM risk?
> > > > > Sure you can have many invocations in parallel and that would add on
> > > > > but the same might happen with SWAP_CLUSTER_MAX. So I would just remove
> > > > > the batching for now and think of it only if we really see this being a
> > > > > problem for real. Unless you feel really strong about this, of course.
> > > >
> > > > I don't have the number to support SWAP_CLUSTER_MAX batching for hinting
> > > > operations. However, I wanted to be consistent with other LRU batching
> > > > logic so that it could affect altogether if someone try to increase
> > > > SWAP_CLUSTER_MAX which is more efficienty for batching operation, later.
> > > > (AFAIK, someone tried it a few years ago but rollback soon, I couldn't
> > > > rebemeber what was the reason at that time, anyway).
> > >
> > > Then please drop this part. It makes the code more complex while any
> > > benefit is not demonstrated.
> >
> > The history says the benefit.
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/patch/?id=d37dd5dcb955dd8c2cdd4eaef1f15d1b7ecbc379
>
> Limiting the number of isolated pages is fine. All I am saying is that
> SWAP_CLUSTER_MAX is an arbitrary number same as 512 pages for one PMD as
> a unit of work. Both can lead to the same effect if there are too many
> parallel tasks doing the same thing.
>
> I do not want you to change that in the reclaim path. All I am asking
> for is to add a bathing without any actual data to back that because
> that makes the code more complex without any gains.
I understand what you meant and I'm really one to make code simple.
However, my concern was that we have isolated by SWAP_CLUSTER_MAX(32 pages)
for other path(reclaim/compaction) so I want to be consistent with others.
If you think that the consistency(IOW, others are 32 limit but here 256
limit) is no helpful this case, I don't have any strong opinion.
Let's drop the part. I will add it into description, then.
Thanks.
> --
> Michal Hocko
> SUSE Labs
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2019-07-11 0:25 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-27 11:54 [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Minchan Kim
2019-06-27 11:54 ` [PATCH v3 1/5] mm: introduce MADV_COLD Minchan Kim
2019-06-27 13:13 ` Dave Hansen
2019-06-27 14:02 ` Michal Hocko
2019-06-27 14:36 ` Dave Hansen
2019-06-27 14:53 ` Michal Hocko
2019-06-27 23:56 ` Minchan Kim
2019-07-01 7:35 ` Minchan Kim
2019-07-09 9:19 ` Michal Hocko
2019-06-27 23:46 ` Minchan Kim
2019-06-27 11:54 ` [PATCH v3 2/5] mm: change PAGEREF_RECLAIM_CLEAN with PAGE_REFRECLAIM Minchan Kim
2019-06-27 11:54 ` [PATCH v3 3/5] mm: account nr_isolated_xxx in [isolate|putback]_lru_page Minchan Kim
2019-07-09 9:38 ` Michal Hocko
2019-06-27 11:54 ` [PATCH v3 4/5] mm: introduce MADV_PAGEOUT Minchan Kim
2019-07-09 9:55 ` Michal Hocko
2019-07-10 10:48 ` Minchan Kim
2019-07-10 11:16 ` Michal Hocko
2019-07-10 11:53 ` Minchan Kim
2019-07-10 19:47 ` Michal Hocko
2019-07-11 0:25 ` Minchan Kim
2019-06-27 11:54 ` [PATCH v3 5/5] mm: factor out pmd young/dirty bit handling and THP split Minchan Kim
2019-07-09 14:10 ` Michal Hocko
2019-07-10 10:56 ` Minchan Kim
2019-06-27 18:06 ` [PATCH v3 0/5] Introduce MADV_COLD and MADV_PAGEOUT Kirill A. Shutemov
2019-06-27 23:12 ` Minchan Kim
2019-07-01 7:38 ` Minchan Kim
2019-07-01 10:22 ` Michal Hocko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).