From: Peter Xu <peterx@redhat.com> Date: 2021-02-09 03:05:40
v4:
- add r-b for Mike on the last patch, add some more commit message explains
that why we don't need wr-protect trick
- fix one warning of unused var in copy_present_page() [Gal]
v3:
- rebase to linux-next/akpm, switch to the new HPAGE helpers [MikeK]
- correct error check for alloc_huge_page(); test it this time to make sure
fork() fails gracefully when overcommit [MikeK]
- move page copy out of pgtable lock: this changed quite a bit of the logic in
the last patch, prealloc is dropped since I found it easier to understand
without looping at all [MikeK]
v2:
- pass in 1 to alloc_huge_page() last param [Mike]
- reduce comment, unify the comment in one place [Linus]
- add r-bs for Mike and Miaohe
---- original cover letter ----
As reported by Gal [1], we still miss the code clip to handle early cow for
hugetlb case, which is true. Again, it still feels odd to fork() after using a
few huge pages, especially if they're privately mapped to me.. However I do
agree with Gal and Jason in that we should still have that since that'll
complete the early cow on fork effort at least, and it'll still fix issues
where buffers are not well under control and not easy to apply MADV_DONTFORK.
The first two patches (1-2) are some cleanups I noticed when reading into the
hugetlb reserve map code. I think it's good to have but they're not necessary
for fixing the fork issue.
The last two patches (3-4) is the real fix.
I tested this with a fork() after some vfio-pci assignment, so I'm pretty sure
the page copy path could trigger well (page will be accounted right after the
fork()), but I didn't do data check since the card I assigned is some random
nic. Gal, please feel free to try this if you have better way to verify the
series.
https://github.com/xzpeter/linux/tree/fork-cow-pin-huge
Please review, thanks!
[1] https://lore.kernel.org/lkml/27564187-4a08-f187-5a84-3df50009f6ca@amazon.com/
Peter Xu (5):
hugetlb: Dedup the code to add a new file_region
hugetlg: Break earlier in add_reservation_in_range() when we can
mm: Introduce page_needs_cow_for_dma() for deciding whether cow
mm: Use is_cow_mapping() across tree where proper
hugetlb: Do early cow when page pinned on src mm
drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 4 +-
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 2 +-
fs/proc/task_mmu.c | 2 -
include/linux/mm.h | 21 ++++
mm/huge_memory.c | 8 +-
mm/hugetlb.c | 123 +++++++++++++++------
mm/internal.h | 5 -
mm/memory.c | 8 +-
8 files changed, 117 insertions(+), 56 deletions(-)
--
2.26.2
From: Peter Xu <peterx@redhat.com> Date: 2021-02-09 03:06:31
All the regions maintained in hugetlb reserved map is inclusive on "from" but
exclusive on "to". We can break earlier even if rg->from==t because it already
means no possible intersection.
This does not need a Fixes in all cases because when it happens (rg->from==t)
we'll not break out of the loop while we should, however the next thing we'd do
is still add the last file_region we'd need and quit the loop in the next
round. So this change is not a bugfix (since the old code should still run
okay iiuc), but we'd better still touch it up to make it logically sane.
Reviewed-by: Mike Kravetz <redacted>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
mm/hugetlb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -387,7 +387,7 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,/* When we find a region that starts beyond our range, we've*finished.*/-if(rg->from>t)+if(rg->from>=t)break;/* Add an entry for last_accounted_offset -> rg->from, and
From: Peter Xu <peterx@redhat.com> Date: 2021-02-09 03:07:42
Introduce hugetlb_resv_map_add() helper to add a new file_region rather than
duplication the similar code twice in add_reservation_in_range().
Reviewed-by: Mike Kravetz <redacted>
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
mm/hugetlb.c | 51 +++++++++++++++++++++++++++------------------------
1 file changed, 27 insertions(+), 24 deletions(-)
@@ -346,7 +364,7 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,longadd=0;structlist_head*head=&resv->regions;longlast_accounted_offset=f;-structfile_region*rg=NULL,*trg=NULL,*nrg=NULL;+structfile_region*rg=NULL,*trg=NULL;if(regions_needed)*regions_needed=0;
@@ -375,18 +393,11 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,/* Add an entry for last_accounted_offset -> rg->from, and*updatelast_accounted_offset.*/-if(rg->from>last_accounted_offset){-add+=rg->from-last_accounted_offset;-if(!regions_needed){-nrg=get_file_region_entry_from_cache(-resv,last_accounted_offset,rg->from);-record_hugetlb_cgroup_uncharge_info(h_cg,h,-resv,nrg);-list_add(&nrg->link,rg->link.prev);-coalesce_file_region(resv,nrg);-}else-*regions_needed+=1;-}+if(rg->from>last_accounted_offset)+add+=hugetlb_resv_map_add(resv,rg,+last_accounted_offset,+rg->from,h,h_cg,+regions_needed);last_accounted_offset=rg->to;}
@@ -394,17 +405,9 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t,/* Handle the case where our range extends beyond*last_accounted_offset.*/-if(last_accounted_offset<t){-add+=t-last_accounted_offset;-if(!regions_needed){-nrg=get_file_region_entry_from_cache(-resv,last_accounted_offset,t);-record_hugetlb_cgroup_uncharge_info(h_cg,h,resv,nrg);-list_add(&nrg->link,rg->link.prev);-coalesce_file_region(resv,nrg);-}else-*regions_needed+=1;-}+if(last_accounted_offset<t)+add+=hugetlb_resv_map_add(resv,rg,last_accounted_offset,+t,h,h_cg,regions_needed);VM_BUG_ON(add<0);returnadd;
From: Peter Xu <peterx@redhat.com> Date: 2021-02-09 03:09:24
This is the last missing piece of the COW-during-fork effort when there're
pinned pages found. One can reference 70e806e4e645 ("mm: Do early cow for
pinned pages during fork() for ptes", 2020-09-27) for more information, since
we do similar things here rather than pte this time, but just for hugetlb.
Note that after Jason's recent work on 57efa1fe5957 ("mm/gup: prevent gup_fast
from racing with COW during fork", 2020-12-15) which is safer and easier to
understand, we're safe now within the whole copy_page_range() against gup-fast,
we don't need the wr-protect trick that proposed in 70e806e4e645 anymore.
Reviewed-by: Mike Kravetz <redacted>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
mm/hugetlb.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 62 insertions(+), 4 deletions(-)
@@ -3807,6 +3821,52 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,}set_huge_swap_pte_at(dst,addr,dst_pte,entry,sz);}else{+entry=huge_ptep_get(src_pte);+ptepage=pte_page(entry);+get_page(ptepage);++/*+*Thisisararecasewhereweseepinnedhugetlb+*pageswhilethey'repronetoCOW.Weneedtodothe+*COWearlierduringfork.+*+*Whenpre-allocatingthepageorcopyingdata,we+*needtobewithoutthepgtablelockssincewecould+*sleepduringtheprocess.+*/+if(unlikely(page_needs_cow_for_dma(vma,ptepage))){+pte_tsrc_pte_old=entry;+structpage*new;++spin_unlock(src_ptl);+spin_unlock(dst_ptl);+/* Do not use reserve as it's private owned */+new=alloc_huge_page(vma,addr,1);+if(IS_ERR(new)){+put_page(ptepage);+ret=PTR_ERR(new);+break;+}+copy_user_huge_page(new,ptepage,addr,vma,+npages);+put_page(ptepage);++/* Install the new huge page if src pte stable */+dst_ptl=huge_pte_lock(h,dst,dst_pte);+src_ptl=huge_pte_lockptr(h,src,src_pte);+spin_lock_nested(src_ptl,SINGLE_DEPTH_NESTING);+entry=huge_ptep_get(src_pte);+if(!pte_same(src_pte_old,entry)){+put_page(new);+/* dst_entry won't change as in child */+gotoagain;+}+hugetlb_install_page(vma,dst_pte,addr,new);+spin_unlock(src_ptl);+spin_unlock(dst_ptl);+continue;+}+if(cow){/**Noneedtonotifyaswearedowngradingpage
From: Peter Xu <peterx@redhat.com> Date: 2021-02-09 03:10:48
We've got quite a few places (pte, pmd, pud) that explicitly checked against
whether we should break the cow right now during fork(). It's easier to
provide a helper, especially before we work the same thing on hugetlbfs.
Since we'll reference is_cow_mapping() in mm.h, move it there too. Actually it
suites mm.h more since internal.h is mm/ only, but mm.h is exported to the
whole kernel. With that we should expect another patch to use is_cow_mapping()
whenever we can across the kernel since we do use it quite a lot but it's
always done with raw code against VM_* flags.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/linux/mm.h | 21 +++++++++++++++++++++
mm/huge_memory.c | 8 ++------
mm/internal.h | 5 -----
mm/memory.c | 8 +-------
4 files changed, 24 insertions(+), 18 deletions(-)
From: Peter Xu <peterx@redhat.com> Date: 2021-02-09 03:13:04
After is_cow_mapping() is exported in mm.h, replace some manual checks
elsewhere throughout the tree but start to use the new helper.
Cc: VMware Graphics <redacted>
Cc: Roland Scheidegger <redacted>
Cc: David Airlie <redacted>
Cc: Daniel Vetter <redacted>
Cc: Mike Kravetz <redacted>
Cc: Alexey Dobriyan <redacted>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c | 4 +---
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 2 +-
fs/proc/task_mmu.c | 2 --
mm/hugetlb.c | 4 +---
4 files changed, 3 insertions(+), 9 deletions(-)
@@ -49,7 +49,7 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)vma->vm_ops=&vmw_vm_ops;/* Use VM_PFNMAP rather than VM_MIXEDMAP if not a COW mapping */-if((vma->vm_flags&(VM_SHARED|VM_MAYWRITE))!=VM_MAYWRITE)+if(!is_cow_mapping(vma->vm_flags))vma->vm_flags=(vma->vm_flags&~VM_MIXEDMAP)|VM_PFNMAP;return0;
From: Jason Gunthorpe <jgg@ziepe.ca> Date: 2021-02-09 17:46:55
On Mon, Feb 08, 2021 at 10:02:27PM -0500, Peter Xu wrote:
We've got quite a few places (pte, pmd, pud) that explicitly checked against
whether we should break the cow right now during fork(). It's easier to
provide a helper, especially before we work the same thing on hugetlbfs.
Since we'll reference is_cow_mapping() in mm.h, move it there too. Actually it
suites mm.h more since internal.h is mm/ only, but mm.h is exported to the
whole kernel. With that we should expect another patch to use is_cow_mapping()
whenever we can across the kernel since we do use it quite a lot but it's
always done with raw code against VM_* flags.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/linux/mm.h | 21 +++++++++++++++++++++
mm/huge_memory.c | 8 ++------
mm/internal.h | 5 -----
mm/memory.c | 8 +-------
4 files changed, 24 insertions(+), 18 deletions(-)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
From: Jason Gunthorpe <jgg@ziepe.ca> Date: 2021-02-09 19:14:42
On Mon, Feb 08, 2021 at 10:02:29PM -0500, Peter Xu wrote:
This is the last missing piece of the COW-during-fork effort when there're
pinned pages found. One can reference 70e806e4e645 ("mm: Do early cow for
pinned pages during fork() for ptes", 2020-09-27) for more information, since
we do similar things here rather than pte this time, but just for hugetlb.
Note that after Jason's recent work on 57efa1fe5957 ("mm/gup: prevent gup_fast
from racing with COW during fork", 2020-12-15) which is safer and easier to
understand, we're safe now within the whole copy_page_range() against gup-fast,
we don't need the wr-protect trick that proposed in 70e806e4e645 anymore.
Reviewed-by: Mike Kravetz <redacted>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
mm/hugetlb.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 62 insertions(+), 4 deletions(-)
Didn't check every hugetlbfs detail, but looks reasonable
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason