* [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup()
@ 2023-08-09 13:14 Bartosz Golaszewski
2023-08-09 13:14 ` [PATCH 2/2] gpio: sim: simplify code with cleanup helpers Bartosz Golaszewski
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2023-08-09 13:14 UTC (permalink / raw)
To: Linus Walleij, Andy Shevchenko, Kent Gibson
Cc: linux-gpio, linux-kernel, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
When comparing strings passed to us from configfs, we can pass the page
argument directly to sysfs_streq() and avoid manual string trimming.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-sim.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
index 8b49b0abacd5..dc4097dc0fbc 100644
--- a/drivers/gpio/gpio-sim.c
+++ b/drivers/gpio/gpio-sim.c
@@ -1272,7 +1272,6 @@ gpio_sim_hog_config_direction_store(struct config_item *item,
{
struct gpio_sim_hog *hog = to_gpio_sim_hog(item);
struct gpio_sim_device *dev = gpio_sim_hog_get_device(hog);
- char *trimmed;
int dir;
mutex_lock(&dev->lock);
@@ -1282,23 +1281,15 @@ gpio_sim_hog_config_direction_store(struct config_item *item,
return -EBUSY;
}
- trimmed = gpio_sim_strdup_trimmed(page, count);
- if (!trimmed) {
- mutex_unlock(&dev->lock);
- return -ENOMEM;
- }
-
- if (strcmp(trimmed, "input") == 0)
+ if (sysfs_streq(page, "input"))
dir = GPIOD_IN;
- else if (strcmp(trimmed, "output-high") == 0)
+ else if (sysfs_streq(page, "output-high"))
dir = GPIOD_OUT_HIGH;
- else if (strcmp(trimmed, "output-low") == 0)
+ else if (sysfs_streq(page, "output-low"))
dir = GPIOD_OUT_LOW;
else
dir = -EINVAL;
- kfree(trimmed);
-
if (dir < 0) {
mutex_unlock(&dev->lock);
return dir;
--
2.39.2
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-09 13:14 [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup() Bartosz Golaszewski @ 2023-08-09 13:14 ` Bartosz Golaszewski 2023-08-10 14:39 ` Andy Shevchenko ` (2 more replies) 2023-08-10 14:04 ` [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup() Andy Shevchenko 2023-08-11 11:59 ` Bartosz Golaszewski 2 siblings, 3 replies; 18+ messages in thread From: Bartosz Golaszewski @ 2023-08-09 13:14 UTC (permalink / raw) To: Linus Walleij, Andy Shevchenko, Kent Gibson Cc: linux-gpio, linux-kernel, Bartosz Golaszewski From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Use macros defined in linux/cleanup.h to automate resource lifetime control in the gpio-simulator. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> --- drivers/gpio/gpio-sim.c | 224 ++++++++++++++-------------------------- 1 file changed, 79 insertions(+), 145 deletions(-) diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c index dc4097dc0fbc..715e79dc3978 100644 --- a/drivers/gpio/gpio-sim.c +++ b/drivers/gpio/gpio-sim.c @@ -8,6 +8,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/bitmap.h> +#include <linux/cleanup.h> #include <linux/completion.h> #include <linux/configfs.h> #include <linux/device.h> @@ -68,7 +69,7 @@ static int gpio_sim_apply_pull(struct gpio_sim_chip *chip, gc = &chip->gc; desc = &gc->gpiodev->descs[offset]; - mutex_lock(&chip->lock); + guard(mutex)(&chip->lock); if (test_bit(FLAG_REQUESTED, &desc->flags) && !test_bit(FLAG_IS_OUT, &desc->flags)) { @@ -104,7 +105,6 @@ static int gpio_sim_apply_pull(struct gpio_sim_chip *chip, set_pull: __assign_bit(offset, chip->pull_map, value); - mutex_unlock(&chip->lock); return 0; } @@ -113,9 +113,8 @@ static int gpio_sim_get(struct gpio_chip *gc, unsigned int offset) struct gpio_sim_chip *chip = gpiochip_get_data(gc); int ret; - mutex_lock(&chip->lock); - ret = !!test_bit(offset, chip->value_map); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) + ret = !!test_bit(offset, chip->value_map); return ret; } @@ -124,9 +123,8 @@ static void gpio_sim_set(struct gpio_chip *gc, unsigned int offset, int value) { struct gpio_sim_chip *chip = gpiochip_get_data(gc); - mutex_lock(&chip->lock); - __assign_bit(offset, chip->value_map, value); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) + __assign_bit(offset, chip->value_map, value); } static int gpio_sim_get_multiple(struct gpio_chip *gc, @@ -134,9 +132,8 @@ static int gpio_sim_get_multiple(struct gpio_chip *gc, { struct gpio_sim_chip *chip = gpiochip_get_data(gc); - mutex_lock(&chip->lock); - bitmap_replace(bits, bits, chip->value_map, mask, gc->ngpio); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) + bitmap_replace(bits, bits, chip->value_map, mask, gc->ngpio); return 0; } @@ -146,9 +143,9 @@ static void gpio_sim_set_multiple(struct gpio_chip *gc, { struct gpio_sim_chip *chip = gpiochip_get_data(gc); - mutex_lock(&chip->lock); - bitmap_replace(chip->value_map, chip->value_map, bits, mask, gc->ngpio); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) + bitmap_replace(chip->value_map, chip->value_map, bits, mask, + gc->ngpio); } static int gpio_sim_direction_output(struct gpio_chip *gc, @@ -156,10 +153,10 @@ static int gpio_sim_direction_output(struct gpio_chip *gc, { struct gpio_sim_chip *chip = gpiochip_get_data(gc); - mutex_lock(&chip->lock); - __clear_bit(offset, chip->direction_map); - __assign_bit(offset, chip->value_map, value); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) { + __clear_bit(offset, chip->direction_map); + __assign_bit(offset, chip->value_map, value); + } return 0; } @@ -168,9 +165,8 @@ static int gpio_sim_direction_input(struct gpio_chip *gc, unsigned int offset) { struct gpio_sim_chip *chip = gpiochip_get_data(gc); - mutex_lock(&chip->lock); - __set_bit(offset, chip->direction_map); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) + __set_bit(offset, chip->direction_map); return 0; } @@ -180,9 +176,8 @@ static int gpio_sim_get_direction(struct gpio_chip *gc, unsigned int offset) struct gpio_sim_chip *chip = gpiochip_get_data(gc); int direction; - mutex_lock(&chip->lock); - direction = !!test_bit(offset, chip->direction_map); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) + direction = !!test_bit(offset, chip->direction_map); return direction ? GPIO_LINE_DIRECTION_IN : GPIO_LINE_DIRECTION_OUT; } @@ -215,9 +210,9 @@ static void gpio_sim_free(struct gpio_chip *gc, unsigned int offset) { struct gpio_sim_chip *chip = gpiochip_get_data(gc); - mutex_lock(&chip->lock); - __assign_bit(offset, chip->value_map, !!test_bit(offset, chip->pull_map)); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) + __assign_bit(offset, chip->value_map, + !!test_bit(offset, chip->pull_map)); } static ssize_t gpio_sim_sysfs_val_show(struct device *dev, @@ -227,9 +222,8 @@ static ssize_t gpio_sim_sysfs_val_show(struct device *dev, struct gpio_sim_chip *chip = dev_get_drvdata(dev); int val; - mutex_lock(&chip->lock); - val = !!test_bit(line_attr->offset, chip->value_map); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) + val = !!test_bit(line_attr->offset, chip->value_map); return sysfs_emit(buf, "%d\n", val); } @@ -258,9 +252,8 @@ static ssize_t gpio_sim_sysfs_pull_show(struct device *dev, struct gpio_sim_chip *chip = dev_get_drvdata(dev); int pull; - mutex_lock(&chip->lock); - pull = !!test_bit(line_attr->offset, chip->pull_map); - mutex_unlock(&chip->lock); + scoped_guard(mutex, &chip->lock) + pull = !!test_bit(line_attr->offset, chip->pull_map); return sysfs_emit(buf, "%s\n", gpio_sim_sysfs_pull_strings[pull]); } @@ -661,13 +654,13 @@ static ssize_t gpio_sim_device_config_dev_name_show(struct config_item *item, struct platform_device *pdev; int ret; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); + pdev = dev->pdev; if (pdev) ret = sprintf(page, "%s\n", dev_name(&pdev->dev)); else ret = sprintf(page, "gpio-sim.%d\n", dev->id); - mutex_unlock(&dev->lock); return ret; } @@ -680,9 +673,8 @@ gpio_sim_device_config_live_show(struct config_item *item, char *page) struct gpio_sim_device *dev = to_gpio_sim_device(item); bool live; - mutex_lock(&dev->lock); - live = gpio_sim_device_is_live_unlocked(dev); - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) + live = gpio_sim_device_is_live_unlocked(dev); return sprintf(page, "%c\n", live ? '1' : '0'); } @@ -837,8 +829,8 @@ gpio_sim_make_bank_swnode(struct gpio_sim_bank *bank, { struct property_entry properties[GPIO_SIM_PROP_MAX]; unsigned int prop_idx = 0, line_names_size = 0; + char **line_names __free(kfree) = NULL; struct fwnode_handle *swnode; - char **line_names; memset(properties, 0, sizeof(properties)); @@ -858,7 +850,6 @@ gpio_sim_make_bank_swnode(struct gpio_sim_bank *bank, line_names, line_names_size); swnode = fwnode_create_software_node(properties, parent); - kfree(line_names); return swnode; } @@ -984,7 +975,7 @@ gpio_sim_device_config_live_store(struct config_item *item, if (ret) return ret; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); if ((!live && !gpio_sim_device_is_live_unlocked(dev)) || (live && gpio_sim_device_is_live_unlocked(dev))) @@ -994,8 +985,6 @@ gpio_sim_device_config_live_store(struct config_item *item, else gpio_sim_device_deactivate_unlocked(dev); - mutex_unlock(&dev->lock); - return ret ?: count; } @@ -1034,13 +1023,13 @@ static ssize_t gpio_sim_bank_config_chip_name_show(struct config_item *item, struct gpio_sim_chip_name_ctx ctx = { bank->swnode, page }; int ret; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); + if (gpio_sim_device_is_live_unlocked(dev)) ret = device_for_each_child(&dev->pdev->dev, &ctx, gpio_sim_emit_chip_name); else ret = sprintf(page, "none\n"); - mutex_unlock(&dev->lock); return ret; } @@ -1054,9 +1043,8 @@ gpio_sim_bank_config_label_show(struct config_item *item, char *page) struct gpio_sim_device *dev = gpio_sim_bank_get_device(bank); int ret; - mutex_lock(&dev->lock); - ret = sprintf(page, "%s\n", bank->label ?: ""); - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) + ret = sprintf(page, "%s\n", bank->label ?: ""); return ret; } @@ -1068,23 +1056,18 @@ static ssize_t gpio_sim_bank_config_label_store(struct config_item *item, struct gpio_sim_device *dev = gpio_sim_bank_get_device(bank); char *trimmed; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); - if (gpio_sim_device_is_live_unlocked(dev)) { - mutex_unlock(&dev->lock); + if (gpio_sim_device_is_live_unlocked(dev)) return -EBUSY; - } trimmed = gpio_sim_strdup_trimmed(page, count); - if (!trimmed) { - mutex_unlock(&dev->lock); + if (!trimmed) return -ENOMEM; - } kfree(bank->label); bank->label = trimmed; - mutex_unlock(&dev->lock); return count; } @@ -1097,9 +1080,8 @@ gpio_sim_bank_config_num_lines_show(struct config_item *item, char *page) struct gpio_sim_device *dev = gpio_sim_bank_get_device(bank); int ret; - mutex_lock(&dev->lock); - ret = sprintf(page, "%u\n", bank->num_lines); - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) + ret = sprintf(page, "%u\n", bank->num_lines); return ret; } @@ -1120,16 +1102,13 @@ gpio_sim_bank_config_num_lines_store(struct config_item *item, if (num_lines == 0) return -EINVAL; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); - if (gpio_sim_device_is_live_unlocked(dev)) { - mutex_unlock(&dev->lock); + if (gpio_sim_device_is_live_unlocked(dev)) return -EBUSY; - } bank->num_lines = num_lines; - mutex_unlock(&dev->lock); return count; } @@ -1149,9 +1128,8 @@ gpio_sim_line_config_name_show(struct config_item *item, char *page) struct gpio_sim_device *dev = gpio_sim_line_get_device(line); int ret; - mutex_lock(&dev->lock); - ret = sprintf(page, "%s\n", line->name ?: ""); - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) + ret = sprintf(page, "%s\n", line->name ?: ""); return ret; } @@ -1163,24 +1141,18 @@ static ssize_t gpio_sim_line_config_name_store(struct config_item *item, struct gpio_sim_device *dev = gpio_sim_line_get_device(line); char *trimmed; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); - if (gpio_sim_device_is_live_unlocked(dev)) { - mutex_unlock(&dev->lock); + if (gpio_sim_device_is_live_unlocked(dev)) return -EBUSY; - } trimmed = gpio_sim_strdup_trimmed(page, count); - if (!trimmed) { - mutex_unlock(&dev->lock); + if (!trimmed) return -ENOMEM; - } kfree(line->name); line->name = trimmed; - mutex_unlock(&dev->lock); - return count; } @@ -1198,9 +1170,8 @@ static ssize_t gpio_sim_hog_config_name_show(struct config_item *item, struct gpio_sim_device *dev = gpio_sim_hog_get_device(hog); int ret; - mutex_lock(&dev->lock); - ret = sprintf(page, "%s\n", hog->name ?: ""); - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) + ret = sprintf(page, "%s\n", hog->name ?: ""); return ret; } @@ -1212,24 +1183,18 @@ static ssize_t gpio_sim_hog_config_name_store(struct config_item *item, struct gpio_sim_device *dev = gpio_sim_hog_get_device(hog); char *trimmed; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); - if (gpio_sim_device_is_live_unlocked(dev)) { - mutex_unlock(&dev->lock); + if (gpio_sim_device_is_live_unlocked(dev)) return -EBUSY; - } trimmed = gpio_sim_strdup_trimmed(page, count); - if (!trimmed) { - mutex_unlock(&dev->lock); + if (!trimmed) return -ENOMEM; - } kfree(hog->name); hog->name = trimmed; - mutex_unlock(&dev->lock); - return count; } @@ -1243,9 +1208,8 @@ static ssize_t gpio_sim_hog_config_direction_show(struct config_item *item, char *repr; int dir; - mutex_lock(&dev->lock); - dir = hog->dir; - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) + dir = hog->dir; switch (dir) { case GPIOD_IN: @@ -1274,12 +1238,10 @@ gpio_sim_hog_config_direction_store(struct config_item *item, struct gpio_sim_device *dev = gpio_sim_hog_get_device(hog); int dir; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); - if (gpio_sim_device_is_live_unlocked(dev)) { - mutex_unlock(&dev->lock); + if (gpio_sim_device_is_live_unlocked(dev)) return -EBUSY; - } if (sysfs_streq(page, "input")) dir = GPIOD_IN; @@ -1288,17 +1250,10 @@ gpio_sim_hog_config_direction_store(struct config_item *item, else if (sysfs_streq(page, "output-low")) dir = GPIOD_OUT_LOW; else - dir = -EINVAL; - - if (dir < 0) { - mutex_unlock(&dev->lock); - return dir; - } + return -EINVAL; hog->dir = dir; - mutex_unlock(&dev->lock); - return count; } @@ -1316,9 +1271,8 @@ static void gpio_sim_hog_config_item_release(struct config_item *item) struct gpio_sim_line *line = hog->parent; struct gpio_sim_device *dev = gpio_sim_hog_get_device(hog); - mutex_lock(&dev->lock); - line->hog = NULL; - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) + line->hog = NULL; kfree(hog->name); kfree(hog); @@ -1344,13 +1298,11 @@ gpio_sim_line_config_make_hog_item(struct config_group *group, const char *name) if (strcmp(name, "hog") != 0) return ERR_PTR(-EINVAL); - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); hog = kzalloc(sizeof(*hog), GFP_KERNEL); - if (!hog) { - mutex_unlock(&dev->lock); + if (!hog) return ERR_PTR(-ENOMEM); - } config_item_init_type_name(&hog->item, name, &gpio_sim_hog_config_type); @@ -1360,8 +1312,6 @@ gpio_sim_line_config_make_hog_item(struct config_group *group, const char *name) hog->parent = line; line->hog = hog; - mutex_unlock(&dev->lock); - return &hog->item; } @@ -1370,9 +1320,8 @@ static void gpio_sim_line_config_group_release(struct config_item *item) struct gpio_sim_line *line = to_gpio_sim_line(item); struct gpio_sim_device *dev = gpio_sim_line_get_device(line); - mutex_lock(&dev->lock); - list_del(&line->siblings); - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) + list_del(&line->siblings); kfree(line->name); kfree(line); @@ -1407,18 +1356,14 @@ gpio_sim_bank_config_make_line_group(struct config_group *group, if (ret != 1 || nchar != strlen(name)) return ERR_PTR(-EINVAL); - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); - if (gpio_sim_device_is_live_unlocked(dev)) { - mutex_unlock(&dev->lock); + if (gpio_sim_device_is_live_unlocked(dev)) return ERR_PTR(-EBUSY); - } line = kzalloc(sizeof(*line), GFP_KERNEL); - if (!line) { - mutex_unlock(&dev->lock); + if (!line) return ERR_PTR(-ENOMEM); - } config_group_init_type_name(&line->group, name, &gpio_sim_line_config_type); @@ -1427,8 +1372,6 @@ gpio_sim_bank_config_make_line_group(struct config_group *group, line->offset = offset; list_add_tail(&line->siblings, &bank->line_list); - mutex_unlock(&dev->lock); - return &line->group; } @@ -1437,9 +1380,8 @@ static void gpio_sim_bank_config_group_release(struct config_item *item) struct gpio_sim_bank *bank = to_gpio_sim_bank(item); struct gpio_sim_device *dev = gpio_sim_bank_get_device(bank); - mutex_lock(&dev->lock); - list_del(&bank->siblings); - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) + list_del(&bank->siblings); kfree(bank->label); kfree(bank); @@ -1467,18 +1409,14 @@ gpio_sim_device_config_make_bank_group(struct config_group *group, struct gpio_sim_device *dev = to_gpio_sim_device(&group->cg_item); struct gpio_sim_bank *bank; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); - if (gpio_sim_device_is_live_unlocked(dev)) { - mutex_unlock(&dev->lock); + if (gpio_sim_device_is_live_unlocked(dev)) return ERR_PTR(-EBUSY); - } bank = kzalloc(sizeof(*bank), GFP_KERNEL); - if (!bank) { - mutex_unlock(&dev->lock); + if (!bank) return ERR_PTR(-ENOMEM); - } config_group_init_type_name(&bank->group, name, &gpio_sim_bank_config_group_type); @@ -1487,8 +1425,6 @@ gpio_sim_device_config_make_bank_group(struct config_group *group, INIT_LIST_HEAD(&bank->line_list); list_add_tail(&bank->siblings, &dev->bank_list); - mutex_unlock(&dev->lock); - return &bank->group; } @@ -1496,10 +1432,10 @@ static void gpio_sim_device_config_group_release(struct config_item *item) { struct gpio_sim_device *dev = to_gpio_sim_device(item); - mutex_lock(&dev->lock); - if (gpio_sim_device_is_live_unlocked(dev)) - gpio_sim_device_deactivate_unlocked(dev); - mutex_unlock(&dev->lock); + scoped_guard(mutex, &dev->lock) { + if (gpio_sim_device_is_live_unlocked(dev)) + gpio_sim_device_deactivate_unlocked(dev); + } mutex_destroy(&dev->lock); ida_free(&gpio_sim_ida, dev->id); @@ -1524,7 +1460,7 @@ static const struct config_item_type gpio_sim_device_config_group_type = { static struct config_group * gpio_sim_config_make_device_group(struct config_group *group, const char *name) { - struct gpio_sim_device *dev; + struct gpio_sim_device *dev __free(kfree) = NULL; int id; dev = kzalloc(sizeof(*dev), GFP_KERNEL); @@ -1532,10 +1468,8 @@ gpio_sim_config_make_device_group(struct config_group *group, const char *name) return ERR_PTR(-ENOMEM); id = ida_alloc(&gpio_sim_ida, GFP_KERNEL); - if (id < 0) { - kfree(dev); + if (id < 0) return ERR_PTR(id); - } config_group_init_type_name(&dev->group, name, &gpio_sim_device_config_group_type); @@ -1546,7 +1480,7 @@ gpio_sim_config_make_device_group(struct config_group *group, const char *name) dev->bus_notifier.notifier_call = gpio_sim_bus_notifier_call; init_completion(&dev->probe_completion); - return &dev->group; + return &no_free_ptr(dev)->group; } static struct configfs_group_operations gpio_sim_config_group_ops = { -- 2.39.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-09 13:14 ` [PATCH 2/2] gpio: sim: simplify code with cleanup helpers Bartosz Golaszewski @ 2023-08-10 14:39 ` Andy Shevchenko 2023-08-10 19:04 ` Bartosz Golaszewski 2023-08-11 5:20 ` Dan Carpenter 2023-08-15 8:04 ` Linus Walleij 2 siblings, 1 reply; 18+ messages in thread From: Andy Shevchenko @ 2023-08-10 14:39 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Wed, Aug 09, 2023 at 03:14:42PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Use macros defined in linux/cleanup.h to automate resource lifetime > control in the gpio-simulator. gpio-sim ? ... > - mutex_lock(&chip->lock); > + guard(mutex)(&chip->lock); I hoped to see somehing like guard_mutex(...); But looking into cleanup.h it seems to me that the lock itself on GPIO library can be defined with respective class, no? ... > + scoped_guard(mutex, &chip->lock) > + bitmap_replace(chip->value_map, chip->value_map, bits, mask, > + gc->ngpio); Perhaps with {} ? ... > int ret; > > - mutex_lock(&dev->lock); > + guard(mutex)(&dev->lock); > + > pdev = dev->pdev; > if (pdev) > ret = sprintf(page, "%s\n", dev_name(&pdev->dev)); > else > ret = sprintf(page, "gpio-sim.%d\n", dev->id); > - mutex_unlock(&dev->lock); > > return ret; Now can be if (...) return ... else // if you wish (not needed) return ... ... > int ret; > > - mutex_lock(&dev->lock); > + guard(mutex)(&dev->lock); > + > if (gpio_sim_device_is_live_unlocked(dev)) > ret = device_for_each_child(&dev->pdev->dev, &ctx, > gpio_sim_emit_chip_name); > else > ret = sprintf(page, "none\n"); > - mutex_unlock(&dev->lock); > > return ret; As per above. And may be other functions as well. ... > int ret; > > - mutex_lock(&dev->lock); > - ret = sprintf(page, "%s\n", line->name ?: ""); > - mutex_unlock(&dev->lock); > + scoped_guard(mutex, &dev->lock) > + ret = sprintf(page, "%s\n", line->name ?: ""); > > return ret; Why not guard(...); return sprintf(...); ? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-10 14:39 ` Andy Shevchenko @ 2023-08-10 19:04 ` Bartosz Golaszewski 2023-08-11 9:14 ` Andy Shevchenko 0 siblings, 1 reply; 18+ messages in thread From: Bartosz Golaszewski @ 2023-08-10 19:04 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Thu, Aug 10, 2023 at 4:42 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Aug 09, 2023 at 03:14:42PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Use macros defined in linux/cleanup.h to automate resource lifetime > > control in the gpio-simulator. > > gpio-sim ? > Meh, if you insist... > ... > > > - mutex_lock(&chip->lock); > > + guard(mutex)(&chip->lock); > > I hoped to see somehing like > > guard_mutex(...); > > But looking into cleanup.h it seems to me that the lock itself on GPIO library > can be defined with respective class, no? > Why though? This is perfectly clear and concise as it is. It's similar to going bare mutex_lock() everywhere instead of wrapping it with foo_lock() which requires you to go and check what you're locking. > ... > > > + scoped_guard(mutex, &chip->lock) > > + bitmap_replace(chip->value_map, chip->value_map, bits, mask, > > + gc->ngpio); > > Perhaps with {} ? > This scoped_guard() thing is in essence a for loop, so I believe kernel coding style applies and a single statement doesn't require a {}. > ... > > > int ret; > > > > - mutex_lock(&dev->lock); > > + guard(mutex)(&dev->lock); > > + > > pdev = dev->pdev; > > if (pdev) > > ret = sprintf(page, "%s\n", dev_name(&pdev->dev)); > > else > > ret = sprintf(page, "gpio-sim.%d\n", dev->id); > > - mutex_unlock(&dev->lock); > > > > return ret; > > Now can be > > if (...) > return ... > else // if you wish (not needed) > return ... > > ... > > > int ret; > > > > - mutex_lock(&dev->lock); > > + guard(mutex)(&dev->lock); > > + > > if (gpio_sim_device_is_live_unlocked(dev)) > > ret = device_for_each_child(&dev->pdev->dev, &ctx, > > gpio_sim_emit_chip_name); > > else > > ret = sprintf(page, "none\n"); > > - mutex_unlock(&dev->lock); > > > > return ret; > > As per above. And may be other functions as well. > Sure. > ... > > > int ret; > > > > - mutex_lock(&dev->lock); > > - ret = sprintf(page, "%s\n", line->name ?: ""); > > - mutex_unlock(&dev->lock); > > + scoped_guard(mutex, &dev->lock) > > + ret = sprintf(page, "%s\n", line->name ?: ""); > > > > return ret; > > Why not > > guard(...); > return sprintf(...); > > ? I'll change that too. Bart > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-10 19:04 ` Bartosz Golaszewski @ 2023-08-11 9:14 ` Andy Shevchenko 2023-08-11 12:42 ` Bartosz Golaszewski 0 siblings, 1 reply; 18+ messages in thread From: Andy Shevchenko @ 2023-08-11 9:14 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Thu, Aug 10, 2023 at 09:04:12PM +0200, Bartosz Golaszewski wrote: > On Thu, Aug 10, 2023 at 4:42 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Aug 09, 2023 at 03:14:42PM +0200, Bartosz Golaszewski wrote: ... > > > + scoped_guard(mutex, &chip->lock) > > > + bitmap_replace(chip->value_map, chip->value_map, bits, mask, > > > + gc->ngpio); > > > > Perhaps with {} ? > > This scoped_guard() thing is in essence a for loop, so I believe > kernel coding style applies and a single statement doesn't require a > {}. You have two lines (or single wrapped line). I found to read it better with {}. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-11 9:14 ` Andy Shevchenko @ 2023-08-11 12:42 ` Bartosz Golaszewski 0 siblings, 0 replies; 18+ messages in thread From: Bartosz Golaszewski @ 2023-08-11 12:42 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Fri, Aug 11, 2023 at 11:14 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Aug 10, 2023 at 09:04:12PM +0200, Bartosz Golaszewski wrote: > > On Thu, Aug 10, 2023 at 4:42 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Wed, Aug 09, 2023 at 03:14:42PM +0200, Bartosz Golaszewski wrote: > > ... > > > > > + scoped_guard(mutex, &chip->lock) > > > > + bitmap_replace(chip->value_map, chip->value_map, bits, mask, > > > > + gc->ngpio); > > > > > > Perhaps with {} ? > > > > This scoped_guard() thing is in essence a for loop, so I believe > > kernel coding style applies and a single statement doesn't require a > > {}. > > You have two lines (or single wrapped line). I found to read it better with {}. > It's just a broken line, not a compound statement. Matter of personal taste. :) Bart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-09 13:14 ` [PATCH 2/2] gpio: sim: simplify code with cleanup helpers Bartosz Golaszewski 2023-08-10 14:39 ` Andy Shevchenko @ 2023-08-11 5:20 ` Dan Carpenter 2023-08-11 9:14 ` Andy Shevchenko 2023-08-15 8:04 ` Linus Walleij 2 siblings, 1 reply; 18+ messages in thread From: Dan Carpenter @ 2023-08-11 5:20 UTC (permalink / raw) To: oe-kbuild, Bartosz Golaszewski, Linus Walleij, Andy Shevchenko, Kent Gibson Cc: lkp, oe-kbuild-all, linux-gpio, linux-kernel, Bartosz Golaszewski Hi Bartosz, kernel test robot noticed the following build warnings: https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpio-sim-simplify-code-with-cleanup-helpers/20230809-211601 base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next patch link: https://lore.kernel.org/r/20230809131442.25524-2-brgl%40bgdev.pl patch subject: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers config: i386-randconfig-m021-20230809 (https://download.01.org/0day-ci/archive/20230811/202308110253.R2TUMfFr-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce: (https://download.01.org/0day-ci/archive/20230811/202308110253.R2TUMfFr-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202308110253.R2TUMfFr-lkp@intel.com/ smatch warnings: drivers/gpio/gpio-sim.c:1472 gpio_sim_config_make_device_group() warn: possible memory leak of 'dev' vim +/dev +1472 drivers/gpio/gpio-sim.c cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1460 static struct config_group * cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1461 gpio_sim_config_make_device_group(struct config_group *group, const char *name) cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1462 { c7a663cdcfc698 Bartosz Golaszewski 2023-08-09 1463 struct gpio_sim_device *dev __free(kfree) = NULL; cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1464 int id; cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1465 cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1466 dev = kzalloc(sizeof(*dev), GFP_KERNEL); cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1467 if (!dev) cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1468 return ERR_PTR(-ENOMEM); cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1469 cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1470 id = ida_alloc(&gpio_sim_ida, GFP_KERNEL); c7a663cdcfc698 Bartosz Golaszewski 2023-08-09 1471 if (id < 0) cb8c474e79be45 Bartosz Golaszewski 2021-12-07 @1472 return ERR_PTR(id); kfree(dev); cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1473 cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1474 config_group_init_type_name(&dev->group, name, cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1475 &gpio_sim_device_config_group_type); cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1476 dev->id = id; cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1477 mutex_init(&dev->lock); cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1478 INIT_LIST_HEAD(&dev->bank_list); cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1479 cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1480 dev->bus_notifier.notifier_call = gpio_sim_bus_notifier_call; cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1481 init_completion(&dev->probe_completion); cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1482 c7a663cdcfc698 Bartosz Golaszewski 2023-08-09 1483 return &no_free_ptr(dev)->group; cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1484 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-11 5:20 ` Dan Carpenter @ 2023-08-11 9:14 ` Andy Shevchenko 2023-08-11 9:31 ` Dan Carpenter 0 siblings, 1 reply; 18+ messages in thread From: Andy Shevchenko @ 2023-08-11 9:14 UTC (permalink / raw) To: Dan Carpenter Cc: oe-kbuild, Bartosz Golaszewski, Linus Walleij, Kent Gibson, lkp, oe-kbuild-all, linux-gpio, linux-kernel, Bartosz Golaszewski On Fri, Aug 11, 2023 at 08:20:11AM +0300, Dan Carpenter wrote: > smatch warnings: > drivers/gpio/gpio-sim.c:1472 gpio_sim_config_make_device_group() warn: possible memory leak of 'dev' Isn't smatch a bit dumb about cleanup.h? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-11 9:14 ` Andy Shevchenko @ 2023-08-11 9:31 ` Dan Carpenter 0 siblings, 0 replies; 18+ messages in thread From: Dan Carpenter @ 2023-08-11 9:31 UTC (permalink / raw) To: Andy Shevchenko Cc: oe-kbuild, Bartosz Golaszewski, Linus Walleij, Kent Gibson, lkp, oe-kbuild-all, linux-gpio, linux-kernel, Bartosz Golaszewski On Fri, Aug 11, 2023 at 12:14:52PM +0300, Andy Shevchenko wrote: > On Fri, Aug 11, 2023 at 08:20:11AM +0300, Dan Carpenter wrote: > > > smatch warnings: > > drivers/gpio/gpio-sim.c:1472 gpio_sim_config_make_device_group() warn: possible memory leak of 'dev' > > Isn't smatch a bit dumb about cleanup.h? > Aw. Crud. I hadn't seen that this was a cleanup.h thing. I did do some work to suppoort cleanup.h but probably it will take a while to work out the kinks. Let me figure this out. regards, dan carpenter ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-09 13:14 ` [PATCH 2/2] gpio: sim: simplify code with cleanup helpers Bartosz Golaszewski 2023-08-10 14:39 ` Andy Shevchenko 2023-08-11 5:20 ` Dan Carpenter @ 2023-08-15 8:04 ` Linus Walleij 2023-08-15 15:52 ` Peter Zijlstra 2 siblings, 1 reply; 18+ messages in thread From: Linus Walleij @ 2023-08-15 8:04 UTC (permalink / raw) To: Bartosz Golaszewski, Peter Zijlstra Cc: Andy Shevchenko, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Wed, Aug 9, 2023 at 3:14 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Use macros defined in linux/cleanup.h to automate resource lifetime > control in the gpio-simulator. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> (...) > - mutex_lock(&chip->lock); > + guard(mutex)(&chip->lock); (...) > - mutex_lock(&chip->lock); > - ret = !!test_bit(offset, chip->value_map); > - mutex_unlock(&chip->lock); > + scoped_guard(mutex, &chip->lock) > + ret = !!test_bit(offset, chip->value_map); This is really neat. When I grep:ed around in linux-next this seemed like the first user of the scoped guards, so maybe Peter Z want to take a look? I bet there is other code using it coming for this next merge window as well, but this is really the first that will land in linux-next as it seems. It looks good to me FWIW: Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-15 8:04 ` Linus Walleij @ 2023-08-15 15:52 ` Peter Zijlstra 2023-08-15 15:58 ` Andy Shevchenko 0 siblings, 1 reply; 18+ messages in thread From: Peter Zijlstra @ 2023-08-15 15:52 UTC (permalink / raw) To: Linus Walleij Cc: Bartosz Golaszewski, Andy Shevchenko, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Tue, Aug 15, 2023 at 10:04:32AM +0200, Linus Walleij wrote: > On Wed, Aug 9, 2023 at 3:14 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Use macros defined in linux/cleanup.h to automate resource lifetime > > control in the gpio-simulator. > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > (...) > > - mutex_lock(&chip->lock); > > + guard(mutex)(&chip->lock); > (...) > > - mutex_lock(&chip->lock); > > - ret = !!test_bit(offset, chip->value_map); > > - mutex_unlock(&chip->lock); > > + scoped_guard(mutex, &chip->lock) > > + ret = !!test_bit(offset, chip->value_map); > > This is really neat. When I grep:ed around in linux-next this seemed like > the first user of the scoped guards, so maybe Peter Z want to take a look? Looks about right. > I bet there is other code using it coming for this next merge window as > well, but this is really the first that will land in linux-next as it seems. There's more people starting to use it indeed. There should be some in tip/sched/core as well. I have more pending, but got side-tracked a bit with other things :/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-15 15:52 ` Peter Zijlstra @ 2023-08-15 15:58 ` Andy Shevchenko 2023-08-15 20:31 ` Peter Zijlstra 0 siblings, 1 reply; 18+ messages in thread From: Andy Shevchenko @ 2023-08-15 15:58 UTC (permalink / raw) To: Peter Zijlstra Cc: Linus Walleij, Bartosz Golaszewski, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Tue, Aug 15, 2023 at 05:52:53PM +0200, Peter Zijlstra wrote: > On Tue, Aug 15, 2023 at 10:04:32AM +0200, Linus Walleij wrote: > > On Wed, Aug 9, 2023 at 3:14 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > - mutex_lock(&chip->lock); > > > + guard(mutex)(&chip->lock); > Looks about right. Btw, why don't we have something like guard_mutex() to be used as guard_mutex(&chip->lock); Moreover, maybe some macro that can predict the API call from the type of the parameter? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-15 15:58 ` Andy Shevchenko @ 2023-08-15 20:31 ` Peter Zijlstra 2023-08-16 12:47 ` Peter Zijlstra 0 siblings, 1 reply; 18+ messages in thread From: Peter Zijlstra @ 2023-08-15 20:31 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Bartosz Golaszewski, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Tue, Aug 15, 2023 at 06:58:10PM +0300, Andy Shevchenko wrote: > On Tue, Aug 15, 2023 at 05:52:53PM +0200, Peter Zijlstra wrote: > > On Tue, Aug 15, 2023 at 10:04:32AM +0200, Linus Walleij wrote: > > > On Wed, Aug 9, 2023 at 3:14 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > - mutex_lock(&chip->lock); > > > > + guard(mutex)(&chip->lock); > > > Looks about right. > > Btw, why don't we have something like > > guard_mutex() > > to be used as > > guard_mutex(&chip->lock); Because this way I can write: DEFINE_LOCK_GUARD_1(rq_lock_irqsave, struct rq, rq_lock_irqsave(_T->lock, &_T->rf), rq_unlock_irqrestore(_T->lock, &_T->rf), struct rq_flags rf); And have: guard(rq_lock_irqsave)(rq); and scoped_guard (rq_lock_irqsave, rq) { } just work. And if you look in tip/sched/core, you'll find exactly this. Or look here: https://lkml.kernel.org/r/20230612090713.652690195@infradead.org for a bunch more examples -- I've wanted to get more of that merged, but alas, only 24h in a day and life got in the way. Defining local guard types is very useful. > Moreover, maybe some macro that can predict the API call from the type of > the parameter? The whole type inferrence in C is not extensible. That is, you get to write a single _Generic() statement, and every case that is included in it will work, but the moment you use a new type, one that is not included in your giant _Generic() statement, you're out of luck. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-15 20:31 ` Peter Zijlstra @ 2023-08-16 12:47 ` Peter Zijlstra 2023-08-17 9:21 ` Andy Shevchenko 0 siblings, 1 reply; 18+ messages in thread From: Peter Zijlstra @ 2023-08-16 12:47 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Bartosz Golaszewski, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Tue, Aug 15, 2023 at 10:31:17PM +0200, Peter Zijlstra wrote: > > Moreover, maybe some macro that can predict the API call from the type of > > the parameter? > > The whole type inferrence in C is not extensible. That is, you get to > write a single _Generic() statement, and every case that is included in > it will work, but the moment you use a new type, one that is not > included in your giant _Generic() statement, you're out of luck. Additionally, spinlock_t, does that map to spinlock, spinlock_irq or spinlock_irqsave ? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers 2023-08-16 12:47 ` Peter Zijlstra @ 2023-08-17 9:21 ` Andy Shevchenko 0 siblings, 0 replies; 18+ messages in thread From: Andy Shevchenko @ 2023-08-17 9:21 UTC (permalink / raw) To: Peter Zijlstra Cc: Linus Walleij, Bartosz Golaszewski, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Wed, Aug 16, 2023 at 02:47:57PM +0200, Peter Zijlstra wrote: > On Tue, Aug 15, 2023 at 10:31:17PM +0200, Peter Zijlstra wrote: > > > > Moreover, maybe some macro that can predict the API call from the type of > > > the parameter? > > > > The whole type inferrence in C is not extensible. That is, you get to > > write a single _Generic() statement, and every case that is included in > > it will work, but the moment you use a new type, one that is not > > included in your giant _Generic() statement, you're out of luck. > > Additionally, spinlock_t, does that map to spinlock, spinlock_irq or > spinlock_irqsave ? Thank you for a good explanation. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup() 2023-08-09 13:14 [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup() Bartosz Golaszewski 2023-08-09 13:14 ` [PATCH 2/2] gpio: sim: simplify code with cleanup helpers Bartosz Golaszewski @ 2023-08-10 14:04 ` Andy Shevchenko 2023-08-11 11:59 ` Bartosz Golaszewski 2 siblings, 0 replies; 18+ messages in thread From: Andy Shevchenko @ 2023-08-10 14:04 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, Kent Gibson, linux-gpio, linux-kernel, Bartosz Golaszewski On Wed, Aug 09, 2023 at 03:14:41PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > When comparing strings passed to us from configfs, we can pass the page > argument directly to sysfs_streq() and avoid manual string trimming. Good one! Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup() 2023-08-09 13:14 [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup() Bartosz Golaszewski 2023-08-09 13:14 ` [PATCH 2/2] gpio: sim: simplify code with cleanup helpers Bartosz Golaszewski 2023-08-10 14:04 ` [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup() Andy Shevchenko @ 2023-08-11 11:59 ` Bartosz Golaszewski 2 siblings, 0 replies; 18+ messages in thread From: Bartosz Golaszewski @ 2023-08-11 11:59 UTC (permalink / raw) To: Linus Walleij, Andy Shevchenko, Kent Gibson Cc: linux-gpio, linux-kernel, Bartosz Golaszewski On Wed, Aug 9, 2023 at 3:14 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > When comparing strings passed to us from configfs, we can pass the page > argument directly to sysfs_streq() and avoid manual string trimming. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- I applied this one, I'll send a v2 for other one. Bart ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers
@ 2023-08-10 19:05 kernel test robot
0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2023-08-10 19:05 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230809131442.25524-2-brgl@bgdev.pl>
References: <20230809131442.25524-2-brgl@bgdev.pl>
TO: Bartosz Golaszewski <brgl@bgdev.pl>
TO: Linus Walleij <linus.walleij@linaro.org>
TO: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
TO: Kent Gibson <warthog618@gmail.com>
CC: linux-gpio@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Hi Bartosz,
kernel test robot noticed the following build warnings:
[auto build test WARNING on brgl/gpio/for-next]
[also build test WARNING on linus/master v6.5-rc5 next-20230809]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpio-sim-simplify-code-with-cleanup-helpers/20230809-211601
base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next
patch link: https://lore.kernel.org/r/20230809131442.25524-2-brgl%40bgdev.pl
patch subject: [PATCH 2/2] gpio: sim: simplify code with cleanup helpers
:::::: branch date: 30 hours ago
:::::: commit date: 30 hours ago
config: i386-randconfig-m021-20230809 (https://download.01.org/0day-ci/archive/20230811/202308110253.R2TUMfFr-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230811/202308110253.R2TUMfFr-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202308110253.R2TUMfFr-lkp@intel.com/
smatch warnings:
drivers/gpio/gpio-sim.c:1472 gpio_sim_config_make_device_group() warn: possible memory leak of 'dev'
vim +/dev +1472 drivers/gpio/gpio-sim.c
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1459
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1460 static struct config_group *
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1461 gpio_sim_config_make_device_group(struct config_group *group, const char *name)
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1462 {
c7a663cdcfc698 Bartosz Golaszewski 2023-08-09 1463 struct gpio_sim_device *dev __free(kfree) = NULL;
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1464 int id;
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1465
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1466 dev = kzalloc(sizeof(*dev), GFP_KERNEL);
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1467 if (!dev)
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1468 return ERR_PTR(-ENOMEM);
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1469
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1470 id = ida_alloc(&gpio_sim_ida, GFP_KERNEL);
c7a663cdcfc698 Bartosz Golaszewski 2023-08-09 1471 if (id < 0)
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 @1472 return ERR_PTR(id);
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1473
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1474 config_group_init_type_name(&dev->group, name,
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1475 &gpio_sim_device_config_group_type);
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1476 dev->id = id;
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1477 mutex_init(&dev->lock);
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1478 INIT_LIST_HEAD(&dev->bank_list);
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1479
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1480 dev->bus_notifier.notifier_call = gpio_sim_bus_notifier_call;
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1481 init_completion(&dev->probe_completion);
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1482
c7a663cdcfc698 Bartosz Golaszewski 2023-08-09 1483 return &no_free_ptr(dev)->group;
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1484 }
cb8c474e79be45 Bartosz Golaszewski 2021-12-07 1485
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 18+ messages in threadend of thread, other threads:[~2023-08-17 9:22 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-09 13:14 [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup() Bartosz Golaszewski 2023-08-09 13:14 ` [PATCH 2/2] gpio: sim: simplify code with cleanup helpers Bartosz Golaszewski 2023-08-10 14:39 ` Andy Shevchenko 2023-08-10 19:04 ` Bartosz Golaszewski 2023-08-11 9:14 ` Andy Shevchenko 2023-08-11 12:42 ` Bartosz Golaszewski 2023-08-11 5:20 ` Dan Carpenter 2023-08-11 9:14 ` Andy Shevchenko 2023-08-11 9:31 ` Dan Carpenter 2023-08-15 8:04 ` Linus Walleij 2023-08-15 15:52 ` Peter Zijlstra 2023-08-15 15:58 ` Andy Shevchenko 2023-08-15 20:31 ` Peter Zijlstra 2023-08-16 12:47 ` Peter Zijlstra 2023-08-17 9:21 ` Andy Shevchenko 2023-08-10 14:04 ` [PATCH 1/2] gpio: sim: use sysfs_streq() and avoid an strdup() Andy Shevchenko 2023-08-11 11:59 ` Bartosz Golaszewski -- strict thread matches above, loose matches on Subject: below -- 2023-08-10 19:05 [PATCH 2/2] gpio: sim: simplify code with cleanup helpers kernel test robot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.