Re: [PATCH 2/4] ice: use kzalloc() to allocate staging buffer for reading from GNSS

[Mike Rapoport <rppt@kernel.org>]

On Thu, Jul 02, 2026 at 03:49:36PM +0200, Przemek Kitszel wrote:
> On 7/1/26 15:57, Mike Rapoport (Microsoft) wrote:
> > ice_gnss_read() uses get_zeroed_page() to  allocate a staging buffer for
> > reading GNSS module data via I2C bus.
> > 
> > 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_zeroed_page() with kzalloc() and free_page() with
> > kfree().
> > 
> > Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > ---
> >   drivers/net/ethernet/intel/ice/ice_gnss.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_gnss.c b/drivers/net/ethernet/intel/ice/ice_gnss.c
> > index 8fd954f1ebd6..7d21c3417b0b 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_gnss.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_gnss.c
> > @@ -2,6 +2,7 @@
> >   /* Copyright (C) 2021-2022, Intel Corporation. */
> >   #include "ice.h"
> > +#include <linux/slab.h>
> >   #include "ice_lib.h"
> >   /**
> > @@ -124,7 +125,7 @@ static void ice_gnss_read(struct kthread_work *work)
> >   	data_len = min_t(typeof(data_len), data_len, PAGE_SIZE);
> > -	buf = (char *)get_zeroed_page(GFP_KERNEL);
> > +	buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> 
> nit:
> from the code it is clear that we read at most a page, and @data_len
> stores the actual amount needed
> 
> comment:
> I don't know why we limit to a page, it's outside of the scope of this
> series, but likely you have removed the limit (which will go into the
> loop - single AQ call is likely limited by a PAGE too).

Not sure I follow how the limit changed. buf remains PAGE_SIZE regardless
of allocation API.
 
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

Thanks!

-- 
Sincerely yours,
Mike.