From 62fffa45dca89dc87e8d75b360a995db334af9ac Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Sat, 9 Nov 2013 07:40:09 +0000 Subject: [PATCH] scst_sysfs: Trigger a lockdep complaint if sysfs work could trigger a deadlock This patch causes lockdep to complain if sysfs work could trigger a deadlock. As an example, for the patch below the following lockdep complaint is generated: ====================================================== [ INFO: possible circular locking dependency detected ] 3.12.0-rc7-dbg+ #5 Tainted: GF O ------------------------------------------------------- scst_mgmtd/7527 is trying to acquire lock: (scst_tgt_dev){+.+.+.}, at: [] scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst] but task is already holding lock: (&scst_mutex){+.+.+.}, at: [] scst_free_session+0x56/0x2c0 [scst] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&scst_mutex){+.+.+.}: [] lock_acquire+0x93/0x150 [] mutex_lock_interruptible_nested+0x77/0x500 [] scst_tgt_dev_process_thread_pid_show+0x5f/0x120 [scst] [] scst_process_sysfs_works+0xe6/0x1d0 [scst] [] sysfs_work_thread_fn+0x120/0x2b0 [scst] [] kthread+0xea/0xf0 [] ret_from_fork+0x7c/0xb0 -> #0 (scst_tgt_dev){+.+.+.}: [] __lock_acquire+0x14fa/0x1a60 [] lock_acquire+0x93/0x150 [] scst_kobject_put_and_wait+0x6f/0x140 [scst] [] scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst] [] scst_free_tgt_dev+0x84/0x210 [scst] [] scst_sess_free_tgt_devs+0x6b/0x100 [scst] [] scst_free_session+0x5e/0x2c0 [scst] [] scst_free_session_callback+0x9e/0x170 [scst] [] scst_global_mgmt_thread+0x24a/0x550 [scst] [] kthread+0xea/0xf0 [] ret_from_fork+0x7c/0xb0 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&scst_mutex); lock(scst_tgt_dev); lock(&scst_mutex); lock(scst_tgt_dev); *** DEADLOCK *** 1 lock held by scst_mgmtd/7527: #0: (&scst_mutex){+.+.+.}, at: [] scst_free_session+0x56/0x2c0 [scst] stack backtrace: CPU: 1 PID: 7527 Comm: scst_mgmtd Tainted: GF O 3.12.0-rc7-dbg+ #5 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/31/2013 ffffffff822efd40 ffff88007b7c1ac8 ffffffff8161464a ffffffff822efd40 ffff88007b7c1b08 ffffffff81610d02 ffff88007b7c1b60 ffff8800692748c0 0000000000000000 ffff880069274898 ffff880069274120 ffff8800692748c0 Call Trace: [] dump_stack+0x4e/0x82 [] print_circular_bug+0x201/0x210 [] __lock_acquire+0x14fa/0x1a60 [] lock_acquire+0x93/0x150 [] ? scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst] [] scst_kobject_put_and_wait+0x6f/0x140 [scst] [] ? scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst] [] scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst] [] scst_free_tgt_dev+0x84/0x210 [scst] [] scst_sess_free_tgt_devs+0x6b/0x100 [scst] [] scst_free_session+0x5e/0x2c0 [scst] [] scst_free_session_callback+0x9e/0x170 [scst] [] scst_global_mgmt_thread+0x24a/0x550 [scst] [] ? wake_up_atomic_t+0x30/0x30 [] ? scst_register_session_non_gpl+0x20/0x20 [scst] [] kthread+0xea/0xf0 [] ? kthread_create_on_node+0x140/0x140 [] ret_from_fork+0x7c/0xb0 [] ? kthread_create_on_node+0x140/0x140 git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@5095 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/conn.c | 12 +++- scst/include/scst.h | 15 ++++- scst/src/dev_handlers/scst_vdisk.c | 3 + scst/src/scst_mem.c | 11 +++- scst/src/scst_sysfs.c | 93 +++++++++++++++++++++++++++--- 5 files changed, 122 insertions(+), 12 deletions(-) diff --git a/iscsi-scst/kernel/conn.c b/iscsi-scst/kernel/conn.c index 0aa83dcbb..ffa22eeb0 100644 --- a/iscsi-scst/kernel/conn.c +++ b/iscsi-scst/kernel/conn.c @@ -22,6 +22,14 @@ #include "iscsi.h" #include "digest.h" +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 29) +#ifdef CONFIG_LOCKDEP +static struct lock_class_key scst_conn_key; +struct lockdep_map scst_conn_dep_map = + STATIC_LOCKDEP_MAP_INIT("iscsi_conn_kref", &scst_conn_key); +#endif +#endif + static int print_conn_state(char *p, size_t size, struct iscsi_conn *conn) { int pos = 0; @@ -245,7 +253,9 @@ static void conn_sysfs_del(struct iscsi_conn *conn) kobject_del(&conn->conn_kobj); - scst_kobject_put_and_wait(&conn->conn_kobj, "conn", &c); + scst_kobject_put_and_wait(&conn->conn_kobj, "conn", + conn->conn_kobj_release_cmpl, + &scst_conn_dep_map); TRACE_EXIT(); return; diff --git a/scst/include/scst.h b/scst/include/scst.h index b2698062a..ded2245be 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -4211,7 +4211,8 @@ struct sysfs_ops *scst_sysfs_get_sysfs_ops(void); #endif void scst_kobject_put_and_wait(struct kobject *kobj, const char *category, - struct completion *c); + struct completion *c, + struct lockdep_map *dep_map); /* * Returns target driver's root sysfs kobject. @@ -4481,6 +4482,12 @@ struct scst_sysfs_work_item { int (*sysfs_work_fn)(struct scst_sysfs_work_item *work); struct completion sysfs_work_done; char *buf; + /* + * If the caller of scst_sysfs_queue_wait_work() holds a reference on + * a kobject, must point at the lockdep_map structure associated with + * that kobject. + */ + struct lockdep_map *dep_map; union { struct scst_dev_type *devt; @@ -4519,6 +4526,12 @@ int scst_sysfs_queue_wait_work(struct scst_sysfs_work_item *work); void scst_sysfs_work_get(struct scst_sysfs_work_item *work); void scst_sysfs_work_put(struct scst_sysfs_work_item *work); +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 29) +#ifdef CONFIG_LOCKDEP +extern struct lockdep_map scst_dev_dep_map; +#endif +#endif + #endif /* CONFIG_SCST_PROC */ char *scst_get_next_lexem(char **token_str); diff --git a/scst/src/dev_handlers/scst_vdisk.c b/scst/src/dev_handlers/scst_vdisk.c index ffc0b9750..2d596f276 100644 --- a/scst/src/dev_handlers/scst_vdisk.c +++ b/scst/src/dev_handlers/scst_vdisk.c @@ -5362,6 +5362,7 @@ static ssize_t vcdrom_sysfs_filename_store(struct kobject *kobj, work->buf = i_buf; work->dev = dev; + work->dep_map = &scst_dev_dep_map; kobject_get(&dev->dev_kobj); res = scst_sysfs_queue_wait_work(work); @@ -5651,6 +5652,7 @@ static ssize_t vdev_sysfs_filename_show(struct kobject *kobj, work->dev = dev; + work->dep_map = &scst_dev_dep_map; kobject_get(&dev->dev_kobj); scst_sysfs_work_get(work); @@ -5707,6 +5709,7 @@ static ssize_t vdisk_sysfs_resync_size_store(struct kobject *kobj, work->dev = dev; + work->dep_map = &scst_dev_dep_map; kobject_get(&dev->dev_kobj); res = scst_sysfs_queue_wait_work(work); diff --git a/scst/src/scst_mem.c b/scst/src/scst_mem.c index 02bb5d3f4..fb768f2b3 100644 --- a/scst/src/scst_mem.c +++ b/scst/src/scst_mem.c @@ -41,6 +41,14 @@ /* Max pages freed from a pool per shrinking iteration */ #define MAX_PAGES_PER_POOL 50 +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 29) +#ifdef CONFIG_LOCKDEP +static struct lock_class_key scst_pool_key; +struct lockdep_map scst_pool_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_pool_kref", &scst_pool_key); +#endif +#endif + static struct sgv_pool *sgv_norm_clust_pool, *sgv_norm_pool, *sgv_dma_pool; static atomic_t sgv_pages_total = ATOMIC_INIT(0); @@ -2085,7 +2093,8 @@ static void scst_sgv_sysfs_del(struct sgv_pool *pool) kobject_del(&pool->sgv_kobj); - scst_kobject_put_and_wait(&pool->sgv_kobj, "SGV pool", &c); + scst_kobject_put_and_wait(&pool->sgv_kobj, "SGV pool", &c, + &scst_pool_dep_map); TRACE_EXIT(); } diff --git a/scst/src/scst_sysfs.c b/scst/src/scst_sysfs.c index dc838d725..64f7f192e 100644 --- a/scst/src/scst_sysfs.c +++ b/scst/src/scst_sysfs.c @@ -34,6 +34,42 @@ #include "scst_priv.h" #include "scst_pres.h" +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 29) +#ifdef CONFIG_LOCKDEP +static struct lock_class_key scst_tgtt_key; +static struct lockdep_map scst_tgtt_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_tgtt_kref", &scst_tgtt_key); +static struct lock_class_key scst_tgt_key; +static struct lockdep_map scst_tgt_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_tgt_kref", &scst_tgt_key); +static struct lock_class_key scst_devt_key; +static struct lockdep_map scst_devt_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_devt_kref", &scst_devt_key); +static struct lock_class_key scst_dev_key; +struct lockdep_map scst_dev_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_dev_kref", &scst_dev_key); +EXPORT_SYMBOL(scst_dev_dep_map); +static struct lock_class_key scst_sess_key; +static struct lockdep_map scst_sess_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_sess_kref", &scst_sess_key); +static struct lock_class_key scst_acg_dev_key; +static struct lockdep_map scst_acg_dev_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_acg_dev_kref", &scst_acg_dev_key); +static struct lock_class_key scst_acg_key; +static struct lockdep_map scst_acg_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_acg_kref", &scst_acg_key); +static struct lock_class_key scst_tgt_dev_key; +static struct lockdep_map scst_tgt_dev_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_tgt_dev_kref", &scst_tgt_dev_key); +static struct lock_class_key scst_dg_key; +static struct lockdep_map scst_dg_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_dg_kref", &scst_dg_key); +static struct lock_class_key scst_tg_key; +static struct lockdep_map scst_tg_dep_map = + STATIC_LOCKDEP_MAP_INIT("scst_tg_kref", &scst_tg_key); +#endif +#endif + static DECLARE_COMPLETION(scst_sysfs_root_release_completion); static struct kobject *scst_targets_kobj; @@ -412,8 +448,20 @@ static void scst_process_sysfs_works(void) TRACE_DBG("Sysfs work %p", work); +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 29) + if (work->dep_map) { + mutex_acquire(work->dep_map, 0, 0, _RET_IP_); + lock_acquired(work->dep_map, _RET_IP_); + } +#endif + work->work_res = work->sysfs_work_fn(work); +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 29) + if (work->dep_map) + mutex_release(work->dep_map, 0, _RET_IP_); +#endif + spin_lock(&sysfs_work_lock); if (!work->read_only_action) last_sysfs_work_res = work->work_res; @@ -1058,7 +1106,8 @@ out_del: } void scst_kobject_put_and_wait(struct kobject *kobj, const char *category, - struct completion *c) + struct completion *c, + struct lockdep_map *dep_map) { char *name; @@ -1068,6 +1117,10 @@ void scst_kobject_put_and_wait(struct kobject *kobj, const char *category, kobject_put(kobj); +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 29) + mutex_acquire(dep_map, 0, 0, _RET_IP_); +#endif + if (wait_for_completion_timeout(c, HZ) > 0) goto out_free; @@ -1078,6 +1131,11 @@ void scst_kobject_put_and_wait(struct kobject *kobj, const char *category, category, name ? : "(?)"); out_free: +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 29) + lock_acquired(dep_map, _RET_IP_); + mutex_release(dep_map, 0, _RET_IP_); +#endif + kfree(name); TRACE_EXIT(); @@ -1100,7 +1158,8 @@ void scst_tgtt_sysfs_del(struct scst_tgt_template *tgtt) kobject_del(&tgtt->tgtt_kobj); - scst_kobject_put_and_wait(&tgtt->tgtt_kobj, "target template", &c); + scst_kobject_put_and_wait(&tgtt->tgtt_kobj, "target template", &c, + &scst_tgtt_dep_map); TRACE_EXIT(); return; @@ -2119,6 +2178,7 @@ static ssize_t scst_tgt_enable_store(struct kobject *kobj, work->tgt = tgt; work->enable = enable; + work->dep_map = &scst_tgt_dep_map; kobject_get(&tgt->tgt_kobj); res = scst_sysfs_queue_wait_work(work); @@ -2234,6 +2294,7 @@ static ssize_t scst_rel_tgt_id_store(struct kobject *kobj, work->tgt_r = tgt; work->rel_tgt_id = rel_tgt_id; + work->dep_map = &scst_tgt_dep_map; kobject_get(&tgt->tgt_kobj); res = scst_sysfs_queue_wait_work(work); @@ -2497,7 +2558,8 @@ void scst_tgt_sysfs_put(struct scst_tgt *tgt) tgt->tgt_kobj_release_cmpl = &c; - scst_kobject_put_and_wait(&tgt->tgt_kobj, "target", &c); + scst_kobject_put_and_wait(&tgt->tgt_kobj, "target", &c, + &scst_tgt_dep_map); TRACE_EXIT(); return; @@ -3008,7 +3070,8 @@ void scst_dev_sysfs_del(struct scst_device *dev) kobject_put(dev->dev_exp_kobj); - scst_kobject_put_and_wait(&dev->dev_kobj, "device", &c); + scst_kobject_put_and_wait(&dev->dev_kobj, "device", &c, + &scst_dev_dep_map); TRACE_EXIT(); return; @@ -3211,7 +3274,8 @@ void scst_tgt_dev_sysfs_del(struct scst_tgt_dev *tgt_dev) kobject_del(&tgt_dev->tgt_dev_kobj); - scst_kobject_put_and_wait(&tgt_dev->tgt_dev_kobj, "tgt_dev", &c); + scst_kobject_put_and_wait(&tgt_dev->tgt_dev_kobj, "tgt_dev", &c, + &scst_tgt_dev_dep_map); TRACE_EXIT(); return; @@ -3437,6 +3501,7 @@ static ssize_t scst_sess_latency_store(struct kobject *kobj, work->sess = sess; + work->dep_map = &sess->dep_map; kobject_get(&sess->sess_kobj); res = scst_sysfs_queue_wait_work(work); @@ -3518,6 +3583,7 @@ static ssize_t scst_sess_sysfs_active_commands_show(struct kobject *kobj, work->sess = sess; + work->dep_map = &scst_sess_dep_map; kobject_get(&sess->sess_kobj); res = scst_sysfs_queue_wait_work(work); @@ -3764,7 +3830,8 @@ void scst_sess_sysfs_del(struct scst_session *sess) kobject_del(&sess->sess_kobj); - scst_kobject_put_and_wait(&sess->sess_kobj, "session", &c); + scst_kobject_put_and_wait(&sess->sess_kobj, "session", &c, + &scst_sess_dep_map); out: TRACE_EXIT(); @@ -3840,7 +3907,8 @@ void scst_acg_dev_sysfs_del(struct scst_acg_dev *acg_dev) kobject_del(&acg_dev->acg_dev_kobj); - scst_kobject_put_and_wait(&acg_dev->acg_dev_kobj, "acg_dev", &c); + scst_kobject_put_and_wait(&acg_dev->acg_dev_kobj, "acg_dev", &c, + &scst_acg_dev_dep_map); TRACE_EXIT(); return; @@ -4224,7 +4292,8 @@ void scst_acg_sysfs_del(struct scst_acg *acg) kobject_put(acg->luns_kobj); kobject_put(acg->initiators_kobj); - scst_kobject_put_and_wait(&acg->acg_kobj, "acg", &c); + scst_kobject_put_and_wait(&acg->acg_kobj, "acg", &c, + &scst_acg_dep_map); TRACE_EXIT(); return; @@ -4864,7 +4933,8 @@ void scst_devt_sysfs_del(struct scst_dev_type *devt) kobject_del(&devt->devt_kobj); - scst_kobject_put_and_wait(&devt->devt_kobj, "dev handler template", &c); + scst_kobject_put_and_wait(&devt->devt_kobj, "dev handler template", &c, + &scst_devt_dep_map); TRACE_EXIT(); return; @@ -4965,6 +5035,7 @@ static ssize_t scst_dg_devs_mgmt_store(struct kobject *kobj, swap(work->buf, cmd); work->kobj = kobj; + work->dep_map = &scst_dg_dep_map; kobject_get(kobj); res = scst_sysfs_queue_wait_work(work); if (res) @@ -5187,6 +5258,7 @@ static ssize_t scst_tg_preferred_store(struct kobject *kobj, swap(work->buf, cmd); work->kobj = kobj; + work->dep_map = &scst_tg_dep_map; kobject_get(kobj); res = scst_sysfs_queue_wait_work(work); if (res) @@ -5267,6 +5339,7 @@ static ssize_t scst_tg_state_store(struct kobject *kobj, swap(work->buf, cmd); work->kobj = kobj; + work->dep_map = &scst_tg_dep_map; kobject_get(kobj); res = scst_sysfs_queue_wait_work(work); if (res) @@ -5351,6 +5424,7 @@ static ssize_t scst_tg_mgmt_store(struct kobject *kobj, swap(work->buf, cmd); work->kobj = kobj; + work->dep_map = &scst_tg_dep_map; kobject_get(kobj); res = scst_sysfs_queue_wait_work(work); if (res) @@ -5474,6 +5548,7 @@ static ssize_t scst_dg_tgs_mgmt_store(struct kobject *kobj, swap(work->buf, cmd); work->kobj = kobj; + work->dep_map = &scst_dg_dep_map; kobject_get(kobj); res = scst_sysfs_queue_wait_work(work); if (res)