[PATCH 3.2 115/152] Fix circular locking dependency (3.3-rc2)

From: Ben Hutchings
Date: Mon Feb 16 2015 - 21:32:16 EST


3.2.67-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Ming Lei <tom.leiming@xxxxxxxxx>

commit 864533ceb6db336dead389577c102a8b792a121a upstream.

Hi,

On Wed, Feb 8, 2012 at 8:41 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> Hi guys,
>
> I have just triggered the folllowing:
>
> [ Â 84.860321] ======================================================
> [ Â 84.860321] [ INFO: possible circular locking dependency detected ]
> [ Â 84.860321] 3.3.0-rc2-00026-ge4e8a39 #474 Not tainted
> [ Â 84.860321] -------------------------------------------------------
> [ Â 84.860321] bash/949 is trying to acquire lock:
> [ Â 84.860321] Â(sysfs_lock){+.+.+.}, at: [<c0275358>] gpio_value_store+0x24/0xcc
> [ Â 84.860321]
> [ Â 84.860321] but task is already holding lock:
> [ Â 84.860321] Â(s_active#22){++++.+}, at: [<c016996c>] sysfs_write_file+0xdc/0x184
> [ Â 84.911468]
> [ Â 84.911468] which lock already depends on the new lock.
> [ Â 84.911468]
> [ Â 84.920043]
> [ Â 84.920043] the existing dependency chain (in reverse order) is:
> [ Â 84.920043]
> [ Â 84.927886] -> #1 (s_active#22){++++.+}:
> [ Â 84.927886] Â Â Â Â[<c008f640>] check_prevs_add+0xdc/0x150
> [ Â 84.927886] Â Â Â Â[<c008fc18>] validate_chain.clone.24+0x564/0x694
> [ Â 84.927886] Â Â Â Â[<c0090cdc>] __lock_acquire+0x49c/0x980
> [ Â 84.951660] Â Â Â Â[<c0091838>] lock_acquire+0x98/0x100
> [ Â 84.951660] Â Â Â Â[<c016a8e8>] sysfs_deactivate+0xb0/0x100
> [ Â 84.962982] Â Â Â Â[<c016b1b4>] sysfs_addrm_finish+0x2c/0x6c
> [ Â 84.962982] Â Â Â Â[<c016b8bc>] sysfs_remove_dir+0x84/0x98
> [ Â 84.962982] Â Â Â Â[<c02590d8>] kobject_del+0x10/0x78
> [ Â 84.974670] Â Â Â Â[<c02c29e8>] device_del+0x140/0x170
> [ Â 84.974670] Â Â Â Â[<c02c2a24>] device_unregister+0xc/0x18
> [ Â 84.985382] Â Â Â Â[<c0276894>] gpio_unexport+0xbc/0xdc
> [ Â 84.985382] Â Â Â Â[<c02768c8>] gpio_free+0x14/0xfc
> [ Â 85.001708] Â Â Â Â[<c0276a28>] unexport_store+0x78/0x8c
> [ Â 85.001708] Â Â Â Â[<c02c5af8>] class_attr_store+0x18/0x24
> [ Â 85.007293] Â Â Â Â[<c0169990>] sysfs_write_file+0x100/0x184
> [ Â 85.018981] Â Â Â Â[<c0109d48>] vfs_write+0xb4/0x148
> [ Â 85.018981] Â Â Â Â[<c0109fd0>] sys_write+0x40/0x70
> [ Â 85.018981] Â Â Â Â[<c0013cc0>] ret_fast_syscall+0x0/0x3c
> [ Â 85.035003]
> [ Â 85.035003] -> #0 (sysfs_lock){+.+.+.}:
> [ Â 85.035003] Â Â Â Â[<c008f54c>] check_prev_add+0x680/0x698
> [ Â 85.035003] Â Â Â Â[<c008f640>] check_prevs_add+0xdc/0x150
> [ Â 85.052093] Â Â Â Â[<c008fc18>] validate_chain.clone.24+0x564/0x694
> [ Â 85.052093] Â Â Â Â[<c0090cdc>] __lock_acquire+0x49c/0x980
> [ Â 85.052093] Â Â Â Â[<c0091838>] lock_acquire+0x98/0x100
> [ Â 85.069885] Â Â Â Â[<c047e280>] mutex_lock_nested+0x3c/0x2f4
> [ Â 85.069885] Â Â Â Â[<c0275358>] gpio_value_store+0x24/0xcc
> [ Â 85.069885] Â Â Â Â[<c02c18dc>] dev_attr_store+0x18/0x24
> [ Â 85.087158] Â Â Â Â[<c0169990>] sysfs_write_file+0x100/0x184
> [ Â 85.087158] Â Â Â Â[<c0109d48>] vfs_write+0xb4/0x148
> [ Â 85.098297] Â Â Â Â[<c0109fd0>] sys_write+0x40/0x70
> [ Â 85.098297] Â Â Â Â[<c0013cc0>] ret_fast_syscall+0x0/0x3c
> [ Â 85.109069]
> [ Â 85.109069] other info that might help us debug this:
> [ Â 85.109069]
> [ Â 85.117462] ÂPossible unsafe locking scenario:
> [ Â 85.117462]
> [ Â 85.117462] Â Â Â ÂCPU0 Â Â Â Â Â Â Â Â Â ÂCPU1
> [ Â 85.128417] Â Â Â Â---- Â Â Â Â Â Â Â Â Â Â----
> [ Â 85.128417] Â lock(s_active#22);
> [ Â 85.128417] Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âlock(sysfs_lock);
> [ Â 85.128417] Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âlock(s_active#22);
> [ Â 85.142486] Â lock(sysfs_lock);
> [ Â 85.151794]
> [ Â 85.151794] Â*** DEADLOCK ***
> [ Â 85.151794]
> [ Â 85.151794] 2 locks held by bash/949:
> [ Â 85.158020] Â#0: Â(&buffer->mutex){+.+.+.}, at: [<c01698b8>] sysfs_write_file+0x28/0x184
> [ Â 85.170349] Â#1: Â(s_active#22){++++.+}, at: [<c016996c>] sysfs_write_file+0xdc/0x184
> [ Â 85.170349]
> [ Â 85.178588] stack backtrace:
> [ Â 85.178588] [<c001b824>] (unwind_backtrace+0x0/0xf0) from [<c008de64>] (print_circular_bug+0x100/0x114)
> [ Â 85.193023] [<c008de64>] (print_circular_bug+0x100/0x114) from [<c008f54c>] (check_prev_add+0x680/0x698)
> [ Â 85.193023] [<c008f54c>] (check_prev_add+0x680/0x698) from [<c008f640>] (check_prevs_add+0xdc/0x150)
> [ Â 85.212524] [<c008f640>] (check_prevs_add+0xdc/0x150) from [<c008fc18>] (validate_chain.clone.24+0x564/0x694)
> [ Â 85.212524] [<c008fc18>] (validate_chain.clone.24+0x564/0x694) from [<c0090cdc>] (__lock_acquire+0x49c/0x980)
> [ Â 85.233306] [<c0090cdc>] (__lock_acquire+0x49c/0x980) from [<c0091838>] (lock_acquire+0x98/0x100)
> [ Â 85.233306] [<c0091838>] (lock_acquire+0x98/0x100) from [<c047e280>] (mutex_lock_nested+0x3c/0x2f4)
> [ Â 85.242614] [<c047e280>] (mutex_lock_nested+0x3c/0x2f4) from [<c0275358>] (gpio_value_store+0x24/0xcc)
> [ Â 85.261840] [<c0275358>] (gpio_value_store+0x24/0xcc) from [<c02c18dc>] (dev_attr_store+0x18/0x24)
> [ Â 85.261840] [<c02c18dc>] (dev_attr_store+0x18/0x24) from [<c0169990>] (sysfs_write_file+0x100/0x184)
> [ Â 85.271240] [<c0169990>] (sysfs_write_file+0x100/0x184) from [<c0109d48>] (vfs_write+0xb4/0x148)
> [ Â 85.290008] [<c0109d48>] (vfs_write+0xb4/0x148) from [<c0109fd0>] (sys_write+0x40/0x70)
> [ Â 85.298400] [<c0109fd0>] (sys_write+0x40/0x70) from [<c0013cc0>] (ret_fast_syscall+0x0/0x3c)
> -bash: echo: write error: Operation not permitted
>
> the way to trigger is:
>
> root@legolas:~# cd /sys/class/gpio/
> root@legolas:/sys/class/gpio# echo 2 > export
> root@legolas:/sys/class/gpio# echo 2 > unexport
> root@legolas:/sys/class/gpio# echo 2 > export
> root@legolas:/sys/class/gpio# cd gpio2/
> root@legolas:/sys/class/gpio/gpio2# echo 1 > value

Looks 'sysfs_lock' needn't to be held for unregister, so the patch below may
fix the problem.

Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Signed-off-by: Grant Likely <grant.likely@xxxxxxxxxxxx>
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
drivers/gpio/gpiolib.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -877,6 +877,7 @@ void gpio_unexport(unsigned gpio)
{
struct gpio_desc *desc;
int status = 0;
+ struct device *dev = NULL;

if (!gpio_is_valid(gpio)) {
status = -EINVAL;
@@ -888,19 +889,20 @@ void gpio_unexport(unsigned gpio)
desc = &gpio_desc[gpio];

if (test_bit(FLAG_EXPORT, &desc->flags)) {
- struct device *dev = NULL;

dev = class_find_device(&gpio_class, NULL, desc, match_export);
if (dev) {
gpio_setup_irq(desc, dev, 0);
clear_bit(FLAG_EXPORT, &desc->flags);
- put_device(dev);
- device_unregister(dev);
} else
status = -ENODEV;
}

mutex_unlock(&sysfs_lock);
+ if (dev) {
+ device_unregister(dev);
+ put_device(dev);
+ }
done:
if (status)
pr_debug("%s: gpio%d status %d\n", __func__, gpio, status);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/