Re: [PATCH v2 7/7] virtio_balloon: Stop calling page_address() in free_pages()

["Vishal Moola (Oracle)" <vishal.moola@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1700 bytes --]

On Wed, Aug 27, 2025 at 02:01:01PM +0200, David Hildenbrand wrote:
> On 26.08.25 22:56, 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>
> > ---
> >   drivers/virtio/virtio_balloon.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index eae65136cdfb..d4e6865ce355 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);
> 
> I think you missed another nastiness of similar kind in
> get_free_page_and_send() where we do
> 
> 	p = page_address(page);
> 
> Just to call
> 
> 	free_pages((unsigned long)p, VIRTIO_BALLOON_HINT_BLOCK_ORDER);

Thanks for catching that. Andrew can you fold the attached patch into
this one please? It looks like the page_address() call is needed for other
things, but since we're changing the file we might as well clean these
up as well.

I imagine theres more of these lingering in the kernel, but theres so
many callers and I only looked for the ones that were calling
page_address() inline :(.

[-- Attachment #2: 0001-virtio_ballon-Call-__free_pages-in-get_free_page_and.patch --]
[-- Type: text/plain, Size: 1413 bytes --]

From a7d439154c7990418da976e5864b91fce9d49d58 Mon Sep 17 00:00:00 2001
From: "Vishal Moola (Oracle)" <vishal.moola@gmail.com>
Date: Wed, 27 Aug 2025 11:10:22 -0700
Subject: [PATCH] virtio_ballon: Call __free_pages() in
 get_free_page_and_send()

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 | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index d4e6865ce355..7f3fd72678eb 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -718,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);
@@ -732,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