Commit 7765ffed authored by Bartosz Golaszewski's avatar Bartosz Golaszewski
Browse files

gpiolib: use a single SRCU struct for all GPIO descriptors 



We used a per-descriptor SRCU struct in order to not impose a wait with
synchronize_srcu() for descriptor X on read-only operations of
descriptor Y. Now that we no longer call synchronize_srcu() on
descriptor label change but only when releasing descriptor resources, we
can use a single SRCU structure for all GPIO descriptors in a given chip.

Suggested-by: default avatar"Paul E. McKenney" <paulmck@kernel.org>
Acked-by: default avatar"Paul E. McKenney" <paulmck@kernel.org>
Link: https://lore.kernel.org/r/20240507172414.28513-1-brgl@bgdev.pl


Signed-off-by: default avatarBartosz Golaszewski <bartosz.golaszewski@linaro.org>
parent a86d2769

drivers/gpio/gpiolib-cdev.c

+1 −1
Original line number Diff line number Diff line
@@ -2351,7 +2351,7 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc, 


	dflags = READ_ONCE(desc->flags);



	scoped_guard(srcu, &desc->srcu) {

	scoped_guard(srcu, &desc->gdev->desc_srcu) {

		label = gpiod_get_label(desc);

		if (label && test_bit(FLAG_REQUESTED, &dflags))

			strscpy(info->consumer, label,

drivers/gpio/gpiolib.c

+19 −22
Original line number Diff line number Diff line
@@ -112,8 +112,8 @@ const char *gpiod_get_label(struct gpio_desc *desc) 
	if (!test_bit(FLAG_REQUESTED, &flags))

		return NULL;



	label = srcu_dereference_check(desc->label, &desc->srcu,

				       srcu_read_lock_held(&desc->srcu));

	label = srcu_dereference_check(desc->label, &desc->gdev->desc_srcu,

				srcu_read_lock_held(&desc->gdev->desc_srcu));



	return label->str;

}
@@ -138,7 +138,7 @@ static int desc_set_label(struct gpio_desc *desc, const char *label) 


	old = rcu_replace_pointer(desc->label, new, 1);

	if (old)

		call_srcu(&desc->srcu, &old->rh, desc_free_label);

		call_srcu(&desc->gdev->desc_srcu, &old->rh, desc_free_label);



	return 0;

}
@@ -709,13 +709,10 @@ EXPORT_SYMBOL_GPL(gpiochip_line_is_valid); 
static void gpiodev_release(struct device *dev)

{

	struct gpio_device *gdev = to_gpio_device(dev);

	unsigned int i;



	for (i = 0; i < gdev->ngpio; i++) {

		/* Free pending label. */

		synchronize_srcu(&gdev->descs[i].srcu);

		cleanup_srcu_struct(&gdev->descs[i].srcu);

	}

	/* Call pending kfree()s for descriptor labels. */

	synchronize_srcu(&gdev->desc_srcu);

	cleanup_srcu_struct(&gdev->desc_srcu);



	ida_free(&gpio_ida, gdev->id);

	kfree_const(gdev->label);
@@ -992,6 +989,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 
	if (ret)

		goto err_remove_from_list;



	ret = init_srcu_struct(&gdev->desc_srcu);

	if (ret)

		goto err_cleanup_gdev_srcu;



#ifdef CONFIG_PINCTRL

	INIT_LIST_HEAD(&gdev->pin_ranges);

#endif
@@ -999,23 +1000,19 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 
	if (gc->names) {

		ret = gpiochip_set_desc_names(gc);

		if (ret)

			goto err_cleanup_gdev_srcu;

			goto err_cleanup_desc_srcu;

	}

	ret = gpiochip_set_names(gc);

	if (ret)

		goto err_cleanup_gdev_srcu;

		goto err_cleanup_desc_srcu;



	ret = gpiochip_init_valid_mask(gc);

	if (ret)

		goto err_cleanup_gdev_srcu;

		goto err_cleanup_desc_srcu;



	for (desc_index = 0; desc_index < gc->ngpio; desc_index++) {

		struct gpio_desc *desc = &gdev->descs[desc_index];



		ret = init_srcu_struct(&desc->srcu);

		if (ret)

			goto err_cleanup_desc_srcu;



		if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index)) {

			assign_bit(FLAG_IS_OUT,

				   &desc->flags, !gc->get_direction(gc, desc_index));
@@ -1027,7 +1024,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 


	ret = of_gpiochip_add(gc);

	if (ret)

		goto err_cleanup_desc_srcu;

		goto err_free_valid_mask;



	ret = gpiochip_add_pin_ranges(gc);

	if (ret)
@@ -1074,10 +1071,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data, 
	gpiochip_remove_pin_ranges(gc);

err_remove_of_chip:

	of_gpiochip_remove(gc);

err_cleanup_desc_srcu:

	while (desc_index--)

		cleanup_srcu_struct(&gdev->descs[desc_index].srcu);

err_free_valid_mask:

	gpiochip_free_valid_mask(gc);

err_cleanup_desc_srcu:

	cleanup_srcu_struct(&gdev->desc_srcu);

err_cleanup_gdev_srcu:

	cleanup_srcu_struct(&gdev->srcu);

err_remove_from_list:
@@ -2407,7 +2404,7 @@ char *gpiochip_dup_line_label(struct gpio_chip *gc, unsigned int offset) 
	if (!test_bit(FLAG_REQUESTED, &desc->flags))

		return NULL;



	guard(srcu)(&desc->srcu);

	guard(srcu)(&desc->gdev->desc_srcu);



	label = kstrdup(gpiod_get_label(desc), GFP_KERNEL);

	if (!label)
@@ -4798,7 +4795,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev) 
	}



	for_each_gpio_desc(gc, desc) {

		guard(srcu)(&desc->srcu);

		guard(srcu)(&desc->gdev->desc_srcu);

		if (test_bit(FLAG_REQUESTED, &desc->flags)) {

			gpiod_get_direction(desc);

			is_out = test_bit(FLAG_IS_OUT, &desc->flags);

drivers/gpio/gpiolib.h

+5 −5
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ 
 * @chip: pointer to the corresponding gpiochip, holding static

 * data for this device

 * @descs: array of ngpio descriptors.

 * @desc_srcu: ensures consistent state of GPIO descriptors exposed to users

 * @ngpio: the number of GPIO lines on this GPIO device, equal to the size

 * of the @descs array.

 * @can_sleep: indicate whether the GPIO chip driver's callbacks can sleep
@@ -61,6 +62,7 @@ struct gpio_device { 
	struct module		*owner;

	struct gpio_chip __rcu	*chip;

	struct gpio_desc	*descs;

	struct srcu_struct	desc_srcu;

	int			base;

	u16			ngpio;

	bool			can_sleep;
@@ -150,7 +152,6 @@ struct gpio_desc_label { 
 * @label:		Name of the consumer

 * @name:		Line name

 * @hog:		Pointer to the device node that hogs this line (if any)

 * @srcu:		SRCU struct protecting the label pointer.

 *

 * These are obtained using gpiod_get() and are preferable to the old

 * integer-based handles.
@@ -188,7 +189,6 @@ struct gpio_desc { 
#ifdef CONFIG_OF_DYNAMIC

	struct device_node	*hog;

#endif

	struct srcu_struct	srcu;

};



#define gpiod_not_found(desc)		(IS_ERR(desc) && PTR_ERR(desc) == -ENOENT)
@@ -256,7 +256,7 @@ static inline int gpio_chip_hwgpio(const struct gpio_desc *desc) 


#define gpiod_err(desc, fmt, ...) \

do { \

	scoped_guard(srcu, &desc->srcu) { \

	scoped_guard(srcu, &desc->gdev->desc_srcu) { \

		pr_err("gpio-%d (%s): " fmt, desc_to_gpio(desc), \

		       gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \

	} \
@@ -264,7 +264,7 @@ do { \ 


#define gpiod_warn(desc, fmt, ...) \

do { \

	scoped_guard(srcu, &desc->srcu) { \

	scoped_guard(srcu, &desc->gdev->desc_srcu) { \

		pr_warn("gpio-%d (%s): " fmt, desc_to_gpio(desc), \

			gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \

	} \
@@ -272,7 +272,7 @@ do { \ 


#define gpiod_dbg(desc, fmt, ...) \

do { \

	scoped_guard(srcu, &desc->srcu) { \

	scoped_guard(srcu, &desc->gdev->desc_srcu) { \

		pr_debug("gpio-%d (%s): " fmt, desc_to_gpio(desc), \

			 gpiod_get_label(desc) ? : "?", ##__VA_ARGS__); \

	} \