[PATCH v1 1/2] gpiolib: Replace open coded krealloc()

[PATCH v1 1/2] gpiolib: Replace open coded krealloc()

[Andy Shevchenko]

gpiod_get_array() does a new allocation in some cases, followed
by copying previously allocated placeholder for the descriptors.

Replace that with krealloc(__GFP_ZERO), since it was kzalloc()
originally.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 112d99a5eec4..3e94990f1f90 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4283,12 +4283,14 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 	struct gpio_array *array_info = NULL;
 	struct gpio_chip *gc;
 	int count, bitmap_size;
+	size_t descs_size;
 
 	count = gpiod_count(dev, con_id);
 	if (count < 0)
 		return ERR_PTR(count);
 
-	descs = kzalloc(struct_size(descs, desc, count), GFP_KERNEL);
+	descs_size = struct_size(descs, desc, count);
+	descs = kzalloc(descs_size, GFP_KERNEL);
 	if (!descs)
 		return ERR_PTR(-ENOMEM);
 
@@ -4312,20 +4314,17 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 			bitmap_size = BITS_TO_LONGS(gc->ngpio > count ?
 						    gc->ngpio : count);
 
-			array = kzalloc(struct_size(descs, desc, count) +
-					struct_size(array_info, invert_mask,
-					3 * bitmap_size), GFP_KERNEL);
+			array = krealloc(descs, descs_size +
+					 struct_size(array_info, invert_mask, 3 * bitmap_size),
+					 GFP_KERNEL | __GFP_ZERO);
 			if (!array) {
 				gpiod_put_array(descs);
 				return ERR_PTR(-ENOMEM);
 			}
 
-			memcpy(array, descs,
-			       struct_size(descs, desc, descs->ndescs + 1));
-			kfree(descs);
-
 			descs = array;
-			array_info = (void *)(descs->desc + count);
+
+			array_info = (void *)descs + descs_size;
 			array_info->get_mask = array_info->invert_mask +
 						  bitmap_size;
 			array_info->set_mask = array_info->get_mask +
-- 
2.40.0.1.gaa8946217a0b

[PATCH v1 2/2] gpiolib: Check array_info for NULL only once in gpiod_get_array()

[Andy Shevchenko]

gpiod_get_array() has a long if-else-if branching where each of them
tests for the same variable to be not NULL. Instead, check for NULL
before even going to that flow.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3e94990f1f90..ee24428c5fe3 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4294,7 +4294,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 	if (!descs)
 		return ERR_PTR(-ENOMEM);
 
-	for (descs->ndescs = 0; descs->ndescs < count; ) {
+	for (descs->ndescs = 0; descs->ndescs < count; descs->ndescs++) {
 		desc = gpiod_get_index(dev, con_id, descs->ndescs, flags);
 		if (IS_ERR(desc)) {
 			gpiod_put_array(descs);
@@ -4339,8 +4339,13 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 				   count - descs->ndescs);
 			descs->info = array_info;
 		}
+
+		/* If there is no cache for fast bitmap processing path, continue */
+		if (!array_info)
+			continue;
+
 		/* Unmark array members which don't belong to the 'fast' chip */
-		if (array_info && array_info->chip != gc) {
+		if (array_info->chip != gc) {
 			__clear_bit(descs->ndescs, array_info->get_mask);
 			__clear_bit(descs->ndescs, array_info->set_mask);
 		}
@@ -4348,8 +4353,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 		 * Detect array members which belong to the 'fast' chip
 		 * but their pins are not in hardware order.
 		 */
-		else if (array_info &&
-			   gpio_chip_hwgpio(desc) != descs->ndescs) {
+		else if (gpio_chip_hwgpio(desc) != descs->ndescs) {
 			/*
 			 * Don't use fast path if all array members processed so
 			 * far belong to the same chip as this one but its pin
@@ -4363,7 +4367,7 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 				__clear_bit(descs->ndescs,
 					    array_info->set_mask);
 			}
-		} else if (array_info) {
+		} else {
 			/* Exclude open drain or open source from fast output */
 			if (gpiochip_line_is_open_drain(gc, descs->ndescs) ||
 			    gpiochip_line_is_open_source(gc, descs->ndescs))
@@ -4374,8 +4378,6 @@ struct gpio_descs *__must_check gpiod_get_array(struct device *dev,
 				__set_bit(descs->ndescs,
 					  array_info->invert_mask);
 		}
-
-		descs->ndescs++;
 	}
 	if (array_info)
 		dev_dbg(dev,
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v1 1/2] gpiolib: Replace open coded krealloc()

[Linus Walleij]

On Tue, Mar 21, 2023 at 2:53 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> gpiod_get_array() does a new allocation in some cases, followed
> by copying previously allocated placeholder for the descriptors.
>
> Replace that with krealloc(__GFP_ZERO), since it was kzalloc()
> originally.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Wow that's a really nice patch!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v1 2/2] gpiolib: Check array_info for NULL only once in gpiod_get_array()

[Linus Walleij]

On Tue, Mar 21, 2023 at 2:53 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> gpiod_get_array() has a long if-else-if branching where each of them
> tests for the same variable to be not NULL. Instead, check for NULL
> before even going to that flow.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

It's way more readable like this.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v1 1/2] gpiolib: Replace open coded krealloc()

[Bartosz Golaszewski]

On Tue, Mar 21, 2023 at 2:53 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> gpiod_get_array() does a new allocation in some cases, followed
> by copying previously allocated placeholder for the descriptors.
>
> Replace that with krealloc(__GFP_ZERO), since it was kzalloc()
> originally.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---

Series applied, thanks!

Bart