| Message ID | 20220421125348.62483-3-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few fixup patches for mm | expand |
On 21.04.22 14:53, Miaohe Lin wrote: > This is observed by code review only but not any real report. > > When we turn off swapping we could have lost the bits stored in the swap > ptes. The new rmap-exclusive bit is fine since that turned into a page > flag, but not for soft-dirty and uffd-wp. Add them. > > Suggested-by: Peter Xu <peterx@redhat.com> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/swapfile.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 95b63f69f388..332ccfc76142 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -1783,7 +1783,7 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, > { > struct page *swapcache; > spinlock_t *ptl; > - pte_t *pte; > + pte_t *pte, new_pte; > int ret = 1; > > swapcache = page; > @@ -1832,8 +1832,14 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, > page_add_new_anon_rmap(page, vma, addr); > lru_cache_add_inactive_or_unevictable(page, vma); > } > - set_pte_at(vma->vm_mm, addr, pte, > - pte_mkold(mk_pte(page, vma->vm_page_prot))); > + new_pte = pte_mkold(mk_pte(page, vma->vm_page_prot)); > + if (pte_swp_soft_dirty(*pte)) > + new_pte = pte_mksoft_dirty(new_pte); > + if (pte_swp_uffd_wp(*pte)) { > + new_pte = pte_mkuffd_wp(new_pte); > + new_pte = pte_wrprotect(new_pte); The wrprotect shouldn't be necessary, we don't do a pte_mkwrite(). Note that in do_swap_page() we might have done a maybe_mkwrite(pte_mkdirty(pte)), which is why the pte_wrprotect() is required there.
On 2022/4/21 21:13, David Hildenbrand wrote: > On 21.04.22 14:53, Miaohe Lin wrote: >> This is observed by code review only but not any real report. >> >> When we turn off swapping we could have lost the bits stored in the swap >> ptes. The new rmap-exclusive bit is fine since that turned into a page >> flag, but not for soft-dirty and uffd-wp. Add them. >> >> Suggested-by: Peter Xu <peterx@redhat.com> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> mm/swapfile.c | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/mm/swapfile.c b/mm/swapfile.c >> index 95b63f69f388..332ccfc76142 100644 >> --- a/mm/swapfile.c >> +++ b/mm/swapfile.c >> @@ -1783,7 +1783,7 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, >> { >> struct page *swapcache; >> spinlock_t *ptl; >> - pte_t *pte; >> + pte_t *pte, new_pte; >> int ret = 1; >> >> swapcache = page; >> @@ -1832,8 +1832,14 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, >> page_add_new_anon_rmap(page, vma, addr); >> lru_cache_add_inactive_or_unevictable(page, vma); >> } >> - set_pte_at(vma->vm_mm, addr, pte, >> - pte_mkold(mk_pte(page, vma->vm_page_prot))); >> + new_pte = pte_mkold(mk_pte(page, vma->vm_page_prot)); >> + if (pte_swp_soft_dirty(*pte)) >> + new_pte = pte_mksoft_dirty(new_pte); >> + if (pte_swp_uffd_wp(*pte)) { >> + new_pte = pte_mkuffd_wp(new_pte); >> + new_pte = pte_wrprotect(new_pte); > > The wrprotect shouldn't be necessary, we don't do a pte_mkwrite(). Note > that in do_swap_page() we might have done a > maybe_mkwrite(pte_mkdirty(pte)), which is why the pte_wrprotect() is > required there. You're so smart. I happened to be referring to the code in do_swap_page. ;) Now I see why pte_wrprotect() is only required there. Will remove it in the next verison when there is enough feedback. Many thanks! >
diff --git a/mm/swapfile.c b/mm/swapfile.c index 95b63f69f388..332ccfc76142 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -1783,7 +1783,7 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, { struct page *swapcache; spinlock_t *ptl; - pte_t *pte; + pte_t *pte, new_pte; int ret = 1; swapcache = page; @@ -1832,8 +1832,14 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd, page_add_new_anon_rmap(page, vma, addr); lru_cache_add_inactive_or_unevictable(page, vma); } - set_pte_at(vma->vm_mm, addr, pte, - pte_mkold(mk_pte(page, vma->vm_page_prot))); + new_pte = pte_mkold(mk_pte(page, vma->vm_page_prot)); + if (pte_swp_soft_dirty(*pte)) + new_pte = pte_mksoft_dirty(new_pte); + if (pte_swp_uffd_wp(*pte)) { + new_pte = pte_mkuffd_wp(new_pte); + new_pte = pte_wrprotect(new_pte); + } + set_pte_at(vma->vm_mm, addr, pte, new_pte); swap_free(entry); out: pte_unmap_unlock(pte, ptl);
This is observed by code review only but not any real report. When we turn off swapping we could have lost the bits stored in the swap ptes. The new rmap-exclusive bit is fine since that turned into a page flag, but not for soft-dirty and uffd-wp. Add them. Suggested-by: Peter Xu <peterx@redhat.com> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/swapfile.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)