block: create the request_queue debugfs_dir on registration

We were only creating the request_queue debugfs_dir only
for make_request block drivers (multiqueue), but never for
request-based block drivers. We did this as we were only
creating non-blktrace additional debugfs files on that directory
for make_request drivers. However, since blktrace *always* creates
that directory anyway, we special-case the use of that directory
on blktrace. Other than this being an eye-sore, this exposes
request-based block drivers to the same debugfs fragile
race that used to exist with make_request block drivers
where if we start adding files onto that directory we can later
run a race with a double removal of dentries on the directory
if we don't deal with this carefully on blktrace.

Instead, just simplify things by always creating the request_queue
debugfs_dir on request_queue registration. Rename the mutex also to
reflect the fact that this is used outside of the blktrace context.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/block/blk-core.c b/block/blk-core.c
index a99b22fac38a2e32564802e248328dcc62f5ba1d..a9769c1a287546b2e04419d543a2d0f7fca49abe 100644 (file)
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -51,9 +51,7 @@
 #include "blk-pm.h"
 #include "blk-rq-qos.h"
 
-#ifdef CONFIG_DEBUG_FS
 struct dentry *blk_debugfs_root;
-#endif
 
 EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_remap);
 EXPORT_TRACEPOINT_SYMBOL_GPL(block_rq_remap);
@@ -555,9 +553,7 @@ struct request_queue *__blk_alloc_queue(int node_id)
 
        kobject_init(&q->kobj, &blk_queue_ktype);
 
-#ifdef CONFIG_BLK_DEV_IO_TRACE
-       mutex_init(&q->blk_trace_mutex);
-#endif
+       mutex_init(&q->debugfs_mutex);
        mutex_init(&q->sysfs_lock);
        mutex_init(&q->sysfs_dir_lock);
        spin_lock_init(&q->queue_lock);
@@ -1931,9 +1927,7 @@ int __init blk_dev_init(void)
        blk_requestq_cachep = kmem_cache_create("request_queue",
                        sizeof(struct request_queue), 0, SLAB_PANIC, NULL);
 
-#ifdef CONFIG_DEBUG_FS
        blk_debugfs_root = debugfs_create_dir("block", NULL);
-#endif
 
        return 0;
 }

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 15df3a36e9fa43f9d1daa8b03938fd4d0e7cede0..a2800bc56fb4d3c5c0e7ed8862961fd112b2e6c4 100644 (file)
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -824,9 +824,6 @@ void blk_mq_debugfs_register(struct request_queue *q)
        struct blk_mq_hw_ctx *hctx;
        int i;
 
-       q->debugfs_dir = debugfs_create_dir(kobject_name(q->kobj.parent),
-                                           blk_debugfs_root);
-
        debugfs_create_files(q->debugfs_dir, q, blk_mq_debugfs_queue_attrs);
 
        /*
@@ -857,9 +854,7 @@ void blk_mq_debugfs_register(struct request_queue *q)
 
 void blk_mq_debugfs_unregister(struct request_queue *q)
 {
-       debugfs_remove_recursive(q->debugfs_dir);
        q->sched_debugfs_dir = NULL;
-       q->debugfs_dir = NULL;
 }
 
 static void blk_mq_debugfs_register_ctx(struct blk_mq_hw_ctx *hctx,

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 561624d4cc4e7f4a1771a6a23283d0e34d907c24..be67952e7be229ed333c5089f556ff444a72f739 100644 (file)
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -11,6 +11,7 @@
 #include <linux/blktrace_api.h>
 #include <linux/blk-mq.h>
 #include <linux/blk-cgroup.h>
+#include <linux/debugfs.h>
 
 #include "blk.h"
 #include "blk-mq.h"
@@ -917,6 +918,9 @@ static void blk_release_queue(struct kobject *kobj)
                blk_mq_release(q);
 
        blk_trace_shutdown(q);
+       mutex_lock(&q->debugfs_mutex);
+       debugfs_remove_recursive(q->debugfs_dir);
+       mutex_unlock(&q->debugfs_mutex);
 
        if (queue_is_mq(q))
                blk_mq_debugfs_unregister(q);
@@ -989,6 +993,11 @@ int blk_register_queue(struct gendisk *disk)
                goto unlock;
        }
 
+       mutex_lock(&q->debugfs_mutex);
+       q->debugfs_dir = debugfs_create_dir(kobject_name(q->kobj.parent),
+                                           blk_debugfs_root);
+       mutex_unlock(&q->debugfs_mutex);
+
        if (queue_is_mq(q)) {
                __blk_mq_register_dev(dev, q);
                blk_mq_debugfs_register(q);

diff --git a/block/blk.h b/block/blk.h
index 8ba4a5e4fe07ac006afe2c43c893214e6b28978a..3a120a070dac82ced2109864a55991b3e215ea20 100644 (file)
--- a/block/blk.h
+++ b/block/blk.h
@@ -14,9 +14,7 @@
 /* Max future timer expiry for timeouts */
 #define BLK_MAX_TIMEOUT                (5 * HZ)
 
-#ifdef CONFIG_DEBUG_FS
 extern struct dentry *blk_debugfs_root;
-#endif
 
 struct blk_flush_queue {
        unsigned int            flush_pending_idx:1;

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e214e0e9f8681cbbbf665ae91ec7864cdbbbe461..c0701237116d0c096c13ef27f690525c20572dc8 100644 (file)
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -528,9 +528,9 @@ struct request_queue {
        unsigned int            sg_timeout;
        unsigned int            sg_reserved_size;
        int                     node;
+       struct mutex            debugfs_mutex;
 #ifdef CONFIG_BLK_DEV_IO_TRACE
        struct blk_trace __rcu  *blk_trace;
-       struct mutex            blk_trace_mutex;
 #endif
        /*
         * for flush operations
@@ -574,8 +574,9 @@ struct request_queue {
        struct list_head        tag_set_list;
        struct bio_set          bio_split;
 
-#ifdef CONFIG_BLK_DEBUG_FS
        struct dentry           *debugfs_dir;
+
+#ifdef CONFIG_BLK_DEBUG_FS
        struct dentry           *sched_debugfs_dir;
        struct dentry           *rqos_debugfs_dir;
 #endif

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 46c273f4dec5d3b477d3eaa2a6ecafacc689c9a9..c086c38f495427bc1ff48c8f3d9600193a9260b9 100644 (file)
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -348,7 +348,7 @@ static int __blk_trace_remove(struct request_queue *q)
        struct blk_trace *bt;
 
        bt = rcu_replace_pointer(q->blk_trace, NULL,
-                                lockdep_is_held(&q->blk_trace_mutex));
+                                lockdep_is_held(&q->debugfs_mutex));
        if (!bt)
                return -EINVAL;
 
@@ -362,9 +362,9 @@ int blk_trace_remove(struct request_queue *q)
 {
        int ret;
 
-       mutex_lock(&q->blk_trace_mutex);
+       mutex_lock(&q->debugfs_mutex);
        ret = __blk_trace_remove(q);
-       mutex_unlock(&q->blk_trace_mutex);
+       mutex_unlock(&q->debugfs_mutex);
 
        return ret;
 }
@@ -483,14 +483,11 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
        struct dentry *dir = NULL;
        int ret;
 
-       lockdep_assert_held(&q->blk_trace_mutex);
+       lockdep_assert_held(&q->debugfs_mutex);
 
        if (!buts->buf_size || !buts->buf_nr)
                return -EINVAL;
 
-       if (!blk_debugfs_root)
-               return -ENOENT;
-
        strncpy(buts->name, name, BLKTRACE_BDEV_SIZE);
        buts->name[BLKTRACE_BDEV_SIZE - 1] = '\0';
 
@@ -505,7 +502,7 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
         * we can be.
         */
        if (rcu_dereference_protected(q->blk_trace,
-                                     lockdep_is_held(&q->blk_trace_mutex))) {
+                                     lockdep_is_held(&q->debugfs_mutex))) {
                pr_warn("Concurrent blktraces are not allowed on %s\n",
                        buts->name);
                return -EBUSY;
@@ -524,18 +521,15 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
        if (!bt->msg_data)
                goto err;
 
-#ifdef CONFIG_BLK_DEBUG_FS
        /*
-        * When tracing whole make_request drivers (multiqueue) block devices,
-        * reuse the existing debugfs directory created by the block layer on
-        * init. For request-based block devices, all partitions block devices,
+        * When tracing the whole disk reuse the existing debugfs directory
+        * created by the block layer on init. For partitions block devices,
         * and scsi-generic block devices we create a temporary new debugfs
         * directory that will be removed once the trace ends.
         */
-       if (queue_is_mq(q) && bdev && bdev == bdev->bd_contains)
+       if (bdev && bdev == bdev->bd_contains)
                dir = q->debugfs_dir;
        else
-#endif
                bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root);
 
        /*
@@ -617,9 +611,9 @@ int blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
 {
        int ret;
 
-       mutex_lock(&q->blk_trace_mutex);
+       mutex_lock(&q->debugfs_mutex);
        ret = __blk_trace_setup(q, name, dev, bdev, arg);
-       mutex_unlock(&q->blk_trace_mutex);
+       mutex_unlock(&q->debugfs_mutex);
 
        return ret;
 }
@@ -665,7 +659,7 @@ static int __blk_trace_startstop(struct request_queue *q, int start)
        struct blk_trace *bt;
 
        bt = rcu_dereference_protected(q->blk_trace,
-                                      lockdep_is_held(&q->blk_trace_mutex));
+                                      lockdep_is_held(&q->debugfs_mutex));
        if (bt == NULL)
                return -EINVAL;
 
@@ -705,9 +699,9 @@ int blk_trace_startstop(struct request_queue *q, int start)
 {
        int ret;
 
-       mutex_lock(&q->blk_trace_mutex);
+       mutex_lock(&q->debugfs_mutex);
        ret = __blk_trace_startstop(q, start);
-       mutex_unlock(&q->blk_trace_mutex);
+       mutex_unlock(&q->debugfs_mutex);
 
        return ret;
 }
@@ -736,7 +730,7 @@ int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
        if (!q)
                return -ENXIO;
 
-       mutex_lock(&q->blk_trace_mutex);
+       mutex_lock(&q->debugfs_mutex);
 
        switch (cmd) {
        case BLKTRACESETUP:
@@ -763,7 +757,7 @@ int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
                break;
        }
 
-       mutex_unlock(&q->blk_trace_mutex);
+       mutex_unlock(&q->debugfs_mutex);
        return ret;
 }
 
@@ -774,14 +768,14 @@ int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
  **/
 void blk_trace_shutdown(struct request_queue *q)
 {
-       mutex_lock(&q->blk_trace_mutex);
+       mutex_lock(&q->debugfs_mutex);
        if (rcu_dereference_protected(q->blk_trace,
-                                     lockdep_is_held(&q->blk_trace_mutex))) {
+                                     lockdep_is_held(&q->debugfs_mutex))) {
                __blk_trace_startstop(q, 0);
                __blk_trace_remove(q);
        }
 
-       mutex_unlock(&q->blk_trace_mutex);
+       mutex_unlock(&q->debugfs_mutex);
 }
 
 #ifdef CONFIG_BLK_CGROUP
@@ -1662,7 +1656,7 @@ static int blk_trace_remove_queue(struct request_queue *q)
        struct blk_trace *bt;
 
        bt = rcu_replace_pointer(q->blk_trace, NULL,
-                                lockdep_is_held(&q->blk_trace_mutex));
+                                lockdep_is_held(&q->debugfs_mutex));
        if (bt == NULL)
                return -EINVAL;
 
@@ -1837,10 +1831,10 @@ static ssize_t sysfs_blk_trace_attr_show(struct device *dev,
        if (q == NULL)
                goto out_bdput;
 
-       mutex_lock(&q->blk_trace_mutex);
+       mutex_lock(&q->debugfs_mutex);
 
        bt = rcu_dereference_protected(q->blk_trace,
-                                      lockdep_is_held(&q->blk_trace_mutex));
+                                      lockdep_is_held(&q->debugfs_mutex));
        if (attr == &dev_attr_enable) {
                ret = sprintf(buf, "%u\n", !!bt);
                goto out_unlock_bdev;
@@ -1858,7 +1852,7 @@ static ssize_t sysfs_blk_trace_attr_show(struct device *dev,
                ret = sprintf(buf, "%llu\n", bt->end_lba);
 
 out_unlock_bdev:
-       mutex_unlock(&q->blk_trace_mutex);
+       mutex_unlock(&q->debugfs_mutex);
 out_bdput:
        bdput(bdev);
 out:
@@ -1901,10 +1895,10 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
        if (q == NULL)
                goto out_bdput;
 
-       mutex_lock(&q->blk_trace_mutex);
+       mutex_lock(&q->debugfs_mutex);
 
        bt = rcu_dereference_protected(q->blk_trace,
-                                      lockdep_is_held(&q->blk_trace_mutex));
+                                      lockdep_is_held(&q->debugfs_mutex));
        if (attr == &dev_attr_enable) {
                if (!!value == !!bt) {
                        ret = 0;
@@ -1921,7 +1915,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
        if (bt == NULL) {
                ret = blk_trace_setup_queue(q, bdev);
                bt = rcu_dereference_protected(q->blk_trace,
-                               lockdep_is_held(&q->blk_trace_mutex));
+                               lockdep_is_held(&q->debugfs_mutex));
        }
 
        if (ret == 0) {
@@ -1936,7 +1930,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
        }
 
 out_unlock_bdev:
-       mutex_unlock(&q->blk_trace_mutex);
+       mutex_unlock(&q->debugfs_mutex);
 out_bdput:
        bdput(bdev);
 out: