Merge series "regulator: fix deadlock vs memory reclaim" from Michał Mirosław <mirq-linux@rere.qmqm.pl>:

For systems that have eg. eMMC storage using voltage regulator, memory
reclaim path might call back into regulator subsystem. This means we
have to make sure no allocations happen with a regulator or regulator
list locked.

After this series I see no more lockdep complaints on my test system,
but please review and test further.

First four patches move allocations out of locked regions, next three
came as a drive-by cleanups.

---
v2: fix bug in patch #4 spotted by kernel test robot
    reworded commit #7 description

Michał Mirosław (7):
  regulator: push allocation in regulator_init_coupling() outside of
    lock
  regulator: push allocation in regulator_ena_gpio_request() out of lock
  regulator: push allocations in create_regulator() outside of lock
  regulator: push allocation in set_consumer_device_supply() out of lock
  regulator: plug of_node leak in regulator_register()'s error path
  regulator: cleanup regulator_ena_gpio_free()
  regulator: remove superfluous lock in regulator_resolve_coupling()

 drivers/regulator/core.c | 164 +++++++++++++++++++++------------------
 1 file changed, 87 insertions(+), 77 deletions(-)

--
2.20.1
This commit is contained in:
Mark Brown 2020-08-18 17:18:46 +01:00
commit 4c13f934e3
No known key found for this signature in database
GPG Key ID: 24D68B725D5487D0

View File

@ -1480,7 +1480,7 @@ static int set_consumer_device_supply(struct regulator_dev *rdev,
const char *consumer_dev_name,
const char *supply)
{
struct regulator_map *node;
struct regulator_map *node, *new_node;
int has_dev;
if (supply == NULL)
@ -1491,6 +1491,22 @@ static int set_consumer_device_supply(struct regulator_dev *rdev,
else
has_dev = 0;
new_node = kzalloc(sizeof(struct regulator_map), GFP_KERNEL);
if (new_node == NULL)
return -ENOMEM;
new_node->regulator = rdev;
new_node->supply = supply;
if (has_dev) {
new_node->dev_name = kstrdup(consumer_dev_name, GFP_KERNEL);
if (new_node->dev_name == NULL) {
kfree(new_node);
return -ENOMEM;
}
}
mutex_lock(&regulator_list_mutex);
list_for_each_entry(node, &regulator_map_list, list) {
if (node->dev_name && consumer_dev_name) {
if (strcmp(node->dev_name, consumer_dev_name) != 0)
@ -1508,26 +1524,19 @@ static int set_consumer_device_supply(struct regulator_dev *rdev,
node->regulator->desc->name,
supply,
dev_name(&rdev->dev), rdev_get_name(rdev));
return -EBUSY;
goto fail;
}
node = kzalloc(sizeof(struct regulator_map), GFP_KERNEL);
if (node == NULL)
return -ENOMEM;
list_add(&new_node->list, &regulator_map_list);
mutex_unlock(&regulator_list_mutex);
node->regulator = rdev;
node->supply = supply;
if (has_dev) {
node->dev_name = kstrdup(consumer_dev_name, GFP_KERNEL);
if (node->dev_name == NULL) {
kfree(node);
return -ENOMEM;
}
}
list_add(&node->list, &regulator_map_list);
return 0;
fail:
mutex_unlock(&regulator_list_mutex);
kfree(new_node->dev_name);
kfree(new_node);
return -EBUSY;
}
static void unset_regulator_supplies(struct regulator_dev *rdev)
@ -1599,44 +1608,53 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
const char *supply_name)
{
struct regulator *regulator;
char buf[REG_STR_SIZE];
int err, size;
int err;
if (dev) {
char buf[REG_STR_SIZE];
int size;
size = snprintf(buf, REG_STR_SIZE, "%s-%s",
dev->kobj.name, supply_name);
if (size >= REG_STR_SIZE)
return NULL;
supply_name = kstrdup(buf, GFP_KERNEL);
if (supply_name == NULL)
return NULL;
} else {
supply_name = kstrdup_const(supply_name, GFP_KERNEL);
if (supply_name == NULL)
return NULL;
}
regulator = kzalloc(sizeof(*regulator), GFP_KERNEL);
if (regulator == NULL)
if (regulator == NULL) {
kfree(supply_name);
return NULL;
}
regulator->rdev = rdev;
regulator->supply_name = supply_name;
regulator_lock(rdev);
regulator->rdev = rdev;
list_add(&regulator->list, &rdev->consumer_list);
regulator_unlock(rdev);
if (dev) {
regulator->dev = dev;
/* Add a link to the device sysfs entry */
size = snprintf(buf, REG_STR_SIZE, "%s-%s",
dev->kobj.name, supply_name);
if (size >= REG_STR_SIZE)
goto overflow_err;
regulator->supply_name = kstrdup(buf, GFP_KERNEL);
if (regulator->supply_name == NULL)
goto overflow_err;
err = sysfs_create_link_nowarn(&rdev->dev.kobj, &dev->kobj,
buf);
supply_name);
if (err) {
rdev_dbg(rdev, "could not add device link %s err %d\n",
dev->kobj.name, err);
/* non-fatal */
}
} else {
regulator->supply_name = kstrdup_const(supply_name, GFP_KERNEL);
if (regulator->supply_name == NULL)
goto overflow_err;
}
regulator->debugfs = debugfs_create_dir(regulator->supply_name,
regulator->debugfs = debugfs_create_dir(supply_name,
rdev->debugfs);
if (!regulator->debugfs) {
rdev_dbg(rdev, "Failed to create debugfs directory\n");
@ -1661,13 +1679,7 @@ static struct regulator *create_regulator(struct regulator_dev *rdev,
_regulator_is_enabled(rdev))
regulator->always_on = true;
regulator_unlock(rdev);
return regulator;
overflow_err:
list_del(&regulator->list);
kfree(regulator);
regulator_unlock(rdev);
return NULL;
}
static int _regulator_get_enable_time(struct regulator_dev *rdev)
@ -2249,10 +2261,13 @@ EXPORT_SYMBOL_GPL(regulator_bulk_unregister_supply_alias);
static int regulator_ena_gpio_request(struct regulator_dev *rdev,
const struct regulator_config *config)
{
struct regulator_enable_gpio *pin;
struct regulator_enable_gpio *pin, *new_pin;
struct gpio_desc *gpiod;
gpiod = config->ena_gpiod;
new_pin = kzalloc(sizeof(*new_pin), GFP_KERNEL);
mutex_lock(&regulator_list_mutex);
list_for_each_entry(pin, &regulator_ena_gpio_list, list) {
if (pin->gpiod == gpiod) {
@ -2261,9 +2276,13 @@ static int regulator_ena_gpio_request(struct regulator_dev *rdev,
}
}
pin = kzalloc(sizeof(struct regulator_enable_gpio), GFP_KERNEL);
if (pin == NULL)
if (new_pin == NULL) {
mutex_unlock(&regulator_list_mutex);
return -ENOMEM;
}
pin = new_pin;
new_pin = NULL;
pin->gpiod = gpiod;
list_add(&pin->list, &regulator_ena_gpio_list);
@ -2271,6 +2290,10 @@ static int regulator_ena_gpio_request(struct regulator_dev *rdev,
update_ena_gpio_to_rdev:
pin->request_count++;
rdev->ena_pin = pin;
mutex_unlock(&regulator_list_mutex);
kfree(new_pin);
return 0;
}
@ -2283,19 +2306,19 @@ static void regulator_ena_gpio_free(struct regulator_dev *rdev)
/* Free the GPIO only in case of no use */
list_for_each_entry_safe(pin, n, &regulator_ena_gpio_list, list) {
if (pin->gpiod == rdev->ena_pin->gpiod) {
if (pin->request_count <= 1) {
pin->request_count = 0;
gpiod_put(pin->gpiod);
list_del(&pin->list);
kfree(pin);
rdev->ena_pin = NULL;
return;
} else {
pin->request_count--;
}
}
if (pin != rdev->ena_pin)
continue;
if (--pin->request_count)
break;
gpiod_put(pin->gpiod);
list_del(&pin->list);
kfree(pin);
break;
}
rdev->ena_pin = NULL;
}
/**
@ -5059,7 +5082,10 @@ static int regulator_init_coupling(struct regulator_dev *rdev)
if (!of_check_coupling_data(rdev))
return -EPERM;
mutex_lock(&regulator_list_mutex);
rdev->coupling_desc.coupler = regulator_find_coupler(rdev);
mutex_unlock(&regulator_list_mutex);
if (IS_ERR(rdev->coupling_desc.coupler)) {
err = PTR_ERR(rdev->coupling_desc.coupler);
rdev_err(rdev, "failed to get coupler: %d\n", err);
@ -5160,6 +5186,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
ret = -ENOMEM;
goto rinse;
}
device_initialize(&rdev->dev);
/*
* Duplicate the config so the driver could override it after
@ -5167,9 +5194,8 @@ regulator_register(const struct regulator_desc *regulator_desc,
*/
config = kmemdup(cfg, sizeof(*cfg), GFP_KERNEL);
if (config == NULL) {
kfree(rdev);
ret = -ENOMEM;
goto rinse;
goto clean;
}
init_data = regulator_of_get_init_data(dev, regulator_desc, config,
@ -5181,10 +5207,8 @@ regulator_register(const struct regulator_desc *regulator_desc,
* from a gpio extender or something else.
*/
if (PTR_ERR(init_data) == -EPROBE_DEFER) {
kfree(config);
kfree(rdev);
ret = -EPROBE_DEFER;
goto rinse;
goto clean;
}
/*
@ -5225,9 +5249,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
}
if (config->ena_gpiod) {
mutex_lock(&regulator_list_mutex);
ret = regulator_ena_gpio_request(rdev, config);
mutex_unlock(&regulator_list_mutex);
if (ret != 0) {
rdev_err(rdev, "Failed to request enable GPIO: %d\n",
ret);
@ -5239,7 +5261,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
}
/* register with sysfs */
device_initialize(&rdev->dev);
rdev->dev.class = &regulator_class;
rdev->dev.parent = dev;
dev_set_name(&rdev->dev, "regulator.%lu",
@ -5267,27 +5288,22 @@ regulator_register(const struct regulator_desc *regulator_desc,
if (ret < 0)
goto wash;
mutex_lock(&regulator_list_mutex);
ret = regulator_init_coupling(rdev);
mutex_unlock(&regulator_list_mutex);
if (ret < 0)
goto wash;
/* add consumers devices */
if (init_data) {
mutex_lock(&regulator_list_mutex);
for (i = 0; i < init_data->num_consumer_supplies; i++) {
ret = set_consumer_device_supply(rdev,
init_data->consumer_supplies[i].dev_name,
init_data->consumer_supplies[i].supply);
if (ret < 0) {
mutex_unlock(&regulator_list_mutex);
dev_err(dev, "Failed to set supply %s\n",
init_data->consumer_supplies[i].supply);
goto unset_supplies;
}
}
mutex_unlock(&regulator_list_mutex);
}
if (!rdev->desc->ops->get_voltage &&
@ -5322,13 +5338,11 @@ regulator_register(const struct regulator_desc *regulator_desc,
mutex_lock(&regulator_list_mutex);
regulator_ena_gpio_free(rdev);
mutex_unlock(&regulator_list_mutex);
put_device(&rdev->dev);
rdev = NULL;
clean:
if (dangling_of_gpiod)
gpiod_put(config->ena_gpiod);
kfree(rdev);
kfree(config);
put_device(&rdev->dev);
rinse:
if (dangling_cfg_gpiod)
gpiod_put(cfg->ena_gpiod);