Commit 3027743f authored by Laura Abbott's avatar Laura Abbott Committed by Linus Walleij
Browse files

gpio: Remove VLA from gpiolib 

The new challenge is to remove VLAs from the kernel
(see https://lkml.org/lkml/2018/3/7/621

) to eventually
turn on -Wvla.

Using a kmalloc array is the easy way to fix this but kmalloc is still
more expensive than stack allocation. Introduce a fast path with a
fixed size stack array to cover most chip with gpios below some fixed
amount. The slow path dynamically allocates an array to cover those
chips with a large number of gpios.

Reviewed-by: default avatarGeert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: default avatarPhil Reid <preid@electromag.com.au>
Reviewed-and-tested-by: default avatarLukas Wunner <lukas@wunner.de>
Signed-off-by: default avatarLukas Wunner <lukas@wunner.de>
Signed-off-by: default avatarLaura Abbott <labbott@redhat.com>
Reviewed-by: default avatarAndy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: default avatarLinus Walleij <linus.walleij@linaro.org>
parent ed5cab43

drivers/gpio/Kconfig

+12 −0
Original line number Diff line number Diff line
@@ -22,6 +22,18 @@ menuconfig GPIOLIB 


if GPIOLIB



config GPIOLIB_FASTPATH_LIMIT

	int "Maximum number of GPIOs for fast path"

	range 32 512

	default 512

	help

	   This adjusts the point at which certain APIs will switch from

	   using a stack allocated buffer to a dynamically allocated buffer.



	   You shouldn't need to change this unless you really need to

	   optimize either stack space or performance. Change this carefully

	   since setting an incorrect value could cause stack corruption.



config OF_GPIO

	def_bool y

	depends on OF

drivers/gpio/gpiolib.c

+59 −17
Original line number Diff line number Diff line
@@ -61,6 +61,11 @@ static struct bus_type gpio_bus_type = { 
	.name = "gpio",

};



/*

 * Number of GPIOs to use for the fast path in set array

 */

#define FASTPATH_NGPIO CONFIG_GPIOLIB_FASTPATH_LIMIT



/* gpio_lock prevents conflicts during gpio_desc[] table updates.

 * While any GPIO is requested, its gpio_chip is not removable;

 * each GPIO's "requested" flag serves as a lock and refcount.
@@ -453,12 +458,11 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd, 
			vals[i] = !!ghd.values[i];



		/* Reuse the array setting function */

		gpiod_set_array_value_complex(false,

		return gpiod_set_array_value_complex(false,

					      true,

					      lh->numdescs,

					      lh->descs,

					      vals);

		return 0;

	}

	return -EINVAL;

}
@@ -1281,6 +1285,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, 
		goto err_free_descs;

	}



	if (chip->ngpio > FASTPATH_NGPIO)

		chip_warn(chip, "line cnt %u is greater than fast path cnt %u\n",

		chip->ngpio, FASTPATH_NGPIO);



	gdev->label = kstrdup_const(chip->label ?: "unknown", GFP_KERNEL);

	if (!gdev->label) {

		status = -ENOMEM;
@@ -2758,16 +2766,28 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, 


	while (i < array_size) {

		struct gpio_chip *chip = desc_array[i]->gdev->chip;

		unsigned long mask[BITS_TO_LONGS(chip->ngpio)];

		unsigned long bits[BITS_TO_LONGS(chip->ngpio)];

		unsigned long fastpath[2 * BITS_TO_LONGS(FASTPATH_NGPIO)];

		unsigned long *mask, *bits;

		int first, j, ret;



		if (likely(chip->ngpio <= FASTPATH_NGPIO)) {

			mask = fastpath;

		} else {

			mask = kmalloc_array(2 * BITS_TO_LONGS(chip->ngpio),

					   sizeof(*mask),

					   can_sleep ? GFP_KERNEL : GFP_ATOMIC);

			if (!mask)

				return -ENOMEM;

		}



		bits = mask + BITS_TO_LONGS(chip->ngpio);

		bitmap_zero(mask, chip->ngpio);



		if (!can_sleep)

			WARN_ON(chip->can_sleep);



		/* collect all inputs belonging to the same chip */

		first = i;

		memset(mask, 0, sizeof(mask));

		do {

			const struct gpio_desc *desc = desc_array[i];

			int hwgpio = gpio_chip_hwgpio(desc);
@@ -2778,8 +2798,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, 
			 (desc_array[i]->gdev->chip == chip));



		ret = gpio_chip_get_multiple(chip, mask, bits);

		if (ret)

		if (ret) {

			if (mask != fastpath)

				kfree(mask);

			return ret;

		}



		for (j = first; j < i; j++) {

			const struct gpio_desc *desc = desc_array[j];
@@ -2791,6 +2814,9 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, 
			value_array[j] = value;

			trace_gpio_value(desc_to_gpio(desc), 1, value);

		}



		if (mask != fastpath)

			kfree(mask);

	}

	return 0;

}
@@ -2974,7 +3000,7 @@ static void gpio_chip_set_multiple(struct gpio_chip *chip, 
	}

}



void gpiod_set_array_value_complex(bool raw, bool can_sleep,

int gpiod_set_array_value_complex(bool raw, bool can_sleep,

				   unsigned int array_size,

				   struct gpio_desc **desc_array,

				   int *value_array)
@@ -2983,14 +3009,26 @@ void gpiod_set_array_value_complex(bool raw, bool can_sleep, 


	while (i < array_size) {

		struct gpio_chip *chip = desc_array[i]->gdev->chip;

		unsigned long mask[BITS_TO_LONGS(chip->ngpio)];

		unsigned long bits[BITS_TO_LONGS(chip->ngpio)];

		unsigned long fastpath[2 * BITS_TO_LONGS(FASTPATH_NGPIO)];

		unsigned long *mask, *bits;

		int count = 0;



		if (likely(chip->ngpio <= FASTPATH_NGPIO)) {

			mask = fastpath;

		} else {

			mask = kmalloc_array(2 * BITS_TO_LONGS(chip->ngpio),

					   sizeof(*mask),

					   can_sleep ? GFP_KERNEL : GFP_ATOMIC);

			if (!mask)

				return -ENOMEM;

		}



		bits = mask + BITS_TO_LONGS(chip->ngpio);

		bitmap_zero(mask, chip->ngpio);



		if (!can_sleep)

			WARN_ON(chip->can_sleep);



		memset(mask, 0, sizeof(mask));

		do {

			struct gpio_desc *desc = desc_array[i];

			int hwgpio = gpio_chip_hwgpio(desc);
@@ -3021,7 +3059,11 @@ void gpiod_set_array_value_complex(bool raw, bool can_sleep, 
		/* push collected bits to outputs */

		if (count != 0)

			gpio_chip_set_multiple(chip, mask, bits);



		if (mask != fastpath)

			kfree(mask);

	}

	return 0;

}



/**
@@ -3096,13 +3138,13 @@ EXPORT_SYMBOL_GPL(gpiod_set_value); 
 * This function should be called from contexts where we cannot sleep, and will

 * complain if the GPIO chip functions potentially sleep.

 */

void gpiod_set_raw_array_value(unsigned int array_size,

int gpiod_set_raw_array_value(unsigned int array_size,

			 struct gpio_desc **desc_array, int *value_array)

{

	if (!desc_array)

		return;

	gpiod_set_array_value_complex(true, false, array_size, desc_array,

				      value_array);

		return -EINVAL;

	return gpiod_set_array_value_complex(true, false, array_size,

					desc_array, value_array);

}

EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value);
@@ -3422,14 +3464,14 @@ EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep); 
 *

 * This function is to be called from contexts that can sleep.

 */

void gpiod_set_raw_array_value_cansleep(unsigned int array_size,

int gpiod_set_raw_array_value_cansleep(unsigned int array_size,

					struct gpio_desc **desc_array,

					int *value_array)

{

	might_sleep_if(extra_checks);

	if (!desc_array)

		return;

	gpiod_set_array_value_complex(true, true, array_size, desc_array,

		return -EINVAL;

	return gpiod_set_array_value_complex(true, true, array_size, desc_array,

				      value_array);

}

EXPORT_SYMBOL_GPL(gpiod_set_raw_array_value_cansleep);

drivers/gpio/gpiolib.h

+1 −1
Original line number Diff line number Diff line
@@ -188,7 +188,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep, 
				  unsigned int array_size,

				  struct gpio_desc **desc_array,

				  int *value_array);

void gpiod_set_array_value_complex(bool raw, bool can_sleep,

int gpiod_set_array_value_complex(bool raw, bool can_sleep,

				   unsigned int array_size,

				   struct gpio_desc **desc_array,

				   int *value_array);

include/linux/gpio/consumer.h

+6 −4
Original line number Diff line number Diff line
@@ -116,7 +116,7 @@ int gpiod_get_raw_array_value(unsigned int array_size, 
			      struct gpio_desc **desc_array,

			      int *value_array);

void gpiod_set_raw_value(struct gpio_desc *desc, int value);

void gpiod_set_raw_array_value(unsigned int array_size,

int gpiod_set_raw_array_value(unsigned int array_size,

			       struct gpio_desc **desc_array,

			       int *value_array);
@@ -134,7 +134,7 @@ int gpiod_get_raw_array_value_cansleep(unsigned int array_size, 
				       struct gpio_desc **desc_array,

				       int *value_array);

void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);

void gpiod_set_raw_array_value_cansleep(unsigned int array_size,

int gpiod_set_raw_array_value_cansleep(unsigned int array_size,

					struct gpio_desc **desc_array,

					int *value_array);
@@ -369,12 +369,13 @@ static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value) 
	/* GPIO can never have been requested */

	WARN_ON(1);

}

static inline void gpiod_set_raw_array_value(unsigned int array_size,

static inline int gpiod_set_raw_array_value(unsigned int array_size,

					     struct gpio_desc **desc_array,

					     int *value_array)

{

	/* GPIO can never have been requested */

	WARN_ON(1);

	return 0;

}



static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc)
@@ -423,12 +424,13 @@ static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, 
	/* GPIO can never have been requested */

	WARN_ON(1);

}

static inline void gpiod_set_raw_array_value_cansleep(unsigned int array_size,

static inline int gpiod_set_raw_array_value_cansleep(unsigned int array_size,

						struct gpio_desc **desc_array,

						int *value_array)

{

	/* GPIO can never have been requested */

	WARN_ON(1);

	return 0;

}



static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)

CRA Git | Maintained and supported by SUSTech CRA and CCSE