Re: [PATCH 5/5] gpiolib: avoid krealloc() in gpiod_get_array()

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, Oct 21, 2026 at 12:02:10PM -0700, Dmitry Torokhov wrote:
> Instead of allocating descs before the loop and reallocating it via
> krealloc() when candidate hardware array matching is detected on the
> first iteration, defer allocation of descs until the first descriptor
> is retrieved.
> 
> Factor descriptor allocation and fast-path validation into helper
> functions gpio_descs_alloc() and gpio_descs_check_fastpath(). Inside
> the loop, acquire the descriptor using __free(gpiod_put), allocate
> descs once on the first iteration, check fast-path candidate
> suitability, and transfer descriptor ownership using no_free_ptr().

...

> +static struct gpio_descs *gpio_descs_alloc(unsigned int count, struct gpio_desc *first_desc)
> +{
> +	struct gpio_device *gdev = gpiod_to_gpio_device(first_desc);
> +	struct gpio_array *array_info;
> +	struct gpio_descs *descs;
> +	size_t descs_size, alloc_size;
> +	unsigned int bitmap_size;
> +
> +	/*
> +	 * If pin hardware number of array member 0 is also 0, select
> +	 * its chip as a candidate for fast bitmap processing path.
> +	 */
> +	if (gpiod_hwgpio(first_desc) != 0)
> +		return kzalloc_flex(*descs, desc, count);
> +
> +	descs_size = struct_size(descs, desc, count);
> +	bitmap_size = BITS_TO_LONGS(max(gdev->ngpio, count));
> +	alloc_size = descs_size + struct_size(array_info, invert_mask, 3 * bitmap_size);
> +
> +	descs = kzalloc(alloc_size, GFP_KERNEL);
> +	if (!descs)
> +		return NULL;
> +
> +	array_info = (void *)descs + descs_size;

> +	array_info->get_mask = array_info->invert_mask + bitmap_size;
> +	array_info->set_mask = array_info->get_mask + bitmap_size;

> +	array_info->desc = descs->desc;
> +	array_info->size = count;
> +	array_info->gdev = gdev;
> +	bitmap_set(array_info->get_mask, 0, count);
> +	bitmap_set(array_info->set_mask, 0, count);

With the above being done, I would rather see bitmap_zalloc() for bitmaps.

> +	descs->info = array_info;
> +
> +	return descs;
> +}

...

>  	count = gpiod_count(dev, con_id);
> -	if (count < 0)
> -		return ERR_PTR(count);

Leave this and simply add

> -	struct gpio_descs *descs __free(gpiod_put_array) =
> -		kzalloc_flex(*descs, desc, count);
> -	if (!descs)
> -		return ERR_PTR(-ENOMEM);
> +	if (count <= 0)
> +		return ERR_PTR(count ?: -ENOENT);

	if (count == 0)
		return -ENOENT;

The currently proposed code is less readable.

...

>  	for (i = 0; i < count; i++) {
> -		desc = gpiod_get_index(dev, con_id, i, flags);
> +		struct gpio_desc *desc __free(gpiod_put) =
> +			gpiod_get_index(dev, con_id, i, flags);
>  		if (IS_ERR(desc))
>  			return ERR_CAST(desc);
>  
> -		descs->desc[descs->ndescs++] = desc;
> -
> -		gdev = gpiod_to_gpio_device(desc);
> -		/*
> -		 * If pin hardware number of array member 0 is also 0, select
> -		 * its chip as a candidate for fast bitmap processing path.
> -		 */
> -		if (i == 0 && gpiod_hwgpio(desc) == 0) {
> -			struct gpio_descs *array;
> -			size_t descs_size = struct_size(descs, desc, count);
> -
> -			bitmap_size = BITS_TO_LONGS(gdev->ngpio > count ?
> -						    gdev->ngpio : count);
> -
> -			array = krealloc(descs, descs_size +
> -					 struct_size(array_info, invert_mask, 3 * bitmap_size),
> -					 GFP_KERNEL | __GFP_ZERO);
> -			if (!array)
> +>  		if (i == 0) {

This is almost invariant to the loop. Can we move it out?

> +			descs = gpio_descs_alloc(count, desc);
> +			if (!descs)
>  				return ERR_PTR(-ENOMEM);
> -
> -			descs = array;
> -
> -			array_info = (void *)descs + descs_size;
> -			array_info->get_mask = array_info->invert_mask +
> -						  bitmap_size;
> -			array_info->set_mask = array_info->get_mask +
> -						  bitmap_size;
> -
> -			array_info->desc = descs->desc;
> -			array_info->size = count;
> -			array_info->gdev = gdev;
> -			bitmap_set(array_info->get_mask, 0, count);
> -			bitmap_set(array_info->set_mask, 0, count);
> -			descs->info = array_info;
>  		}
>  
> -		/* If there is no cache for fast bitmap processing path, continue */
> -		if (!array_info)
> -			continue;
> +		gpio_descs_check_fastpath(descs, desc);
>  
> -		/* Unmark array members which don't belong to the 'fast' chip */
> -		if (array_info->gdev != gdev) {
> -			__clear_bit(i, array_info->get_mask);
> -			__clear_bit(i, array_info->set_mask);
> -		}
> -		/*
> -		 * Detect array members which belong to the 'fast' chip
> -		 * but their pins are not in hardware order.
> -		 */
> -		else if (gpiod_hwgpio(desc) != i) {
> -			/*
> -			 * Don't use fast path if all array members processed so
> -			 * far belong to the same chip as this one but its pin
> -			 * hardware number is different from its array index.
> -			 */
> -			if (bitmap_full(array_info->get_mask, i)) {
> -				descs->info = NULL;
> -				array_info = NULL;
> -			} else {
> -				__clear_bit(i, array_info->get_mask);
> -				__clear_bit(i, array_info->set_mask);
> -			}
> -		} else {
> -			dflags = READ_ONCE(desc->flags);
> -			/* Exclude open drain or open source from fast output */
> -			if (test_bit(GPIOD_FLAG_OPEN_DRAIN, &dflags) ||
> -			    test_bit(GPIOD_FLAG_OPEN_SOURCE, &dflags))
> -				__clear_bit(i, array_info->set_mask);
> -			/* Identify 'fast' pins which require invertion */
> -			if (gpiod_is_active_low(desc))
> -				__set_bit(i, array_info->invert_mask);
> -		}
> +		descs->desc[descs->ndescs++] = no_free_ptr(desc);
>  	}

-- 
With Best Regards,
Andy Shevchenko