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: [<ffffffffa01f9365>] scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst]

but task is already holding lock:
 (&scst_mutex){+.+.+.}, at: [<ffffffffa01eaa46>] 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){+.+.+.}:
       [<ffffffff810be983>] lock_acquire+0x93/0x150
       [<ffffffff81619617>] mutex_lock_interruptible_nested+0x77/0x500
       [<ffffffffa01f1f9f>] scst_tgt_dev_process_thread_pid_show+0x5f/0x120 [scst]
       [<ffffffffa01f38d6>] scst_process_sysfs_works+0xe6/0x1d0 [scst]
       [<ffffffffa01f7af0>] sysfs_work_thread_fn+0x120/0x2b0 [scst]
       [<ffffffff8107482a>] kthread+0xea/0xf0
       [<ffffffff81625f6c>] ret_from_fork+0x7c/0xb0

-> #0 (scst_tgt_dev){+.+.+.}:
       [<ffffffff810bdc8a>] __lock_acquire+0x14fa/0x1a60
       [<ffffffff810be983>] lock_acquire+0x93/0x150
       [<ffffffffa01f20df>] scst_kobject_put_and_wait+0x6f/0x140 [scst]
       [<ffffffffa01f9365>] scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst]
       [<ffffffffa01e9114>] scst_free_tgt_dev+0x84/0x210 [scst]
       [<ffffffffa01ea80b>] scst_sess_free_tgt_devs+0x6b/0x100 [scst]
       [<ffffffffa01eaa4e>] scst_free_session+0x5e/0x2c0 [scst]
       [<ffffffffa01ead4e>] scst_free_session_callback+0x9e/0x170 [scst]
       [<ffffffffa01d5a9a>] scst_global_mgmt_thread+0x24a/0x550 [scst]
       [<ffffffff8107482a>] kthread+0xea/0xf0
       [<ffffffff81625f6c>] 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: [<ffffffffa01eaa46>] 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:
 [<ffffffff8161464a>] dump_stack+0x4e/0x82
 [<ffffffff81610d02>] print_circular_bug+0x201/0x210
 [<ffffffff810bdc8a>] __lock_acquire+0x14fa/0x1a60
 [<ffffffff810be983>] lock_acquire+0x93/0x150
 [<ffffffffa01f9365>] ? scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst]
 [<ffffffffa01f20df>] scst_kobject_put_and_wait+0x6f/0x140 [scst]
 [<ffffffffa01f9365>] ? scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst]
 [<ffffffffa01f9365>] scst_tgt_dev_sysfs_del+0x105/0x1a0 [scst]
 [<ffffffffa01e9114>] scst_free_tgt_dev+0x84/0x210 [scst]
 [<ffffffffa01ea80b>] scst_sess_free_tgt_devs+0x6b/0x100 [scst]
 [<ffffffffa01eaa4e>] scst_free_session+0x5e/0x2c0 [scst]
 [<ffffffffa01ead4e>] scst_free_session_callback+0x9e/0x170 [scst]
 [<ffffffffa01d5a9a>] scst_global_mgmt_thread+0x24a/0x550 [scst]
 [<ffffffff81075850>] ? wake_up_atomic_t+0x30/0x30
 [<ffffffffa01d5850>] ? scst_register_session_non_gpl+0x20/0x20 [scst]
 [<ffffffff8107482a>] kthread+0xea/0xf0
 [<ffffffff81074740>] ? kthread_create_on_node+0x140/0x140
 [<ffffffff81625f6c>] ret_from_fork+0x7c/0xb0
 [<ffffffff81074740>] ? kthread_create_on_node+0x140/0x140



git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@5095 d57e44dd-8a1f-0410-8b47-8ef2f437770f
This commit is contained in:
Vladislav Bolkhovitin
2013-11-09 07:40:09 +00:00
parent 388cb01060
commit 62fffa45dc
5 changed files with 122 additions and 12 deletions

View File

@@ -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;

View File

@@ -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);

View File

@@ -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);

View File

@@ -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();
}

View File

@@ -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)