linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/7] Cleanup free_pages() misuse
@ 2025-09-03 18:59 Vishal Moola (Oracle)
  2025-09-03 18:59 ` [PATCH v3 1/7] mm/page_alloc: Add kernel-docs for free_pages() Vishal Moola (Oracle)
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Vishal Moola (Oracle) @ 2025-09-03 18:59 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Vishal Moola (Oracle), Justin Sanders, Jens Axboe, Dave Hansen,
	Andy Lutomirski, Peter Zijlstra, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Madhavan Srinivasan, Michael Ellerman, Catalin Marinas,
	Will Deacon, Michael S. Tsirkin, David Hildenbrand

free_pages() is supposed to be called when we only have a virtual address.
__free_pages() is supposed to be called when we have a page.

There are a number of callers that use page_address() to get a page's
virtual address then call free_pages() on it when they should just call
__free_pages() directly.

Add kernel-docs for free_pages() to help callers better understand which
function they should be calling, and replace the obvious cases of
misuse.

-----------------
Based on mm-new, I intend to have all of these taken through the mm tree.

I've split the patches into separate subsystems to make it easier to
resolve conflicts, but there aren't any functional changes.

v3:
  - Collect some Reviewed-by Tags
  - Replace remaining free_page() calls in patch 7 (all other patches
    are unchanged from v2)
  - Add all appropriate mailing lists that were missing from v2

v2:
  - Reference __get_free_pages() instead of alloc_pages() in the
  free_pages() kernel-doc
  - Get some Reviewed-by tags
  - cc the subsystem maintainers related to specific patches

Vishal Moola (Oracle) (7):
  mm/page_alloc: Add kernel-docs for free_pages()
  aoe: Stop calling page_address() in free_page()
  x86: Stop calling page_address() in free_pages()
  riscv: Stop calling page_address() in free_pages()
  powerpc: Stop calling page_address() in free_pages()
  arm64: Stop calling page_address() in free_pages()
  virtio_balloon: Stop calling page_address() in free_pages()

 arch/arm64/mm/mmu.c                      | 2 +-
 arch/powerpc/mm/book3s64/radix_pgtable.c | 2 +-
 arch/riscv/mm/init.c                     | 4 ++--
 arch/x86/mm/init_64.c                    | 2 +-
 arch/x86/platform/efi/memmap.c           | 2 +-
 drivers/block/aoe/aoecmd.c               | 2 +-
 drivers/virtio/virtio_balloon.c          | 8 +++-----
 mm/page_alloc.c                          | 9 +++++++++
 8 files changed, 19 insertions(+), 12 deletions(-)

-- 
2.51.0



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/7] mm/page_alloc: Add kernel-docs for free_pages()
  2025-09-03 18:59 [PATCH v3 0/7] Cleanup free_pages() misuse Vishal Moola (Oracle)
@ 2025-09-03 18:59 ` Vishal Moola (Oracle)
  2025-09-03 18:59 ` [PATCH v3 2/7] aoe: Stop calling page_address() in free_page() Vishal Moola (Oracle)
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Vishal Moola (Oracle) @ 2025-09-03 18:59 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Vishal Moola (Oracle), Matthew Wilcox (Oracle), SeongJae Park

Add kernel-docs to free_pages(). This will help callers understand when
to use it instead of __free_pages().

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Acked-by: SeongJae Park <sj@kernel.org>
---
 mm/page_alloc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c2a254d877f8..0277b86b62ac 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5269,6 +5269,15 @@ void free_pages_nolock(struct page *page, unsigned int order)
 	___free_pages(page, order, FPI_TRYLOCK);
 }
 
+/**
+ * free_pages - Free pages allocated with __get_free_pages().
+ * @addr: The virtual address tied to a page returned from __get_free_pages().
+ * @order: The order of the allocation.
+ *
+ * This function behaves the same as __free_pages(). Use this function
+ * to free pages when you only have a valid virtual address. If you have
+ * the page, call __free_pages() instead.
+ */
 void free_pages(unsigned long addr, unsigned int order)
 {
 	if (addr != 0) {
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/7] aoe: Stop calling page_address() in free_page()
  2025-09-03 18:59 [PATCH v3 0/7] Cleanup free_pages() misuse Vishal Moola (Oracle)
  2025-09-03 18:59 ` [PATCH v3 1/7] mm/page_alloc: Add kernel-docs for free_pages() Vishal Moola (Oracle)
@ 2025-09-03 18:59 ` Vishal Moola (Oracle)
  2025-09-03 18:59 ` [PATCH v3 3/7] x86: Stop calling page_address() in free_pages() Vishal Moola (Oracle)
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Vishal Moola (Oracle) @ 2025-09-03 18:59 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Vishal Moola (Oracle), Justin Sanders, Jens Axboe,
	Matthew Wilcox (Oracle)

free_page() should be used when we only have a virtual address. We
should call __free_page() directly on our page instead.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 drivers/block/aoe/aoecmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index 6298f8e271e3..a9affb7c264d 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -1761,6 +1761,6 @@ aoecmd_exit(void)
 	kfree(kts);
 	kfree(ktiowq);
 
-	free_page((unsigned long) page_address(empty_page));
+	__free_page(empty_page);
 	empty_page = NULL;
 }
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/7] x86: Stop calling page_address() in free_pages()
  2025-09-03 18:59 [PATCH v3 0/7] Cleanup free_pages() misuse Vishal Moola (Oracle)
  2025-09-03 18:59 ` [PATCH v3 1/7] mm/page_alloc: Add kernel-docs for free_pages() Vishal Moola (Oracle)
  2025-09-03 18:59 ` [PATCH v3 2/7] aoe: Stop calling page_address() in free_page() Vishal Moola (Oracle)
@ 2025-09-03 18:59 ` Vishal Moola (Oracle)
  2025-09-04 11:51   ` Mike Rapoport
  2025-09-03 18:59 ` [PATCH v3 4/7] riscv: " Vishal Moola (Oracle)
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Vishal Moola (Oracle) @ 2025-09-03 18:59 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Vishal Moola (Oracle), Dave Hansen, Andy Lutomirski,
	Peter Zijlstra

free_pages() should be used when we only have a virtual address. We
should call __free_pages() directly on our page instead.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
---
 arch/x86/mm/init_64.c          | 2 +-
 arch/x86/platform/efi/memmap.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index b9426fce5f3e..0e4270e20fad 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1031,7 +1031,7 @@ static void __meminit free_pagetable(struct page *page, int order)
 		free_reserved_pages(page, nr_pages);
 #endif
 	} else {
-		free_pages((unsigned long)page_address(page), order);
+		__free_pages(page, order);
 	}
 }
 
diff --git a/arch/x86/platform/efi/memmap.c b/arch/x86/platform/efi/memmap.c
index 061b8ecc71a1..023697c88910 100644
--- a/arch/x86/platform/efi/memmap.c
+++ b/arch/x86/platform/efi/memmap.c
@@ -42,7 +42,7 @@ void __init __efi_memmap_free(u64 phys, unsigned long size, unsigned long flags)
 		struct page *p = pfn_to_page(PHYS_PFN(phys));
 		unsigned int order = get_order(size);
 
-		free_pages((unsigned long) page_address(p), order);
+		__free_pages(p, order);
 	}
 }
 
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 4/7] riscv: Stop calling page_address() in free_pages()
  2025-09-03 18:59 [PATCH v3 0/7] Cleanup free_pages() misuse Vishal Moola (Oracle)
                   ` (2 preceding siblings ...)
  2025-09-03 18:59 ` [PATCH v3 3/7] x86: Stop calling page_address() in free_pages() Vishal Moola (Oracle)
@ 2025-09-03 18:59 ` Vishal Moola (Oracle)
  2025-09-04 12:27   ` Alexandre Ghiti
  2025-09-03 18:59 ` [PATCH v3 5/7] powerpc: " Vishal Moola (Oracle)
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Vishal Moola (Oracle) @ 2025-09-03 18:59 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Vishal Moola (Oracle), Paul Walmsley, Palmer Dabbelt, Albert Ou

free_pages() should be used when we only have a virtual address. We
should call __free_pages() directly on our page instead.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 arch/riscv/mm/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 15683ae13fa5..1056c11d3251 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1624,7 +1624,7 @@ static void __meminit free_pud_table(pud_t *pud_start, p4d_t *p4d)
 	if (PageReserved(page))
 		free_reserved_page(page);
 	else
-		free_pages((unsigned long)page_address(page), 0);
+		__free_pages(page, 0);
 	p4d_clear(p4d);
 }
 
@@ -1646,7 +1646,7 @@ static void __meminit free_vmemmap_storage(struct page *page, size_t size,
 		return;
 	}
 
-	free_pages((unsigned long)page_address(page), order);
+	__free_pages(page, order);
 }
 
 static void __meminit remove_pte_mapping(pte_t *pte_base, unsigned long addr, unsigned long end,
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 5/7] powerpc: Stop calling page_address() in free_pages()
  2025-09-03 18:59 [PATCH v3 0/7] Cleanup free_pages() misuse Vishal Moola (Oracle)
                   ` (3 preceding siblings ...)
  2025-09-03 18:59 ` [PATCH v3 4/7] riscv: " Vishal Moola (Oracle)
@ 2025-09-03 18:59 ` Vishal Moola (Oracle)
  2025-09-04  7:10   ` Christophe Leroy
  2025-09-03 18:59 ` [PATCH v3 6/7] arm64: " Vishal Moola (Oracle)
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Vishal Moola (Oracle) @ 2025-09-03 18:59 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Vishal Moola (Oracle), Madhavan Srinivasan, Michael Ellerman,
	Ritesh Harjani (IBM)

free_pages() should be used when we only have a virtual address. We
should call __free_pages() directly on our page instead.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 arch/powerpc/mm/book3s64/radix_pgtable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index be523e5fe9c5..73977dbabcf2 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -780,7 +780,7 @@ static void __meminit free_vmemmap_pages(struct page *page,
 		while (nr_pages--)
 			free_reserved_page(page++);
 	} else
-		free_pages((unsigned long)page_address(page), order);
+		__free_pages(page, order);
 }
 
 static void __meminit remove_pte_table(pte_t *pte_start, unsigned long addr,
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 6/7] arm64: Stop calling page_address() in free_pages()
  2025-09-03 18:59 [PATCH v3 0/7] Cleanup free_pages() misuse Vishal Moola (Oracle)
                   ` (4 preceding siblings ...)
  2025-09-03 18:59 ` [PATCH v3 5/7] powerpc: " Vishal Moola (Oracle)
@ 2025-09-03 18:59 ` Vishal Moola (Oracle)
  2025-09-03 18:59 ` [PATCH v3 7/7] virtio_balloon: " Vishal Moola (Oracle)
  2025-09-04 11:55 ` [PATCH v3 0/7] Cleanup free_pages() misuse Mike Rapoport
  7 siblings, 0 replies; 16+ messages in thread
From: Vishal Moola (Oracle) @ 2025-09-03 18:59 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Vishal Moola (Oracle), Catalin Marinas, Will Deacon

free_pages() should be used when we only have a virtual address. We
should call __free_pages() directly on our page instead.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/mm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 34e5d78af076..e14a75d0dbd3 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -843,7 +843,7 @@ static void free_hotplug_page_range(struct page *page, size_t size,
 		vmem_altmap_free(altmap, size >> PAGE_SHIFT);
 	} else {
 		WARN_ON(PageReserved(page));
-		free_pages((unsigned long)page_address(page), get_order(size));
+		__free_pages(page, get_order(size));
 	}
 }
 
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 7/7] virtio_balloon: Stop calling page_address() in free_pages()
  2025-09-03 18:59 [PATCH v3 0/7] Cleanup free_pages() misuse Vishal Moola (Oracle)
                   ` (5 preceding siblings ...)
  2025-09-03 18:59 ` [PATCH v3 6/7] arm64: " Vishal Moola (Oracle)
@ 2025-09-03 18:59 ` Vishal Moola (Oracle)
  2025-09-03 19:13   ` David Hildenbrand
  2025-09-04 21:38   ` Michael S. Tsirkin
  2025-09-04 11:55 ` [PATCH v3 0/7] Cleanup free_pages() misuse Mike Rapoport
  7 siblings, 2 replies; 16+ messages in thread
From: Vishal Moola (Oracle) @ 2025-09-03 18:59 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Vishal Moola (Oracle), Michael S. Tsirkin, David Hildenbrand

free_pages() should be used when we only have a virtual address. We
should call __free_pages() directly on our page instead.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 drivers/virtio/virtio_balloon.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index eae65136cdfb..7f3fd72678eb 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -488,8 +488,7 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb,
 		page = balloon_page_pop(&vb->free_page_list);
 		if (!page)
 			break;
-		free_pages((unsigned long)page_address(page),
-			   VIRTIO_BALLOON_HINT_BLOCK_ORDER);
+		__free_pages(page, VIRTIO_BALLOON_HINT_BLOCK_ORDER);
 	}
 	vb->num_free_page_blocks -= num_returned;
 	spin_unlock_irq(&vb->free_page_list_lock);
@@ -719,8 +718,7 @@ static int get_free_page_and_send(struct virtio_balloon *vb)
 	if (vq->num_free > 1) {
 		err = virtqueue_add_inbuf(vq, &sg, 1, p, GFP_KERNEL);
 		if (unlikely(err)) {
-			free_pages((unsigned long)p,
-				   VIRTIO_BALLOON_HINT_BLOCK_ORDER);
+			__free_pages(page, VIRTIO_BALLOON_HINT_BLOCK_ORDER);
 			return err;
 		}
 		virtqueue_kick(vq);
@@ -733,7 +731,7 @@ static int get_free_page_and_send(struct virtio_balloon *vb)
 		 * The vq has no available entry to add this page block, so
 		 * just free it.
 		 */
-		free_pages((unsigned long)p, VIRTIO_BALLOON_HINT_BLOCK_ORDER);
+		__free_pages(page, VIRTIO_BALLOON_HINT_BLOCK_ORDER);
 	}
 
 	return 0;
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 7/7] virtio_balloon: Stop calling page_address() in free_pages()
  2025-09-03 18:59 ` [PATCH v3 7/7] virtio_balloon: " Vishal Moola (Oracle)
@ 2025-09-03 19:13   ` David Hildenbrand
  2025-09-04 21:38   ` Michael S. Tsirkin
  1 sibling, 0 replies; 16+ messages in thread
From: David Hildenbrand @ 2025-09-03 19:13 UTC (permalink / raw)
  To: Vishal Moola (Oracle), linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Michael S. Tsirkin

On 03.09.25 20:59, Vishal Moola (Oracle) wrote:
> free_pages() should be used when we only have a virtual address. We
> should call __free_pages() directly on our page instead.
> 
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/7] powerpc: Stop calling page_address() in free_pages()
  2025-09-03 18:59 ` [PATCH v3 5/7] powerpc: " Vishal Moola (Oracle)
@ 2025-09-04  7:10   ` Christophe Leroy
  0 siblings, 0 replies; 16+ messages in thread
From: Christophe Leroy @ 2025-09-04  7:10 UTC (permalink / raw)
  To: Vishal Moola (Oracle), linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Madhavan Srinivasan, Michael Ellerman, Ritesh Harjani (IBM)



Le 03/09/2025 à 20:59, Vishal Moola (Oracle) a écrit :
> free_pages() should be used when we only have a virtual address. We
> should call __free_pages() directly on our page instead.
> 
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/mm/book3s64/radix_pgtable.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index be523e5fe9c5..73977dbabcf2 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -780,7 +780,7 @@ static void __meminit free_vmemmap_pages(struct page *page,
>   		while (nr_pages--)
>   			free_reserved_page(page++);
>   	} else
> -		free_pages((unsigned long)page_address(page), order);
> +		__free_pages(page, order);
>   }
>   
>   static void __meminit remove_pte_table(pte_t *pte_start, unsigned long addr,



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/7] x86: Stop calling page_address() in free_pages()
  2025-09-03 18:59 ` [PATCH v3 3/7] x86: Stop calling page_address() in free_pages() Vishal Moola (Oracle)
@ 2025-09-04 11:51   ` Mike Rapoport
  2025-09-04 11:54     ` Mike Rapoport
  0 siblings, 1 reply; 16+ messages in thread
From: Mike Rapoport @ 2025-09-04 11:51 UTC (permalink / raw)
  To: Vishal Moola (Oracle)
  Cc: linux-mm, linux-kernel, Andrew Morton, linux-block,
	linux-arm-kernel, linuxppc-dev, linux-riscv, linux-efi,
	virtualization, Dave Hansen, Andy Lutomirski, Peter Zijlstra

On Wed, Sep 03, 2025 at 11:59:17AM -0700, Vishal Moola (Oracle) wrote:
> free_pages() should be used when we only have a virtual address. We
> should call __free_pages() directly on our page instead.
> 
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> ---
>  arch/x86/mm/init_64.c          | 2 +-
>  arch/x86/platform/efi/memmap.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index b9426fce5f3e..0e4270e20fad 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -1031,7 +1031,7 @@ static void __meminit free_pagetable(struct page *page, int order)
>  		free_reserved_pages(page, nr_pages);
>  #endif
>  	} else {
> -		free_pages((unsigned long)page_address(page), order);
> +		__free_pages(page, order);
>  	}
>  }
>  
> diff --git a/arch/x86/platform/efi/memmap.c b/arch/x86/platform/efi/memmap.c
> index 061b8ecc71a1..023697c88910 100644
> --- a/arch/x86/platform/efi/memmap.c
> +++ b/arch/x86/platform/efi/memmap.c
> @@ -42,7 +42,7 @@ void __init __efi_memmap_free(u64 phys, unsigned long size, unsigned long flags)
>  		struct page *p = pfn_to_page(PHYS_PFN(phys));
>  		unsigned int order = get_order(size);
>  
> -		free_pages((unsigned long) page_address(p), order);

Could be just free_pages((unsigned long)phys_to_virt(phys), order), then
the page is not needed at all.

> +		__free_pages(p, order);
>  	}
>  }
>  
> -- 
> 2.51.0
> 
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/7] x86: Stop calling page_address() in free_pages()
  2025-09-04 11:51   ` Mike Rapoport
@ 2025-09-04 11:54     ` Mike Rapoport
  2025-09-05 18:02       ` Vishal Moola (Oracle)
  0 siblings, 1 reply; 16+ messages in thread
From: Mike Rapoport @ 2025-09-04 11:54 UTC (permalink / raw)
  To: Vishal Moola (Oracle)
  Cc: linux-mm, linux-kernel, Andrew Morton, linux-block,
	linux-arm-kernel, linuxppc-dev, linux-riscv, linux-efi,
	virtualization, Dave Hansen, Andy Lutomirski, Peter Zijlstra

On Thu, Sep 04, 2025 at 02:51:14PM +0300, Mike Rapoport wrote:
> On Wed, Sep 03, 2025 at 11:59:17AM -0700, Vishal Moola (Oracle) wrote:
> > free_pages() should be used when we only have a virtual address. We
> > should call __free_pages() directly on our page instead.
> > 
> > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> > Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> > ---
> >  arch/x86/mm/init_64.c          | 2 +-
> >  arch/x86/platform/efi/memmap.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> > index b9426fce5f3e..0e4270e20fad 100644
> > --- a/arch/x86/mm/init_64.c
> > +++ b/arch/x86/mm/init_64.c
> > @@ -1031,7 +1031,7 @@ static void __meminit free_pagetable(struct page *page, int order)
> >  		free_reserved_pages(page, nr_pages);
> >  #endif
> >  	} else {
> > -		free_pages((unsigned long)page_address(page), order);
> > +		__free_pages(page, order);
> >  	}
> >  }
> >  
> > diff --git a/arch/x86/platform/efi/memmap.c b/arch/x86/platform/efi/memmap.c
> > index 061b8ecc71a1..023697c88910 100644
> > --- a/arch/x86/platform/efi/memmap.c
> > +++ b/arch/x86/platform/efi/memmap.c
> > @@ -42,7 +42,7 @@ void __init __efi_memmap_free(u64 phys, unsigned long size, unsigned long flags)
> >  		struct page *p = pfn_to_page(PHYS_PFN(phys));
> >  		unsigned int order = get_order(size);
> >  
> > -		free_pages((unsigned long) page_address(p), order);
> 
> Could be just free_pages((unsigned long)phys_to_virt(phys), order), then
> the page is not needed at all.

Or even __free_pages(phys_to_page(phys), order);
 
> > +		__free_pages(p, order);
> >  	}
> >  }
> >  
> > -- 
> > 2.51.0
> > 
> > 
> 
> -- 
> Sincerely yours,
> Mike.

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/7] Cleanup free_pages() misuse
  2025-09-03 18:59 [PATCH v3 0/7] Cleanup free_pages() misuse Vishal Moola (Oracle)
                   ` (6 preceding siblings ...)
  2025-09-03 18:59 ` [PATCH v3 7/7] virtio_balloon: " Vishal Moola (Oracle)
@ 2025-09-04 11:55 ` Mike Rapoport
  7 siblings, 0 replies; 16+ messages in thread
From: Mike Rapoport @ 2025-09-04 11:55 UTC (permalink / raw)
  To: Vishal Moola (Oracle)
  Cc: linux-mm, linux-kernel, Andrew Morton, linux-block,
	linux-arm-kernel, linuxppc-dev, linux-riscv, linux-efi,
	virtualization, Justin Sanders, Jens Axboe, Dave Hansen,
	Andy Lutomirski, Peter Zijlstra, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Madhavan Srinivasan, Michael Ellerman, Catalin Marinas,
	Will Deacon, Michael S. Tsirkin, David Hildenbrand

On Wed, Sep 03, 2025 at 11:59:14AM -0700, Vishal Moola (Oracle) wrote:
> free_pages() is supposed to be called when we only have a virtual address.
> __free_pages() is supposed to be called when we have a page.
> 
> There are a number of callers that use page_address() to get a page's
> virtual address then call free_pages() on it when they should just call
> __free_pages() directly.
> 
> Add kernel-docs for free_pages() to help callers better understand which
> function they should be calling, and replace the obvious cases of
> misuse.
> 
> Vishal Moola (Oracle) (7):
>   mm/page_alloc: Add kernel-docs for free_pages()
>   aoe: Stop calling page_address() in free_page()
>   x86: Stop calling page_address() in free_pages()
>   riscv: Stop calling page_address() in free_pages()
>   powerpc: Stop calling page_address() in free_pages()
>   arm64: Stop calling page_address() in free_pages()
>   virtio_balloon: Stop calling page_address() in free_pages()

Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

>  arch/arm64/mm/mmu.c                      | 2 +-
>  arch/powerpc/mm/book3s64/radix_pgtable.c | 2 +-
>  arch/riscv/mm/init.c                     | 4 ++--
>  arch/x86/mm/init_64.c                    | 2 +-
>  arch/x86/platform/efi/memmap.c           | 2 +-
>  drivers/block/aoe/aoecmd.c               | 2 +-
>  drivers/virtio/virtio_balloon.c          | 8 +++-----
>  mm/page_alloc.c                          | 9 +++++++++
>  8 files changed, 19 insertions(+), 12 deletions(-)
> 
> -- 
> 2.51.0
> 
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/7] riscv: Stop calling page_address() in free_pages()
  2025-09-03 18:59 ` [PATCH v3 4/7] riscv: " Vishal Moola (Oracle)
@ 2025-09-04 12:27   ` Alexandre Ghiti
  0 siblings, 0 replies; 16+ messages in thread
From: Alexandre Ghiti @ 2025-09-04 12:27 UTC (permalink / raw)
  To: Vishal Moola (Oracle), linux-mm
  Cc: linux-kernel, Andrew Morton, linux-block, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-efi, virtualization,
	Paul Walmsley, Palmer Dabbelt, Albert Ou

Hi Vishal,

On 9/3/25 20:59, Vishal Moola (Oracle) wrote:
> free_pages() should be used when we only have a virtual address. We
> should call __free_pages() directly on our page instead.
>
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> ---
>   arch/riscv/mm/init.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 15683ae13fa5..1056c11d3251 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -1624,7 +1624,7 @@ static void __meminit free_pud_table(pud_t *pud_start, p4d_t *p4d)
>   	if (PageReserved(page))
>   		free_reserved_page(page);
>   	else
> -		free_pages((unsigned long)page_address(page), 0);
> +		__free_pages(page, 0);
>   	p4d_clear(p4d);
>   }
>   
> @@ -1646,7 +1646,7 @@ static void __meminit free_vmemmap_storage(struct page *page, size_t size,
>   		return;
>   	}
>   
> -	free_pages((unsigned long)page_address(page), order);
> +	__free_pages(page, order);
>   }
>   
>   static void __meminit remove_pte_mapping(pte_t *pte_base, unsigned long addr, unsigned long end,


Acked-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 7/7] virtio_balloon: Stop calling page_address() in free_pages()
  2025-09-03 18:59 ` [PATCH v3 7/7] virtio_balloon: " Vishal Moola (Oracle)
  2025-09-03 19:13   ` David Hildenbrand
@ 2025-09-04 21:38   ` Michael S. Tsirkin
  1 sibling, 0 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2025-09-04 21:38 UTC (permalink / raw)
  To: Vishal Moola (Oracle)
  Cc: linux-mm, linux-kernel, Andrew Morton, linux-block,
	linux-arm-kernel, linuxppc-dev, linux-riscv, linux-efi,
	virtualization, David Hildenbrand

On Wed, Sep 03, 2025 at 11:59:21AM -0700, Vishal Moola (Oracle) wrote:
> free_pages() should be used when we only have a virtual address. We
> should call __free_pages() directly on our page instead.
> 
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/virtio/virtio_balloon.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index eae65136cdfb..7f3fd72678eb 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -488,8 +488,7 @@ static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb,
>  		page = balloon_page_pop(&vb->free_page_list);
>  		if (!page)
>  			break;
> -		free_pages((unsigned long)page_address(page),
> -			   VIRTIO_BALLOON_HINT_BLOCK_ORDER);
> +		__free_pages(page, VIRTIO_BALLOON_HINT_BLOCK_ORDER);
>  	}
>  	vb->num_free_page_blocks -= num_returned;
>  	spin_unlock_irq(&vb->free_page_list_lock);
> @@ -719,8 +718,7 @@ static int get_free_page_and_send(struct virtio_balloon *vb)
>  	if (vq->num_free > 1) {
>  		err = virtqueue_add_inbuf(vq, &sg, 1, p, GFP_KERNEL);
>  		if (unlikely(err)) {
> -			free_pages((unsigned long)p,
> -				   VIRTIO_BALLOON_HINT_BLOCK_ORDER);
> +			__free_pages(page, VIRTIO_BALLOON_HINT_BLOCK_ORDER);
>  			return err;
>  		}
>  		virtqueue_kick(vq);
> @@ -733,7 +731,7 @@ static int get_free_page_and_send(struct virtio_balloon *vb)
>  		 * The vq has no available entry to add this page block, so
>  		 * just free it.
>  		 */
> -		free_pages((unsigned long)p, VIRTIO_BALLOON_HINT_BLOCK_ORDER);
> +		__free_pages(page, VIRTIO_BALLOON_HINT_BLOCK_ORDER);
>  	}
>  
>  	return 0;
> -- 
> 2.51.0



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/7] x86: Stop calling page_address() in free_pages()
  2025-09-04 11:54     ` Mike Rapoport
@ 2025-09-05 18:02       ` Vishal Moola (Oracle)
  0 siblings, 0 replies; 16+ messages in thread
From: Vishal Moola (Oracle) @ 2025-09-05 18:02 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: linux-mm, linux-kernel, Andrew Morton, linux-block,
	linux-arm-kernel, linuxppc-dev, linux-riscv, linux-efi,
	virtualization, Dave Hansen, Andy Lutomirski, Peter Zijlstra

On Thu, Sep 04, 2025 at 02:54:24PM +0300, Mike Rapoport wrote:
> On Thu, Sep 04, 2025 at 02:51:14PM +0300, Mike Rapoport wrote:
> > On Wed, Sep 03, 2025 at 11:59:17AM -0700, Vishal Moola (Oracle) wrote:
> > > free_pages() should be used when we only have a virtual address. We
> > > should call __free_pages() directly on our page instead.
> > > 
> > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> > > Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> > > ---
> > >  arch/x86/mm/init_64.c          | 2 +-
> > >  arch/x86/platform/efi/memmap.c | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> > > index b9426fce5f3e..0e4270e20fad 100644
> > > --- a/arch/x86/mm/init_64.c
> > > +++ b/arch/x86/mm/init_64.c
> > > @@ -1031,7 +1031,7 @@ static void __meminit free_pagetable(struct page *page, int order)
> > >  		free_reserved_pages(page, nr_pages);
> > >  #endif
> > >  	} else {
> > > -		free_pages((unsigned long)page_address(page), order);
> > > +		__free_pages(page, order);
> > >  	}
> > >  }
> > >  
> > > diff --git a/arch/x86/platform/efi/memmap.c b/arch/x86/platform/efi/memmap.c
> > > index 061b8ecc71a1..023697c88910 100644
> > > --- a/arch/x86/platform/efi/memmap.c
> > > +++ b/arch/x86/platform/efi/memmap.c
> > > @@ -42,7 +42,7 @@ void __init __efi_memmap_free(u64 phys, unsigned long size, unsigned long flags)
> > >  		struct page *p = pfn_to_page(PHYS_PFN(phys));
> > >  		unsigned int order = get_order(size);
> > >  
> > > -		free_pages((unsigned long) page_address(p), order);
> > 
> > Could be just free_pages((unsigned long)phys_to_virt(phys), order), then
> > the page is not needed at all.
> 
> Or even __free_pages(phys_to_page(phys), order);

Right. It actually looks like we could inline this whole block if we
really wanted to...

__free_pages(phys_to_page(phys), get_order(size));

Should I send a fixup (or v4) with this change?


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-09-05 18:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-03 18:59 [PATCH v3 0/7] Cleanup free_pages() misuse Vishal Moola (Oracle)
2025-09-03 18:59 ` [PATCH v3 1/7] mm/page_alloc: Add kernel-docs for free_pages() Vishal Moola (Oracle)
2025-09-03 18:59 ` [PATCH v3 2/7] aoe: Stop calling page_address() in free_page() Vishal Moola (Oracle)
2025-09-03 18:59 ` [PATCH v3 3/7] x86: Stop calling page_address() in free_pages() Vishal Moola (Oracle)
2025-09-04 11:51   ` Mike Rapoport
2025-09-04 11:54     ` Mike Rapoport
2025-09-05 18:02       ` Vishal Moola (Oracle)
2025-09-03 18:59 ` [PATCH v3 4/7] riscv: " Vishal Moola (Oracle)
2025-09-04 12:27   ` Alexandre Ghiti
2025-09-03 18:59 ` [PATCH v3 5/7] powerpc: " Vishal Moola (Oracle)
2025-09-04  7:10   ` Christophe Leroy
2025-09-03 18:59 ` [PATCH v3 6/7] arm64: " Vishal Moola (Oracle)
2025-09-03 18:59 ` [PATCH v3 7/7] virtio_balloon: " Vishal Moola (Oracle)
2025-09-03 19:13   ` David Hildenbrand
2025-09-04 21:38   ` Michael S. Tsirkin
2025-09-04 11:55 ` [PATCH v3 0/7] Cleanup free_pages() misuse Mike Rapoport

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).