From: Waiman Long <longman@redhat.com>
To: Jens Axboe <axboe@kernel.dk>,
Steven Rostedt <rostedt@goodmis.org>,
Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
Bart Van Assche <Bart.VanAssche@wdc.com>,
Waiman Long <longman@redhat.com>
Subject: [PATCH v3] blktrace: Fix potentail deadlock between delete & sysfs ops
Date: Fri, 18 Aug 2017 13:54:08 -0400 [thread overview]
Message-ID: <1503078848-15779-1-git-send-email-longman@redhat.com> (raw)
The lockdep code had reported the following unsafe locking scenario:
CPU0 CPU1
---- ----
lock(s_active#228);
lock(&bdev->bd_mutex/1);
lock(s_active#228);
lock(&bdev->bd_mutex);
*** DEADLOCK ***
The deadlock may happen when one task (CPU1) is trying to delete a
partition in a block device and another task (CPU0) is accessing
tracing sysfs file (e.g. /sys/block/dm-1/trace/act_mask) in that
partition.
The s_active isn't an actual lock. It is a reference count (kn->count)
on the sysfs (kernfs) file. Removal of a sysfs file, however, require
a wait until all the references are gone. The reference count is
treated like a rwsem using lockdep instrumentation code.
The fact that a thread is in the sysfs callback method will guarantee
that the underlying block device structure won't go away as device
deletion requires all the sysfs references to be gone. Therefore,
there is no need to take the bd_mutex. Instead, a global blktrace
mutex will be used to serialize the read/write of the blktrace sysfs
attributes.
Signed-off-by: Waiman Long <longman@redhat.com>
---
v3:
- Use a global blktrace_mutex to serialize sysfs attribute accesses
instead of the bd_mutex.
v2:
- Use READ_ONCE() and smp_store_mb() to read and write bd_deleting.
- Check for signal in the mutex_trylock loops.
- Use usleep() instead of schedule() for RT tasks.
kernel/trace/blktrace.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index bc364f8..e5901c6 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -1605,6 +1605,15 @@ static struct request_queue *blk_trace_get_queue(struct block_device *bdev)
return bdev_get_queue(bdev);
}
+/*
+ * Read/write to the blk_trace sysfs files requires taking references to
+ * the underlying kernfs_node structure which will guarantee that the block
+ * device won't go away as the device deletion code will wait until all the
+ * sysfs references are gone. For serialization of read/write accesses to
+ * the sysfs attributes, a global blk_trace mutex is used.
+ */
+static DEFINE_MUTEX(blktrace_mutex);
+
static ssize_t sysfs_blk_trace_attr_show(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -1622,7 +1631,7 @@ static ssize_t sysfs_blk_trace_attr_show(struct device *dev,
if (q == NULL)
goto out_bdput;
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&blktrace_mutex);
if (attr == &dev_attr_enable) {
ret = sprintf(buf, "%u\n", !!q->blk_trace);
@@ -1641,7 +1650,7 @@ static ssize_t sysfs_blk_trace_attr_show(struct device *dev,
ret = sprintf(buf, "%llu\n", q->blk_trace->end_lba);
out_unlock_bdev:
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&blktrace_mutex);
out_bdput:
bdput(bdev);
out:
@@ -1683,7 +1692,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
if (q == NULL)
goto out_bdput;
- mutex_lock(&bdev->bd_mutex);
+ mutex_lock(&blktrace_mutex);
if (attr == &dev_attr_enable) {
if (value)
@@ -1709,7 +1718,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
}
out_unlock_bdev:
- mutex_unlock(&bdev->bd_mutex);
+ mutex_unlock(&blktrace_mutex);
out_bdput:
bdput(bdev);
out:
--
1.8.3.1
next reply other threads:[~2017-08-18 17:54 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-18 17:54 Waiman Long [this message]
2017-08-18 18:07 ` [PATCH v3] blktrace: Fix potentail deadlock between delete & sysfs ops Bart Van Assche
2017-08-18 19:55 ` Waiman Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1503078848-15779-1-git-send-email-longman@redhat.com \
--to=longman@redhat.com \
--cc=Bart.VanAssche@wdc.com \
--cc=axboe@kernel.dk \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox