* [PATCH] mm: deduct the number of pages reclaimed by madvise from workingset @ 2023-05-24 9:12 zhaoyang.huang 2023-05-24 20:40 ` Suren Baghdasaryan 2023-05-25 13:54 ` Johannes Weiner 0 siblings, 2 replies; 6+ messages in thread From: zhaoyang.huang @ 2023-05-24 9:12 UTC (permalink / raw) To: Andrew Morton, Johannes Weiner, Suren Baghdasaryan, linux-mm, linux-kernel, Zhaoyang Huang, ke.wang From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> The pages reclaimed by madvise_pageout are made of inactive and dropped from LRU forcefully, which lead to the coming up refault pages possess a large refault distance than it should be. These could affect the accuracy of thrashing when madvise_pageout is used as a common way of memory reclaiming as ANDROID does now. Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> --- include/linux/swap.h | 2 +- mm/madvise.c | 4 ++-- mm/vmscan.c | 8 +++++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/include/linux/swap.h b/include/linux/swap.h index 2787b84..0312142 100644 --- a/include/linux/swap.h +++ b/include/linux/swap.h @@ -428,7 +428,7 @@ extern unsigned long mem_cgroup_shrink_node(struct mem_cgroup *mem, extern int vm_swappiness; long remove_mapping(struct address_space *mapping, struct folio *folio); -extern unsigned long reclaim_pages(struct list_head *page_list); +extern unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *page_list); #ifdef CONFIG_NUMA extern int node_reclaim_mode; extern int sysctl_min_unmapped_ratio; diff --git a/mm/madvise.c b/mm/madvise.c index b6ea204..61c8d7b 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -420,7 +420,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, huge_unlock: spin_unlock(ptl); if (pageout) - reclaim_pages(&page_list); + reclaim_pages(mm, &page_list); return 0; } @@ -516,7 +516,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, arch_leave_lazy_mmu_mode(); pte_unmap_unlock(orig_pte, ptl); if (pageout) - reclaim_pages(&page_list); + reclaim_pages(mm, &page_list); cond_resched(); return 0; diff --git a/mm/vmscan.c b/mm/vmscan.c index 20facec..048c10b 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -2741,12 +2741,14 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list, return nr_reclaimed; } -unsigned long reclaim_pages(struct list_head *folio_list) +unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *folio_list) { int nid; unsigned int nr_reclaimed = 0; LIST_HEAD(node_folio_list); unsigned int noreclaim_flag; + struct lruvec *lruvec; + struct mem_cgroup *memcg = get_mem_cgroup_from_mm(mm); if (list_empty(folio_list)) return nr_reclaimed; @@ -2764,10 +2766,14 @@ unsigned long reclaim_pages(struct list_head *folio_list) } nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); + lruvec = &memcg->nodeinfo[nid]->lruvec; + workingset_age_nonresident(lruvec, -nr_reclaimed); nid = folio_nid(lru_to_folio(folio_list)); } while (!list_empty(folio_list)); nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); + lruvec = &memcg->nodeinfo[nid]->lruvec; + workingset_age_nonresident(lruvec, -nr_reclaimed); memalloc_noreclaim_restore(noreclaim_flag); -- 1.9.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: deduct the number of pages reclaimed by madvise from workingset 2023-05-24 9:12 [PATCH] mm: deduct the number of pages reclaimed by madvise from workingset zhaoyang.huang @ 2023-05-24 20:40 ` Suren Baghdasaryan 2023-05-25 1:23 ` Zhaoyang Huang 2023-05-25 13:54 ` Johannes Weiner 1 sibling, 1 reply; 6+ messages in thread From: Suren Baghdasaryan @ 2023-05-24 20:40 UTC (permalink / raw) To: zhaoyang.huang Cc: Andrew Morton, Johannes Weiner, linux-mm, linux-kernel, Zhaoyang Huang, ke.wang On Wed, May 24, 2023 at 2:13 AM zhaoyang.huang <zhaoyang.huang@unisoc.com> wrote: > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > The pages reclaimed by madvise_pageout are made of inactive and dropped from LRU > forcefully, which lead to the coming up refault pages possess a large refault > distance than it should be. These could affect the accuracy of thrashing when > madvise_pageout is used as a common way of memory reclaiming as ANDROID does now. Doesn't workingset_eviction() in the following call chain already handle nonresident page aging?: reclaim_pages reclaim_folio_list shrink_folio_list __remove_mapping workingset_eviction workingset_age_nonresident > > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > --- > include/linux/swap.h | 2 +- > mm/madvise.c | 4 ++-- > mm/vmscan.c | 8 +++++++- > 3 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/include/linux/swap.h b/include/linux/swap.h > index 2787b84..0312142 100644 > --- a/include/linux/swap.h > +++ b/include/linux/swap.h > @@ -428,7 +428,7 @@ extern unsigned long mem_cgroup_shrink_node(struct mem_cgroup *mem, > extern int vm_swappiness; > long remove_mapping(struct address_space *mapping, struct folio *folio); > > -extern unsigned long reclaim_pages(struct list_head *page_list); > +extern unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *page_list); > #ifdef CONFIG_NUMA > extern int node_reclaim_mode; > extern int sysctl_min_unmapped_ratio; > diff --git a/mm/madvise.c b/mm/madvise.c > index b6ea204..61c8d7b 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -420,7 +420,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > huge_unlock: > spin_unlock(ptl); > if (pageout) > - reclaim_pages(&page_list); > + reclaim_pages(mm, &page_list); > return 0; > } > > @@ -516,7 +516,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > arch_leave_lazy_mmu_mode(); > pte_unmap_unlock(orig_pte, ptl); > if (pageout) > - reclaim_pages(&page_list); > + reclaim_pages(mm, &page_list); > cond_resched(); > > return 0; > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 20facec..048c10b 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -2741,12 +2741,14 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list, > return nr_reclaimed; > } > > -unsigned long reclaim_pages(struct list_head *folio_list) > +unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *folio_list) You would also need to change Damon usage of reclaim_pages() here: https://elixir.bootlin.com/linux/v6.4-rc1/source/mm/damon/paddr.c#L253 > { > int nid; > unsigned int nr_reclaimed = 0; > LIST_HEAD(node_folio_list); > unsigned int noreclaim_flag; > + struct lruvec *lruvec; > + struct mem_cgroup *memcg = get_mem_cgroup_from_mm(mm); > > if (list_empty(folio_list)) > return nr_reclaimed; > @@ -2764,10 +2766,14 @@ unsigned long reclaim_pages(struct list_head *folio_list) > } > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > + lruvec = &memcg->nodeinfo[nid]->lruvec; > + workingset_age_nonresident(lruvec, -nr_reclaimed); > nid = folio_nid(lru_to_folio(folio_list)); > } while (!list_empty(folio_list)); > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > + lruvec = &memcg->nodeinfo[nid]->lruvec; > + workingset_age_nonresident(lruvec, -nr_reclaimed); > > memalloc_noreclaim_restore(noreclaim_flag); > > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: deduct the number of pages reclaimed by madvise from workingset 2023-05-24 20:40 ` Suren Baghdasaryan @ 2023-05-25 1:23 ` Zhaoyang Huang 0 siblings, 0 replies; 6+ messages in thread From: Zhaoyang Huang @ 2023-05-25 1:23 UTC (permalink / raw) To: Suren Baghdasaryan Cc: zhaoyang.huang, Andrew Morton, Johannes Weiner, linux-mm, linux-kernel, ke.wang On Thu, May 25, 2023 at 4:41 AM Suren Baghdasaryan <surenb@google.com> wrote: > > On Wed, May 24, 2023 at 2:13 AM zhaoyang.huang > <zhaoyang.huang@unisoc.com> wrote: > > > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > > The pages reclaimed by madvise_pageout are made of inactive and dropped from LRU > > forcefully, which lead to the coming up refault pages possess a large refault > > distance than it should be. These could affect the accuracy of thrashing when > > madvise_pageout is used as a common way of memory reclaiming as ANDROID does now. > > Doesn't workingset_eviction() in the following call chain already > handle nonresident page aging?: > > reclaim_pages > reclaim_folio_list > shrink_folio_list > __remove_mapping > workingset_eviction > workingset_age_nonresident Yes. What I suggest is to minor this pages from non-resident as they are dropped forcefully > > > > > > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > --- > > include/linux/swap.h | 2 +- > > mm/madvise.c | 4 ++-- > > mm/vmscan.c | 8 +++++++- > > 3 files changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/include/linux/swap.h b/include/linux/swap.h > > index 2787b84..0312142 100644 > > --- a/include/linux/swap.h > > +++ b/include/linux/swap.h > > @@ -428,7 +428,7 @@ extern unsigned long mem_cgroup_shrink_node(struct mem_cgroup *mem, > > extern int vm_swappiness; > > long remove_mapping(struct address_space *mapping, struct folio *folio); > > > > -extern unsigned long reclaim_pages(struct list_head *page_list); > > +extern unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *page_list); > > #ifdef CONFIG_NUMA > > extern int node_reclaim_mode; > > extern int sysctl_min_unmapped_ratio; > > diff --git a/mm/madvise.c b/mm/madvise.c > > index b6ea204..61c8d7b 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -420,7 +420,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > > huge_unlock: > > spin_unlock(ptl); > > if (pageout) > > - reclaim_pages(&page_list); > > + reclaim_pages(mm, &page_list); > > return 0; > > } > > > > @@ -516,7 +516,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > > arch_leave_lazy_mmu_mode(); > > pte_unmap_unlock(orig_pte, ptl); > > if (pageout) > > - reclaim_pages(&page_list); > > + reclaim_pages(mm, &page_list); > > cond_resched(); > > > > return 0; > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index 20facec..048c10b 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -2741,12 +2741,14 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list, > > return nr_reclaimed; > > } > > > > -unsigned long reclaim_pages(struct list_head *folio_list) > > +unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *folio_list) > > You would also need to change Damon usage of reclaim_pages() here: > https://elixir.bootlin.com/linux/v6.4-rc1/source/mm/damon/paddr.c#L253 ok, thanks for reminding > > > { > > int nid; > > unsigned int nr_reclaimed = 0; > > LIST_HEAD(node_folio_list); > > unsigned int noreclaim_flag; > > + struct lruvec *lruvec; > > + struct mem_cgroup *memcg = get_mem_cgroup_from_mm(mm); > > > > if (list_empty(folio_list)) > > return nr_reclaimed; > > @@ -2764,10 +2766,14 @@ unsigned long reclaim_pages(struct list_head *folio_list) > > } > > > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > > + lruvec = &memcg->nodeinfo[nid]->lruvec; > > + workingset_age_nonresident(lruvec, -nr_reclaimed); > > nid = folio_nid(lru_to_folio(folio_list)); > > } while (!list_empty(folio_list)); > > > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > > + lruvec = &memcg->nodeinfo[nid]->lruvec; > > + workingset_age_nonresident(lruvec, -nr_reclaimed); > > > > memalloc_noreclaim_restore(noreclaim_flag); > > > > -- > > 1.9.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: deduct the number of pages reclaimed by madvise from workingset 2023-05-24 9:12 [PATCH] mm: deduct the number of pages reclaimed by madvise from workingset zhaoyang.huang 2023-05-24 20:40 ` Suren Baghdasaryan @ 2023-05-25 13:54 ` Johannes Weiner 2023-05-26 6:38 ` Zhaoyang Huang 1 sibling, 1 reply; 6+ messages in thread From: Johannes Weiner @ 2023-05-25 13:54 UTC (permalink / raw) To: zhaoyang.huang Cc: Andrew Morton, Suren Baghdasaryan, linux-mm, linux-kernel, Zhaoyang Huang, ke.wang On Wed, May 24, 2023 at 05:12:54PM +0800, zhaoyang.huang wrote: > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > The pages reclaimed by madvise_pageout are made of inactive and dropped from LRU > forcefully, which lead to the coming up refault pages possess a large refault > distance than it should be. These could affect the accuracy of thrashing when > madvise_pageout is used as a common way of memory reclaiming as ANDROID does now. This alludes to, but doesn't explain, a real world usecase. Yes, madvise_pageout() will record non-resident entries today. This means refault and thrash detection is on for user-driven reclaim. So why is that undesirable? Today we measure and report the cost of reclaim and memory pressure for physical memory shortages, cgroup limits, and user-driven cgroup reclaim. Why should we not do the same for madv_pageout()? If the userspace code that drives pageout has a bug and the result is extreme thrashing, wouldn't you want to know that? Please explain the idea here better. > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > --- > include/linux/swap.h | 2 +- > mm/madvise.c | 4 ++-- > mm/vmscan.c | 8 +++++++- > 3 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/include/linux/swap.h b/include/linux/swap.h > index 2787b84..0312142 100644 > --- a/include/linux/swap.h > +++ b/include/linux/swap.h > @@ -428,7 +428,7 @@ extern unsigned long mem_cgroup_shrink_node(struct mem_cgroup *mem, > extern int vm_swappiness; > long remove_mapping(struct address_space *mapping, struct folio *folio); > > -extern unsigned long reclaim_pages(struct list_head *page_list); > +extern unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *page_list); > #ifdef CONFIG_NUMA > extern int node_reclaim_mode; > extern int sysctl_min_unmapped_ratio; > diff --git a/mm/madvise.c b/mm/madvise.c > index b6ea204..61c8d7b 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -420,7 +420,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > huge_unlock: > spin_unlock(ptl); > if (pageout) > - reclaim_pages(&page_list); > + reclaim_pages(mm, &page_list); > return 0; > } > > @@ -516,7 +516,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > arch_leave_lazy_mmu_mode(); > pte_unmap_unlock(orig_pte, ptl); > if (pageout) > - reclaim_pages(&page_list); > + reclaim_pages(mm, &page_list); > cond_resched(); > > return 0; > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 20facec..048c10b 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -2741,12 +2741,14 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list, > return nr_reclaimed; > } > > -unsigned long reclaim_pages(struct list_head *folio_list) > +unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *folio_list) > { > int nid; > unsigned int nr_reclaimed = 0; > LIST_HEAD(node_folio_list); > unsigned int noreclaim_flag; > + struct lruvec *lruvec; > + struct mem_cgroup *memcg = get_mem_cgroup_from_mm(mm); > > if (list_empty(folio_list)) > return nr_reclaimed; > @@ -2764,10 +2766,14 @@ unsigned long reclaim_pages(struct list_head *folio_list) > } > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > + lruvec = &memcg->nodeinfo[nid]->lruvec; > + workingset_age_nonresident(lruvec, -nr_reclaimed); > nid = folio_nid(lru_to_folio(folio_list)); > } while (!list_empty(folio_list)); > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > + lruvec = &memcg->nodeinfo[nid]->lruvec; > + workingset_age_nonresident(lruvec, -nr_reclaimed); The task might have moved cgroups in between, who knows what kind of artifacts it will introduce if you wind back the wrong clock. If there are reclaim passes that shouldn't participate in non-resident tracking, that should be plumbed through the stack to __remove_mapping (which already has that bool reclaimed param to not record entries). ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: deduct the number of pages reclaimed by madvise from workingset 2023-05-25 13:54 ` Johannes Weiner @ 2023-05-26 6:38 ` Zhaoyang Huang 2023-05-26 17:31 ` Suren Baghdasaryan 0 siblings, 1 reply; 6+ messages in thread From: Zhaoyang Huang @ 2023-05-26 6:38 UTC (permalink / raw) To: Johannes Weiner Cc: zhaoyang.huang, Andrew Morton, Suren Baghdasaryan, linux-mm, linux-kernel, ke.wang On Thu, May 25, 2023 at 9:54 PM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Wed, May 24, 2023 at 05:12:54PM +0800, zhaoyang.huang wrote: > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > > The pages reclaimed by madvise_pageout are made of inactive and dropped from LRU > > forcefully, which lead to the coming up refault pages possess a large refault > > distance than it should be. These could affect the accuracy of thrashing when > > madvise_pageout is used as a common way of memory reclaiming as ANDROID does now. > > This alludes to, but doesn't explain, a real world usecase. More block io(wait_on_page_bit_common) observed during APP start in latest android version where user space memory reclaiming changes from in-kernel PPR to madvise_pageout. We believe that it could be related with inaccuracy of workingset. > > Yes, madvise_pageout() will record non-resident entries today. This > means refault and thrash detection is on for user-driven reclaim. > > So why is that undesirable? Let's raise an extreme scenario, that is, the tail page of LRU could experience a given refault distance without any in-kernel reclaiming and be wrongly deemed as inactive and get less protection. > > Today we measure and report the cost of reclaim and memory pressure > for physical memory shortages, cgroup limits, and user-driven cgroup > reclaim. Why should we not do the same for madv_pageout()? If the > userspace code that drives pageout has a bug and the result is extreme > thrashing, wouldn't you want to know that? Actually, the pages evicted by madv_cold/pageout from active_lru are not marked as WORKINGSET, which will surpass the thrashing account when it faults back and gets struck by IO. I think they should be treated in the same way in terms of SetPageWorkingset and lruvec->non-resident. Please refer to my previous patch "mm: mark folio as workingset in lru_deactivate_fn index 70e2063..4d1c14f 100644" > > Please explain the idea here better. > > > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > --- > > include/linux/swap.h | 2 +- > > mm/madvise.c | 4 ++-- > > mm/vmscan.c | 8 +++++++- > > 3 files changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/include/linux/swap.h b/include/linux/swap.h > > index 2787b84..0312142 100644 > > --- a/include/linux/swap.h > > +++ b/include/linux/swap.h > > @@ -428,7 +428,7 @@ extern unsigned long mem_cgroup_shrink_node(struct mem_cgroup *mem, > > extern int vm_swappiness; > > long remove_mapping(struct address_space *mapping, struct folio *folio); > > > > -extern unsigned long reclaim_pages(struct list_head *page_list); > > +extern unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *page_list); > > #ifdef CONFIG_NUMA > > extern int node_reclaim_mode; > > extern int sysctl_min_unmapped_ratio; > > diff --git a/mm/madvise.c b/mm/madvise.c > > index b6ea204..61c8d7b 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -420,7 +420,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > > huge_unlock: > > spin_unlock(ptl); > > if (pageout) > > - reclaim_pages(&page_list); > > + reclaim_pages(mm, &page_list); > > return 0; > > } > > > > @@ -516,7 +516,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > > arch_leave_lazy_mmu_mode(); > > pte_unmap_unlock(orig_pte, ptl); > > if (pageout) > > - reclaim_pages(&page_list); > > + reclaim_pages(mm, &page_list); > > cond_resched(); > > > > return 0; > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index 20facec..048c10b 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -2741,12 +2741,14 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list, > > return nr_reclaimed; > > } > > > > -unsigned long reclaim_pages(struct list_head *folio_list) > > +unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *folio_list) > > { > > int nid; > > unsigned int nr_reclaimed = 0; > > LIST_HEAD(node_folio_list); > > unsigned int noreclaim_flag; > > + struct lruvec *lruvec; > > + struct mem_cgroup *memcg = get_mem_cgroup_from_mm(mm); > > > > if (list_empty(folio_list)) > > return nr_reclaimed; > > @@ -2764,10 +2766,14 @@ unsigned long reclaim_pages(struct list_head *folio_list) > > } > > > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > > + lruvec = &memcg->nodeinfo[nid]->lruvec; > > + workingset_age_nonresident(lruvec, -nr_reclaimed); > > nid = folio_nid(lru_to_folio(folio_list)); > > } while (!list_empty(folio_list)); > > > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > > + lruvec = &memcg->nodeinfo[nid]->lruvec; > > + workingset_age_nonresident(lruvec, -nr_reclaimed); > > The task might have moved cgroups in between, who knows what kind of > artifacts it will introduce if you wind back the wrong clock. > > If there are reclaim passes that shouldn't participate in non-resident > tracking, that should be plumbed through the stack to __remove_mapping > (which already has that bool reclaimed param to not record entries). ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: deduct the number of pages reclaimed by madvise from workingset 2023-05-26 6:38 ` Zhaoyang Huang @ 2023-05-26 17:31 ` Suren Baghdasaryan 0 siblings, 0 replies; 6+ messages in thread From: Suren Baghdasaryan @ 2023-05-26 17:31 UTC (permalink / raw) To: Zhaoyang Huang Cc: Johannes Weiner, zhaoyang.huang, Andrew Morton, linux-mm, linux-kernel, ke.wang On Thu, May 25, 2023 at 11:39 PM Zhaoyang Huang <huangzhaoyang@gmail.com> wrote: > > On Thu, May 25, 2023 at 9:54 PM Johannes Weiner <hannes@cmpxchg.org> wrote: > > > > On Wed, May 24, 2023 at 05:12:54PM +0800, zhaoyang.huang wrote: > > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > > > > The pages reclaimed by madvise_pageout are made of inactive and dropped from LRU > > > forcefully, which lead to the coming up refault pages possess a large refault > > > distance than it should be. These could affect the accuracy of thrashing when > > > madvise_pageout is used as a common way of memory reclaiming as ANDROID does now. > > > > This alludes to, but doesn't explain, a real world usecase. > More block io(wait_on_page_bit_common) observed during APP start in > latest android version where user space memory reclaiming changes from > in-kernel PPR to madvise_pageout. We believe that it could be related > with inaccuracy of workingset. Do you mean the userspace incorrectly treats the active workingset as inactive and purges it? If so, it sounds like the fix should be in the userspace, not in the kernel. > > > > Yes, madvise_pageout() will record non-resident entries today. This > > means refault and thrash detection is on for user-driven reclaim. > > > > So why is that undesirable? > Let's raise an extreme scenario, that is, the tail page of LRU could > experience a given refault distance without any in-kernel reclaiming > and be wrongly deemed as inactive and get less protection. madvise_pageout is a hint to the kernel that this page should be treated as inactive, which it does. Why is that wrong? > > > > Today we measure and report the cost of reclaim and memory pressure > > for physical memory shortages, cgroup limits, and user-driven cgroup > > reclaim. Why should we not do the same for madv_pageout()? If the > > userspace code that drives pageout has a bug and the result is extreme > > thrashing, wouldn't you want to know that? > Actually, the pages evicted by madv_cold/pageout from active_lru are > not marked as WORKINGSET, which will surpass the thrashing account > when it faults back and gets struck by IO. I think they should be > treated in the same way in terms of SetPageWorkingset and > lruvec->non-resident. Please refer to my previous patch "mm: mark > folio as workingset in lru_deactivate_fn index 70e2063..4d1c14f > 100644" I see your point but it's debatable. If madvise_pageout is a hint to treat the page as inactive, then why should the kernel treat it as part of the active workingset when evicting? I guess it boils down to whether the kernel should try fixing a wrong hint from the userspace. I think not but I would be interested in the opinions of others. Thanks, Suren. > > > > > > Please explain the idea here better. > > > > > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > --- > > > include/linux/swap.h | 2 +- > > > mm/madvise.c | 4 ++-- > > > mm/vmscan.c | 8 +++++++- > > > 3 files changed, 10 insertions(+), 4 deletions(-) > > > > > > diff --git a/include/linux/swap.h b/include/linux/swap.h > > > index 2787b84..0312142 100644 > > > --- a/include/linux/swap.h > > > +++ b/include/linux/swap.h > > > @@ -428,7 +428,7 @@ extern unsigned long mem_cgroup_shrink_node(struct mem_cgroup *mem, > > > extern int vm_swappiness; > > > long remove_mapping(struct address_space *mapping, struct folio *folio); > > > > > > -extern unsigned long reclaim_pages(struct list_head *page_list); > > > +extern unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *page_list); > > > #ifdef CONFIG_NUMA > > > extern int node_reclaim_mode; > > > extern int sysctl_min_unmapped_ratio; > > > diff --git a/mm/madvise.c b/mm/madvise.c > > > index b6ea204..61c8d7b 100644 > > > --- a/mm/madvise.c > > > +++ b/mm/madvise.c > > > @@ -420,7 +420,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > > > huge_unlock: > > > spin_unlock(ptl); > > > if (pageout) > > > - reclaim_pages(&page_list); > > > + reclaim_pages(mm, &page_list); > > > return 0; > > > } > > > > > > @@ -516,7 +516,7 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd, > > > arch_leave_lazy_mmu_mode(); > > > pte_unmap_unlock(orig_pte, ptl); > > > if (pageout) > > > - reclaim_pages(&page_list); > > > + reclaim_pages(mm, &page_list); > > > cond_resched(); > > > > > > return 0; > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > > index 20facec..048c10b 100644 > > > --- a/mm/vmscan.c > > > +++ b/mm/vmscan.c > > > @@ -2741,12 +2741,14 @@ static unsigned int reclaim_folio_list(struct list_head *folio_list, > > > return nr_reclaimed; > > > } > > > > > > -unsigned long reclaim_pages(struct list_head *folio_list) > > > +unsigned long reclaim_pages(struct mm_struct *mm, struct list_head *folio_list) > > > { > > > int nid; > > > unsigned int nr_reclaimed = 0; > > > LIST_HEAD(node_folio_list); > > > unsigned int noreclaim_flag; > > > + struct lruvec *lruvec; > > > + struct mem_cgroup *memcg = get_mem_cgroup_from_mm(mm); > > > > > > if (list_empty(folio_list)) > > > return nr_reclaimed; > > > @@ -2764,10 +2766,14 @@ unsigned long reclaim_pages(struct list_head *folio_list) > > > } > > > > > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > > > + lruvec = &memcg->nodeinfo[nid]->lruvec; > > > + workingset_age_nonresident(lruvec, -nr_reclaimed); > > > nid = folio_nid(lru_to_folio(folio_list)); > > > } while (!list_empty(folio_list)); > > > > > > nr_reclaimed += reclaim_folio_list(&node_folio_list, NODE_DATA(nid)); > > > + lruvec = &memcg->nodeinfo[nid]->lruvec; > > > + workingset_age_nonresident(lruvec, -nr_reclaimed); > > > > The task might have moved cgroups in between, who knows what kind of > > artifacts it will introduce if you wind back the wrong clock. > > > > If there are reclaim passes that shouldn't participate in non-resident > > tracking, that should be plumbed through the stack to __remove_mapping > > (which already has that bool reclaimed param to not record entries). ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-26 17:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-24 9:12 [PATCH] mm: deduct the number of pages reclaimed by madvise from workingset zhaoyang.huang 2023-05-24 20:40 ` Suren Baghdasaryan 2023-05-25 1:23 ` Zhaoyang Huang 2023-05-25 13:54 ` Johannes Weiner 2023-05-26 6:38 ` Zhaoyang Huang 2023-05-26 17:31 ` Suren Baghdasaryan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox