Re: [PATCH] xen/events: replace __get_free_pages() with kmalloc()

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2635 bytes --]

On 08.11.26 09:42, Mike Rapoport (Microsoft) wrote:
> set_evtchn_to_irq() allocates a page for the event channel to IRQ
> number software lookup table.
> 
> This buffer can be allocated with kmalloc() as there's nothing special
> about it to go directly to the page allocator.
> 
> kmalloc() provides a better API that does not require ugly casts and
> kfree() does not need to know the size of the freed object.
> 
> Performance difference between kmalloc() and __get_free_pages() is not
> measurable as both allocators take an object/page from a per-CPU list for
> fast path allocations.
> 
> For the slow path the performance is anyway determined by the amount of
> reclaim involved rather than by what allocator is used.
> 
> Replace use of __get_free_pages() with kmalloc() and free_pages() with
> kfree().
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Shouldn't you also replace __get_free_page() in xen_init_IRQ()? I don't see
why this is being omitted.


Juergen

> ---
> This is a (small) part of larger work of replacing page allocator calls
> with kmalloc.
> 
> Largely, anything that doesn't need struct page (or a memdesc in the
> future) should just use kmalloc() or kvmalloc() to allocate memory.
> Removing needless calls to page allocator helps with memdesc (aka project
> folio) conversion and gives a nicer API without casts.
> ---
>   drivers/xen/events/events_base.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index fd16d652c81f2..8c377493429b7 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -230,7 +230,7 @@ static int set_evtchn_to_irq(evtchn_port_t evtchn, unsigned int irq)
>   		if (irq == -1)
>   			return 0;
>   
> -		evtchn_row = (int *) __get_free_pages(GFP_KERNEL, 0);
> +		evtchn_row = kmalloc(PAGE_SIZE, GFP_KERNEL);
>   		if (evtchn_row == NULL)
>   			return -ENOMEM;
>   
> @@ -241,7 +241,7 @@ static int set_evtchn_to_irq(evtchn_port_t evtchn, unsigned int irq)
>   		 * thread was faster inserting it, we can drop ours.
>   		 */
>   		if (cmpxchg(&evtchn_to_irq[row], NULL, evtchn_row) != NULL)
> -			free_page((unsigned long) evtchn_row);
> +			kfree(evtchn_row);
>   	}
>   
>   	WRITE_ONCE(evtchn_to_irq[row][col], irq);
> 
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20260624-b4-xen-a1cd629ce9f4
> 
> --
> Sincerely yours,
> Mike.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]