[RFC PATCH 0/3] Some remote_vm folio conversions

[RFC PATCH 0/3] Some remote_vm folio conversions

[Vishal Moola (Oracle)]

Both callers of unmap_and_put_page() are for tracing purposes and
interact with user vmas. In a memdesc world, these should end up
operating on folios, so convert them.

This patchset converts those functions so that gup - namely
get_user_page_vma_remote() - can eventually safely pass around
large folios.

This iteration does NOT allow support for handling a large folio all at
once, it merely makes working on subpages of a large folio safe. Does
anyone have any thoughts on if we might want to handle these in larger
than PAGE_SIZE chunks? It seems like a lot of work with how kmap()
works right now...

On a similar note, might we want to convert implementations of
copy_{to,from}_user_page() to pass around folios as well?

Vishal Moola (Oracle) (3):
  mm/memory.c: convert __copy_remote_vm_str() to folios
  mm/memory.c: convert __access_remote_vm() to folios
  mm: Remove unmap_and_put_page()

 include/linux/highmem.h |  6 -----
 mm/memory.c             | 52 +++++++++++++++++++++++++----------------
 2 files changed, 32 insertions(+), 26 deletions(-)

-- 
2.49.0

[RFC PATCH 1/3] mm/memory.c: convert __copy_remote_vm_str() to folios

[Vishal Moola (Oracle)]

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

Re: [RFC PATCH 1/3] mm/memory.c: convert __copy_remote_vm_str() to folios

[Matthew Wilcox]

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.

Re: [RFC PATCH 1/3] mm/memory.c: convert __copy_remote_vm_str() to folios

[David Hildenbrand]

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

Re: [RFC PATCH 1/3] mm/memory.c: convert __copy_remote_vm_str() to folios

[Vishal Moola (Oracle)]

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.

Re: [RFC PATCH 1/3] mm/memory.c: convert __copy_remote_vm_str() to folios

[Matthew Wilcox]

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?

Re: [RFC PATCH 1/3] mm/memory.c: convert __copy_remote_vm_str() to folios

[David Hildenbrand]

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

[RFC PATCH 2/3] mm/memory.c: convert __access_remote_vm() to folios

[Vishal Moola (Oracle)]

Use kmap_local_folio() instead of kmap_local_page().
Replaces 2 calls to compound_head() 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 | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 747866060658..5eeca95b9c61 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6696,8 +6696,9 @@ static int __access_remote_vm(struct mm_struct *mm, unsigned long addr,
 
 	/* ignore errors, just check how much was successfully transferred */
 	while (len) {
-		int bytes, offset;
+		int bytes, folio_offset;
 		void *maddr;
+		struct folio *folio;
 		struct vm_area_struct *vma = NULL;
 		struct page *page = get_user_page_vma_remote(mm, addr,
 							     gup_flags, &vma);
@@ -6729,21 +6730,25 @@ static int __access_remote_vm(struct mm_struct *mm, unsigned long addr,
 			if (bytes <= 0)
 				break;
 		} else {
+			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);
+
+			if (bytes > PAGE_SIZE - offset_in_page(folio_offset))
+				bytes = PAGE_SIZE - offset_in_page(folio_offset);
 
-			maddr = kmap_local_page(page);
+			maddr = kmap_local_folio(folio, folio_offset);
 			if (write) {
-				copy_to_user_page(vma, page, addr,
-						  maddr + offset, buf, bytes);
-				set_page_dirty_lock(page);
+				copy_to_user_page(vma,
+					folio_page(folio, folio_offset / PAGE_SIZE),
+					addr, maddr, buf, bytes);
+				folio_mark_dirty_lock(folio);
 			} else {
-				copy_from_user_page(vma, page, addr,
-						    buf, maddr + offset, bytes);
+				copy_from_user_page(vma,
+					folio_page(folio, folio_offset / PAGE_SIZE),
+					addr, buf, maddr, bytes);
 			}
-			unmap_and_put_page(page, maddr);
+			folio_release_kmap(folio, maddr);
 		}
 		len -= bytes;
 		buf += bytes;
-- 
2.49.0

[RFC PATCH 3/3] mm: Remove unmap_and_put_page()

[Vishal Moola (Oracle)]

There are no callers of unmap_and_put_page() left. Remove it.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 include/linux/highmem.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index a30526cc53a7..6234f316468c 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -682,10 +682,4 @@ static inline void folio_release_kmap(struct folio *folio, void *addr)
 	kunmap_local(addr);
 	folio_put(folio);
 }
-
-static inline void unmap_and_put_page(struct page *page, void *addr)
-{
-	folio_release_kmap(page_folio(page), addr);
-}
-
 #endif /* _LINUX_HIGHMEM_H */
-- 
2.49.0