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

[Juergen Gross <jgross@xxxxxxxx>]

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@xxxxxxxxxx
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
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: OpenPGP_signature.asc
Description: OpenPGP digital signature