| Message ID | 20190627115405.255259-6-minchan@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce MADV_COLD and MADV_PAGEOUT | expand |
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
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
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; }
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(-)