From: Hugo Villeneuve hvilleneuve@dimonoff.com
commit 74abd086d2ee ("gpiolib: sanitize the return value of gpio_chip::get_multiple()") altered the value returned by gc->get_multiple() in case it is positive (> 0), but failed to return for other cases (<= 0).
This may result in the "if (gc->get)" block being executed and thus negates the performance gain that is normally obtained by using gc->get_multiple().
Fix by returning the result of gc->get_multiple() if it is <= 0.
Also move the "ret" variable to the scope where it is used, which as an added bonus fixes an indentation error introduced by the aforementioned commit.
Fixes: 74abd086d2ee ("gpiolib: sanitize the return value of gpio_chip::get_multiple()") Cc: stable@vger.kernel.org Signed-off-by: Hugo Villeneuve hvilleneuve@dimonoff.com --- drivers/gpio/gpiolib.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index fdafa0df1b43..3a3eca5b4c40 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -3297,14 +3297,15 @@ static int gpiod_get_raw_value_commit(const struct gpio_desc *desc) static int gpio_chip_get_multiple(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits) { - int ret; - lockdep_assert_held(&gc->gpiodev->srcu);
if (gc->get_multiple) { + int ret; + ret = gc->get_multiple(gc, mask, bits); if (ret > 0) return -EBADE; + return ret; }
if (gc->get) {
base-commit: b4911fb0b060899e4eebca0151eb56deb86921ec
From: Bartosz Golaszewski bartosz.golaszewski@linaro.org
On Thu, 03 Jul 2025 15:18:29 -0400, Hugo Villeneuve wrote:
commit 74abd086d2ee ("gpiolib: sanitize the return value of gpio_chip::get_multiple()") altered the value returned by gc->get_multiple() in case it is positive (> 0), but failed to return for other cases (<= 0).
This may result in the "if (gc->get)" block being executed and thus negates the performance gain that is normally obtained by using gc->get_multiple().
[...]
Applied, thanks!
[1/1] gpiolib: fix efficiency regression when using gpio_chip_get_multiple() https://git.kernel.org/brgl/linux/c/30e0fd3c0273dc106320081793793a424f1f1950
Best regards,
On Thu, Jul 3, 2025 at 9:18 PM Hugo Villeneuve hugo@hugovil.com wrote:
From: Hugo Villeneuve hvilleneuve@dimonoff.com
commit 74abd086d2ee ("gpiolib: sanitize the return value of gpio_chip::get_multiple()") altered the value returned by gc->get_multiple() in case it is positive (> 0), but failed to return for other cases (<= 0).
This may result in the "if (gc->get)" block being executed and thus negates the performance gain that is normally obtained by using gc->get_multiple().
Fix by returning the result of gc->get_multiple() if it is <= 0.
Also move the "ret" variable to the scope where it is used, which as an added bonus fixes an indentation error introduced by the aforementioned commit.
Thanks, I queued it for fixes. I typically keep local variables at the top of the function (just a personal readability preference) but since this function already has scope-local variables, let's do it. What is the indentation error you're mentioning exactly?
Bart
Fixes: 74abd086d2ee ("gpiolib: sanitize the return value of gpio_chip::get_multiple()") Cc: stable@vger.kernel.org Signed-off-by: Hugo Villeneuve hvilleneuve@dimonoff.com
drivers/gpio/gpiolib.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index fdafa0df1b43..3a3eca5b4c40 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -3297,14 +3297,15 @@ static int gpiod_get_raw_value_commit(const struct gpio_desc *desc) static int gpio_chip_get_multiple(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits) {
int ret;
lockdep_assert_held(&gc->gpiodev->srcu); if (gc->get_multiple) {
int ret;
ret = gc->get_multiple(gc, mask, bits); if (ret > 0) return -EBADE;
return ret; } if (gc->get) {
base-commit: b4911fb0b060899e4eebca0151eb56deb86921ec
2.39.5
Hi Bartosz,
On Fri, 4 Jul 2025 10:26:58 +0200 Bartosz Golaszewski brgl@bgdev.pl wrote:
On Thu, Jul 3, 2025 at 9:18 PM Hugo Villeneuve hugo@hugovil.com wrote:
From: Hugo Villeneuve hvilleneuve@dimonoff.com
commit 74abd086d2ee ("gpiolib: sanitize the return value of gpio_chip::get_multiple()") altered the value returned by gc->get_multiple() in case it is positive (> 0), but failed to return for other cases (<= 0).
This may result in the "if (gc->get)" block being executed and thus negates the performance gain that is normally obtained by using gc->get_multiple().
Fix by returning the result of gc->get_multiple() if it is <= 0.
Also move the "ret" variable to the scope where it is used, which as an added bonus fixes an indentation error introduced by the aforementioned commit.
Thanks, I queued it for fixes. I typically keep local variables at the top of the function (just a personal readability preference) but since this function already has scope-local variables, let's do it. What is the indentation error you're mentioning exactly?
Ok, I was under the assumption that having local-scope variables was the preferred approach in the kernel, as I sometimes see patches just for fixing variables scope. If it is something specific to the GPIO subsystem, no problem.
The ret variable was indented ok, but an empty line with spaces was introduced just after it.
Thanks for applying the patch.
Hugo.
Fixes: 74abd086d2ee ("gpiolib: sanitize the return value of gpio_chip::get_multiple()") Cc: stable@vger.kernel.org Signed-off-by: Hugo Villeneuve hvilleneuve@dimonoff.com
drivers/gpio/gpiolib.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index fdafa0df1b43..3a3eca5b4c40 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -3297,14 +3297,15 @@ static int gpiod_get_raw_value_commit(const struct gpio_desc *desc) static int gpio_chip_get_multiple(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits) {
int ret;
lockdep_assert_held(&gc->gpiodev->srcu); if (gc->get_multiple) {
int ret;
ret = gc->get_multiple(gc, mask, bits); if (ret > 0) return -EBADE;
return ret; } if (gc->get) {
base-commit: b4911fb0b060899e4eebca0151eb56deb86921ec
2.39.5
linux-stable-mirror@lists.linaro.org