Use kmap_local_folio() instead of kmap_local_page().
Replaces 2 calls to compound_head() from unmap_and_put_page() with one.
This prepares us for the removal of unmap_and_put_page(), and helps
prepare for the eventual gup folio conversions since this function
now supports individual subpages from large folios.
Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
mm/memory.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 0f9b32a20e5b..747866060658 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6820,9 +6820,10 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr,
}
while (len) {
- int bytes, offset, retval;
+ int bytes, folio_offset, page_offset retval;
void *maddr;
struct page *page;
+ struct folio *folio;
struct vm_area_struct *vma = NULL;
page = get_user_page_vma_remote(mm, addr, gup_flags, &vma);
@@ -6837,17 +6838,20 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr,
goto out;
}
+ folio = page_folio(page);
bytes = len;
- offset = addr & (PAGE_SIZE - 1);
- if (bytes > PAGE_SIZE - offset)
- bytes = PAGE_SIZE - offset;
+ folio_offset = offset_in_folio(folio, addr);
+ page_offset = offset_in_page(folio_offset);
+
+ if (bytes > PAGE_SIZE - page_offset)
+ bytes = PAGE_SIZE - page_offset;
- maddr = kmap_local_page(page);
- retval = strscpy(buf, maddr + offset, bytes);
+ maddr = kmap_local_folio(folio, folio_offset);
+ retval = strscpy(buf, maddr, bytes);
if (retval >= 0) {
/* Found the end of the string */
buf += retval;
- unmap_and_put_page(page, maddr);
+ folio_release_kmap(folio, maddr);
break;
}
@@ -6859,13 +6863,16 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr,
*/
if (bytes != len) {
addr += bytes - 1;
- copy_from_user_page(vma, page, addr, buf, maddr + (PAGE_SIZE - 1), 1);
+ copy_from_user_page(vma,
+ folio_page(folio, folio_offset / PAGE_SIZE),
+ addr, buf,
+ maddr + (PAGE_SIZE - page_offset - 1), 1);
buf += 1;
addr += 1;
}
len -= bytes;
- unmap_and_put_page(page, maddr);
+ folio_release_kmap(folio, maddr);
}
out:
--
2.49.0
On Wed, Jun 25, 2025 at 10:48:39AM -0700, Vishal Moola (Oracle) wrote: > +++ b/mm/memory.c > @@ -6820,9 +6820,10 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, > } > > while (len) { > - int bytes, offset, retval; > + int bytes, folio_offset, page_offset retval; offset_in_folio() returns a size_t so that we can support folios larger than 2GB (which is a real possibility here; hugetlbfs might end up with a 16GB folio on some architectures). > @@ -6837,17 +6838,20 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, > goto out; > } > > + folio = page_folio(page); > bytes = len; > - offset = addr & (PAGE_SIZE - 1); > - if (bytes > PAGE_SIZE - offset) > - bytes = PAGE_SIZE - offset; > + folio_offset = offset_in_folio(folio, addr); Umm. Not sure this is safe. A folio might be mapped misaligned, so 'addr' might not give you the right offset within the folio. I think you might need to use addr - (vma->vm_pgoff << PAGE_SHIFT). But I'd defer to others here ... particularly when it comes to anonymous folios.
On Wed, Jun 25, 2025 at 07:00:22PM +0100, Matthew Wilcox wrote: > > + folio_offset = offset_in_folio(folio, addr); > > Umm. Not sure this is safe. A folio might be mapped misaligned, so > 'addr' might not give you the right offset within the folio. I think > you might need to use addr - (vma->vm_pgoff << PAGE_SHIFT). But I'd > defer to others here ... particularly when it comes to anonymous folios. Sorry, this calculation is obviously wrong. It should be something like the calculation in linear_page_index(), only without throwing away the bottom PAGE_SHIFT bits. But that's for file VMAs only, and I'm not sure what should be done for anon vmas. Possibly there is no way?
On 26.06.25 05:02, Matthew Wilcox wrote: > On Wed, Jun 25, 2025 at 07:00:22PM +0100, Matthew Wilcox wrote: >>> + folio_offset = offset_in_folio(folio, addr); >> >> Umm. Not sure this is safe. A folio might be mapped misaligned, so >> 'addr' might not give you the right offset within the folio. I think >> you might need to use addr - (vma->vm_pgoff << PAGE_SHIFT). But I'd >> defer to others here ... particularly when it comes to anonymous folios. > > Sorry, this calculation is obviously wrong. It should be something like > the calculation in linear_page_index(), only without throwing away the > bottom PAGE_SHIFT bits. But that's for file VMAs only, and I'm not sure > what should be done for anon vmas. Possibly there is no way? In __folio_set_anon(), we set folio->index = linear_page_index(vma, address); and in __page_check_anon_rmap, we check VM_BUG_ON_PAGE(page_pgoff(folio, page) != linear_page_index(vma, address), So given the VMA, addr, folio, we should be able to figure out the offset into the folio. ... but I didn't have my second coffee yet. -- Cheers, David / dhildenb
On Wed, Jun 25, 2025 at 07:00:22PM +0100, Matthew Wilcox wrote: > On Wed, Jun 25, 2025 at 10:48:39AM -0700, Vishal Moola (Oracle) wrote: > > +++ b/mm/memory.c > > @@ -6820,9 +6820,10 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, > > } > > > > while (len) { > > - int bytes, offset, retval; > > + int bytes, folio_offset, page_offset retval; > > offset_in_folio() returns a size_t so that we can support folios larger > than 2GB (which is a real possibility here; hugetlbfs might end up with > a 16GB folio on some architectures). Got it, I'll change that. > > @@ -6837,17 +6838,20 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, > > goto out; > > } > > > > + folio = page_folio(page); > > bytes = len; > > - offset = addr & (PAGE_SIZE - 1); > > - if (bytes > PAGE_SIZE - offset) > > - bytes = PAGE_SIZE - offset; > > + folio_offset = offset_in_folio(folio, addr); > > Umm. Not sure this is safe. A folio might be mapped misaligned, so > 'addr' might not give you the right offset within the folio. I think > you might need to use addr - (vma->vm_pgoff << PAGE_SHIFT). But I'd > defer to others here ... particularly when it comes to anonymous folios. Ah ok, I didn't realize folios could be misaligned. I'll play around with your proposed calculation.
On 25.06.25 20:00, Matthew Wilcox wrote: > On Wed, Jun 25, 2025 at 10:48:39AM -0700, Vishal Moola (Oracle) wrote: >> +++ b/mm/memory.c >> @@ -6820,9 +6820,10 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, >> } >> >> while (len) { >> - int bytes, offset, retval; >> + int bytes, folio_offset, page_offset retval; > > offset_in_folio() returns a size_t so that we can support folios larger > than 2GB (which is a real possibility here; hugetlbfs might end up with > a 16GB folio on some architectures). > >> @@ -6837,17 +6838,20 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, >> goto out; >> } >> >> + folio = page_folio(page); >> bytes = len; >> - offset = addr & (PAGE_SIZE - 1); >> - if (bytes > PAGE_SIZE - offset) >> - bytes = PAGE_SIZE - offset; >> + folio_offset = offset_in_folio(folio, addr); > > Umm. Not sure this is safe. A folio might be mapped misaligned, so > 'addr' might not give you the right offset within the folio. I think > you might need to use addr - (vma->vm_pgoff << PAGE_SHIFT). But I'd > defer to others here ... particularly when it comes to anonymous folios. Not special to anon memory I think ... :) Only the offset within a page is okay to derive (existing code). -- Cheers, David / dhildenb
© 2016 - 2025 Red Hat, Inc.