linux/drivers/pwm/sysfs.c
Phong Hoang 347ab94803 pwm: Fix deadlock warning when removing PWM device
This patch fixes deadlock warning if removing PWM device
when CONFIG_PROVE_LOCKING is enabled.

This issue can be reproceduced by the following steps on
the R-Car H3 Salvator-X board if the backlight is disabled:

 # cd /sys/class/pwm/pwmchip0
 # echo 0 > export
 # ls
 device  export  npwm  power  pwm0  subsystem  uevent  unexport
 # cd device/driver
 # ls
 bind  e6e31000.pwm  uevent  unbind
 # echo e6e31000.pwm > unbind

[   87.659974] ======================================================
[   87.666149] WARNING: possible circular locking dependency detected
[   87.672327] 5.0.0 #7 Not tainted
[   87.675549] ------------------------------------------------------
[   87.681723] bash/2986 is trying to acquire lock:
[   87.686337] 000000005ea0e178 (kn->count#58){++++}, at: kernfs_remove_by_name_ns+0x50/0xa0
[   87.694528]
[   87.694528] but task is already holding lock:
[   87.700353] 000000006313b17c (pwm_lock){+.+.}, at: pwmchip_remove+0x28/0x13c
[   87.707405]
[   87.707405] which lock already depends on the new lock.
[   87.707405]
[   87.715574]
[   87.715574] the existing dependency chain (in reverse order) is:
[   87.723048]
[   87.723048] -> #1 (pwm_lock){+.+.}:
[   87.728017]        __mutex_lock+0x70/0x7e4
[   87.732108]        mutex_lock_nested+0x1c/0x24
[   87.736547]        pwm_request_from_chip.part.6+0x34/0x74
[   87.741940]        pwm_request_from_chip+0x20/0x40
[   87.746725]        export_store+0x6c/0x1f4
[   87.750820]        dev_attr_store+0x18/0x28
[   87.754998]        sysfs_kf_write+0x54/0x64
[   87.759175]        kernfs_fop_write+0xe4/0x1e8
[   87.763615]        __vfs_write+0x40/0x184
[   87.767619]        vfs_write+0xa8/0x19c
[   87.771448]        ksys_write+0x58/0xbc
[   87.775278]        __arm64_sys_write+0x18/0x20
[   87.779721]        el0_svc_common+0xd0/0x124
[   87.783986]        el0_svc_compat_handler+0x1c/0x24
[   87.788858]        el0_svc_compat+0x8/0x18
[   87.792947]
[   87.792947] -> #0 (kn->count#58){++++}:
[   87.798260]        lock_acquire+0xc4/0x22c
[   87.802353]        __kernfs_remove+0x258/0x2c4
[   87.806790]        kernfs_remove_by_name_ns+0x50/0xa0
[   87.811836]        remove_files.isra.1+0x38/0x78
[   87.816447]        sysfs_remove_group+0x48/0x98
[   87.820971]        sysfs_remove_groups+0x34/0x4c
[   87.825583]        device_remove_attrs+0x6c/0x7c
[   87.830197]        device_del+0x11c/0x33c
[   87.834201]        device_unregister+0x14/0x2c
[   87.838638]        pwmchip_sysfs_unexport+0x40/0x4c
[   87.843509]        pwmchip_remove+0xf4/0x13c
[   87.847773]        rcar_pwm_remove+0x28/0x34
[   87.852039]        platform_drv_remove+0x24/0x64
[   87.856651]        device_release_driver_internal+0x18c/0x21c
[   87.862391]        device_release_driver+0x14/0x1c
[   87.867175]        unbind_store+0xe0/0x124
[   87.871265]        drv_attr_store+0x20/0x30
[   87.875442]        sysfs_kf_write+0x54/0x64
[   87.879618]        kernfs_fop_write+0xe4/0x1e8
[   87.884055]        __vfs_write+0x40/0x184
[   87.888057]        vfs_write+0xa8/0x19c
[   87.891887]        ksys_write+0x58/0xbc
[   87.895716]        __arm64_sys_write+0x18/0x20
[   87.900154]        el0_svc_common+0xd0/0x124
[   87.904417]        el0_svc_compat_handler+0x1c/0x24
[   87.909289]        el0_svc_compat+0x8/0x18
[   87.913378]
[   87.913378] other info that might help us debug this:
[   87.913378]
[   87.921374]  Possible unsafe locking scenario:
[   87.921374]
[   87.927286]        CPU0                    CPU1
[   87.931808]        ----                    ----
[   87.936331]   lock(pwm_lock);
[   87.939293]                                lock(kn->count#58);
[   87.945120]                                lock(pwm_lock);
[   87.950599]   lock(kn->count#58);
[   87.953908]
[   87.953908]  *** DEADLOCK ***
[   87.953908]
[   87.959821] 4 locks held by bash/2986:
[   87.963563]  #0: 00000000ace7bc30 (sb_writers#6){.+.+}, at: vfs_write+0x188/0x19c
[   87.971044]  #1: 00000000287991b2 (&of->mutex){+.+.}, at: kernfs_fop_write+0xb4/0x1e8
[   87.978872]  #2: 00000000f739d016 (&dev->mutex){....}, at: device_release_driver_internal+0x40/0x21c
[   87.988001]  #3: 000000006313b17c (pwm_lock){+.+.}, at: pwmchip_remove+0x28/0x13c
[   87.995481]
[   87.995481] stack backtrace:
[   87.999836] CPU: 0 PID: 2986 Comm: bash Not tainted 5.0.0 #7
[   88.005489] Hardware name: Renesas Salvator-X board based on r8a7795 ES1.x (DT)
[   88.012791] Call trace:
[   88.015235]  dump_backtrace+0x0/0x190
[   88.018891]  show_stack+0x14/0x1c
[   88.022204]  dump_stack+0xb0/0xec
[   88.025514]  print_circular_bug.isra.32+0x1d0/0x2e0
[   88.030385]  __lock_acquire+0x1318/0x1864
[   88.034388]  lock_acquire+0xc4/0x22c
[   88.037958]  __kernfs_remove+0x258/0x2c4
[   88.041874]  kernfs_remove_by_name_ns+0x50/0xa0
[   88.046398]  remove_files.isra.1+0x38/0x78
[   88.050487]  sysfs_remove_group+0x48/0x98
[   88.054490]  sysfs_remove_groups+0x34/0x4c
[   88.058580]  device_remove_attrs+0x6c/0x7c
[   88.062671]  device_del+0x11c/0x33c
[   88.066154]  device_unregister+0x14/0x2c
[   88.070070]  pwmchip_sysfs_unexport+0x40/0x4c
[   88.074421]  pwmchip_remove+0xf4/0x13c
[   88.078163]  rcar_pwm_remove+0x28/0x34
[   88.081906]  platform_drv_remove+0x24/0x64
[   88.085996]  device_release_driver_internal+0x18c/0x21c
[   88.091215]  device_release_driver+0x14/0x1c
[   88.095478]  unbind_store+0xe0/0x124
[   88.099048]  drv_attr_store+0x20/0x30
[   88.102704]  sysfs_kf_write+0x54/0x64
[   88.106359]  kernfs_fop_write+0xe4/0x1e8
[   88.110275]  __vfs_write+0x40/0x184
[   88.113757]  vfs_write+0xa8/0x19c
[   88.117065]  ksys_write+0x58/0xbc
[   88.120374]  __arm64_sys_write+0x18/0x20
[   88.124291]  el0_svc_common+0xd0/0x124
[   88.128034]  el0_svc_compat_handler+0x1c/0x24
[   88.132384]  el0_svc_compat+0x8/0x18

The sysfs unexport in pwmchip_remove() is completely asymmetric
to what we do in pwmchip_add_with_polarity() and commit 0733424c9b
("pwm: Unexport children before chip removal") is a strong indication
that this was wrong to begin with. We should just move
pwmchip_sysfs_unexport() where it belongs, which is right after
pwmchip_sysfs_unexport_children(). In that case, we do not need
separate functions anymore either.

We also really want to remove sysfs irrespective of whether or not
the chip will be removed as a result of pwmchip_remove(). We can only
assume that the driver will be gone after that, so we shouldn't leave
any dangling sysfs files around.

This warning disappears if we move pwmchip_sysfs_unexport() to
the top of pwmchip_remove(), pwmchip_sysfs_unexport_children().
That way it is also outside of the pwm_lock section, which indeed
doesn't seem to be needed.

Moving the pwmchip_sysfs_export() call outside of that section also
seems fine and it'd be perfectly symmetric with pwmchip_remove() again.

So, this patch fixes them.

Signed-off-by: Phong Hoang <phong.hoang.wz@renesas.com>
[shimoda: revise the commit log and code]
Fixes: 76abbdde2d ("pwm: Add sysfs interface")
Fixes: 0733424c9b ("pwm: Unexport children before chip removal")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Tested-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
2019-03-20 12:21:31 +01:00

437 lines
9.5 KiB
C

/*
* A simple sysfs interface for the generic PWM framework
*
* Copyright (C) 2013 H Hartley Sweeten <hsweeten@visionengravers.com>
*
* Based on previous work by Lars Poeschel <poeschel@lemonage.de>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2, or (at your option)
* any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
#include <linux/device.h>
#include <linux/mutex.h>
#include <linux/err.h>
#include <linux/slab.h>
#include <linux/kdev_t.h>
#include <linux/pwm.h>
struct pwm_export {
struct device child;
struct pwm_device *pwm;
struct mutex lock;
};
static struct pwm_export *child_to_pwm_export(struct device *child)
{
return container_of(child, struct pwm_export, child);
}
static struct pwm_device *child_to_pwm_device(struct device *child)
{
struct pwm_export *export = child_to_pwm_export(child);
return export->pwm;
}
static ssize_t period_show(struct device *child,
struct device_attribute *attr,
char *buf)
{
const struct pwm_device *pwm = child_to_pwm_device(child);
struct pwm_state state;
pwm_get_state(pwm, &state);
return sprintf(buf, "%u\n", state.period);
}
static ssize_t period_store(struct device *child,
struct device_attribute *attr,
const char *buf, size_t size)
{
struct pwm_export *export = child_to_pwm_export(child);
struct pwm_device *pwm = export->pwm;
struct pwm_state state;
unsigned int val;
int ret;
ret = kstrtouint(buf, 0, &val);
if (ret)
return ret;
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
state.period = val;
ret = pwm_apply_state(pwm, &state);
mutex_unlock(&export->lock);
return ret ? : size;
}
static ssize_t duty_cycle_show(struct device *child,
struct device_attribute *attr,
char *buf)
{
const struct pwm_device *pwm = child_to_pwm_device(child);
struct pwm_state state;
pwm_get_state(pwm, &state);
return sprintf(buf, "%u\n", state.duty_cycle);
}
static ssize_t duty_cycle_store(struct device *child,
struct device_attribute *attr,
const char *buf, size_t size)
{
struct pwm_export *export = child_to_pwm_export(child);
struct pwm_device *pwm = export->pwm;
struct pwm_state state;
unsigned int val;
int ret;
ret = kstrtouint(buf, 0, &val);
if (ret)
return ret;
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
state.duty_cycle = val;
ret = pwm_apply_state(pwm, &state);
mutex_unlock(&export->lock);
return ret ? : size;
}
static ssize_t enable_show(struct device *child,
struct device_attribute *attr,
char *buf)
{
const struct pwm_device *pwm = child_to_pwm_device(child);
struct pwm_state state;
pwm_get_state(pwm, &state);
return sprintf(buf, "%d\n", state.enabled);
}
static ssize_t enable_store(struct device *child,
struct device_attribute *attr,
const char *buf, size_t size)
{
struct pwm_export *export = child_to_pwm_export(child);
struct pwm_device *pwm = export->pwm;
struct pwm_state state;
int val, ret;
ret = kstrtoint(buf, 0, &val);
if (ret)
return ret;
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
switch (val) {
case 0:
state.enabled = false;
break;
case 1:
state.enabled = true;
break;
default:
ret = -EINVAL;
goto unlock;
}
ret = pwm_apply_state(pwm, &state);
unlock:
mutex_unlock(&export->lock);
return ret ? : size;
}
static ssize_t polarity_show(struct device *child,
struct device_attribute *attr,
char *buf)
{
const struct pwm_device *pwm = child_to_pwm_device(child);
const char *polarity = "unknown";
struct pwm_state state;
pwm_get_state(pwm, &state);
switch (state.polarity) {
case PWM_POLARITY_NORMAL:
polarity = "normal";
break;
case PWM_POLARITY_INVERSED:
polarity = "inversed";
break;
}
return sprintf(buf, "%s\n", polarity);
}
static ssize_t polarity_store(struct device *child,
struct device_attribute *attr,
const char *buf, size_t size)
{
struct pwm_export *export = child_to_pwm_export(child);
struct pwm_device *pwm = export->pwm;
enum pwm_polarity polarity;
struct pwm_state state;
int ret;
if (sysfs_streq(buf, "normal"))
polarity = PWM_POLARITY_NORMAL;
else if (sysfs_streq(buf, "inversed"))
polarity = PWM_POLARITY_INVERSED;
else
return -EINVAL;
mutex_lock(&export->lock);
pwm_get_state(pwm, &state);
state.polarity = polarity;
ret = pwm_apply_state(pwm, &state);
mutex_unlock(&export->lock);
return ret ? : size;
}
static ssize_t capture_show(struct device *child,
struct device_attribute *attr,
char *buf)
{
struct pwm_device *pwm = child_to_pwm_device(child);
struct pwm_capture result;
int ret;
ret = pwm_capture(pwm, &result, jiffies_to_msecs(HZ));
if (ret)
return ret;
return sprintf(buf, "%u %u\n", result.period, result.duty_cycle);
}
static DEVICE_ATTR_RW(period);
static DEVICE_ATTR_RW(duty_cycle);
static DEVICE_ATTR_RW(enable);
static DEVICE_ATTR_RW(polarity);
static DEVICE_ATTR_RO(capture);
static struct attribute *pwm_attrs[] = {
&dev_attr_period.attr,
&dev_attr_duty_cycle.attr,
&dev_attr_enable.attr,
&dev_attr_polarity.attr,
&dev_attr_capture.attr,
NULL
};
ATTRIBUTE_GROUPS(pwm);
static void pwm_export_release(struct device *child)
{
struct pwm_export *export = child_to_pwm_export(child);
kfree(export);
}
static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
{
struct pwm_export *export;
char *pwm_prop[2];
int ret;
if (test_and_set_bit(PWMF_EXPORTED, &pwm->flags))
return -EBUSY;
export = kzalloc(sizeof(*export), GFP_KERNEL);
if (!export) {
clear_bit(PWMF_EXPORTED, &pwm->flags);
return -ENOMEM;
}
export->pwm = pwm;
mutex_init(&export->lock);
export->child.release = pwm_export_release;
export->child.parent = parent;
export->child.devt = MKDEV(0, 0);
export->child.groups = pwm_groups;
dev_set_name(&export->child, "pwm%u", pwm->hwpwm);
ret = device_register(&export->child);
if (ret) {
clear_bit(PWMF_EXPORTED, &pwm->flags);
put_device(&export->child);
export = NULL;
return ret;
}
pwm_prop[0] = kasprintf(GFP_KERNEL, "EXPORT=pwm%u", pwm->hwpwm);
pwm_prop[1] = NULL;
kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop);
kfree(pwm_prop[0]);
return 0;
}
static int pwm_unexport_match(struct device *child, void *data)
{
return child_to_pwm_device(child) == data;
}
static int pwm_unexport_child(struct device *parent, struct pwm_device *pwm)
{
struct device *child;
char *pwm_prop[2];
if (!test_and_clear_bit(PWMF_EXPORTED, &pwm->flags))
return -ENODEV;
child = device_find_child(parent, pwm, pwm_unexport_match);
if (!child)
return -ENODEV;
pwm_prop[0] = kasprintf(GFP_KERNEL, "UNEXPORT=pwm%u", pwm->hwpwm);
pwm_prop[1] = NULL;
kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop);
kfree(pwm_prop[0]);
/* for device_find_child() */
put_device(child);
device_unregister(child);
pwm_put(pwm);
return 0;
}
static ssize_t export_store(struct device *parent,
struct device_attribute *attr,
const char *buf, size_t len)
{
struct pwm_chip *chip = dev_get_drvdata(parent);
struct pwm_device *pwm;
unsigned int hwpwm;
int ret;
ret = kstrtouint(buf, 0, &hwpwm);
if (ret < 0)
return ret;
if (hwpwm >= chip->npwm)
return -ENODEV;
pwm = pwm_request_from_chip(chip, hwpwm, "sysfs");
if (IS_ERR(pwm))
return PTR_ERR(pwm);
ret = pwm_export_child(parent, pwm);
if (ret < 0)
pwm_put(pwm);
return ret ? : len;
}
static DEVICE_ATTR_WO(export);
static ssize_t unexport_store(struct device *parent,
struct device_attribute *attr,
const char *buf, size_t len)
{
struct pwm_chip *chip = dev_get_drvdata(parent);
unsigned int hwpwm;
int ret;
ret = kstrtouint(buf, 0, &hwpwm);
if (ret < 0)
return ret;
if (hwpwm >= chip->npwm)
return -ENODEV;
ret = pwm_unexport_child(parent, &chip->pwms[hwpwm]);
return ret ? : len;
}
static DEVICE_ATTR_WO(unexport);
static ssize_t npwm_show(struct device *parent, struct device_attribute *attr,
char *buf)
{
const struct pwm_chip *chip = dev_get_drvdata(parent);
return sprintf(buf, "%u\n", chip->npwm);
}
static DEVICE_ATTR_RO(npwm);
static struct attribute *pwm_chip_attrs[] = {
&dev_attr_export.attr,
&dev_attr_unexport.attr,
&dev_attr_npwm.attr,
NULL,
};
ATTRIBUTE_GROUPS(pwm_chip);
static struct class pwm_class = {
.name = "pwm",
.owner = THIS_MODULE,
.dev_groups = pwm_chip_groups,
};
static int pwmchip_sysfs_match(struct device *parent, const void *data)
{
return dev_get_drvdata(parent) == data;
}
void pwmchip_sysfs_export(struct pwm_chip *chip)
{
struct device *parent;
/*
* If device_create() fails the pwm_chip is still usable by
* the kernel its just not exported.
*/
parent = device_create(&pwm_class, chip->dev, MKDEV(0, 0), chip,
"pwmchip%d", chip->base);
if (IS_ERR(parent)) {
dev_warn(chip->dev,
"device_create failed for pwm_chip sysfs export\n");
}
}
void pwmchip_sysfs_unexport(struct pwm_chip *chip)
{
struct device *parent;
unsigned int i;
parent = class_find_device(&pwm_class, NULL, chip,
pwmchip_sysfs_match);
if (!parent)
return;
for (i = 0; i < chip->npwm; i++) {
struct pwm_device *pwm = &chip->pwms[i];
if (test_bit(PWMF_EXPORTED, &pwm->flags))
pwm_unexport_child(parent, pwm);
}
put_device(parent);
device_unregister(parent);
}
static int __init pwm_sysfs_init(void)
{
return class_register(&pwm_class);
}
subsys_initcall(pwm_sysfs_init);