blktrace: split do_blk_trace_setup into two functions

Split do_blk_trace_setup into two functions, this is done to prepare for
an incoming new BLKTRACESETUP2 ioctl(2) which can receive extended
parameters from user-space.

Also move the size verification logic to the callers in preparation for
using a new internal structure later.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 15d6788700ca268d51261b8eb0126a2bce697a1b..df90422ae6139ff7571d54e17c95cecdb00439d0 100644 (file)
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -518,9 +518,10 @@ static void blk_trace_setup_lba(struct blk_trace *bt,
 /*
  * Setup everything required to start tracing
  */
-static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
-                             struct block_device *bdev,
-                             struct blk_user_trace_setup *buts)
+static struct blk_trace *blk_trace_setup_prepare(struct request_queue *q,
+                                                char *name, dev_t dev,
+                                                u32 buf_size, u32 buf_nr,
+                                                struct block_device *bdev)
 {
        struct blk_trace *bt = NULL;
        struct dentry *dir = NULL;
@@ -528,31 +529,19 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
 
        lockdep_assert_held(&q->debugfs_mutex);
 
-       if (!buts->buf_size || !buts->buf_nr)
-               return -EINVAL;
-
-       strscpy_pad(buts->name, name, BLKTRACE_BDEV_SIZE);
-
-       /*
-        * some device names have larger paths - convert the slashes
-        * to underscores for this to work as expected
-        */
-       strreplace(buts->name, '/', '_');
-
        /*
         * bdev can be NULL, as with scsi-generic, this is a helpful as
         * we can be.
         */
        if (rcu_dereference_protected(q->blk_trace,
                                      lockdep_is_held(&q->debugfs_mutex))) {
-               pr_warn("Concurrent blktraces are not allowed on %s\n",
-                       buts->name);
-               return -EBUSY;
+               pr_warn("Concurrent blktraces are not allowed on %s\n", name);
+               return ERR_PTR(-EBUSY);
        }
 
        bt = kzalloc(sizeof(*bt), GFP_KERNEL);
        if (!bt)
-               return -ENOMEM;
+               return ERR_PTR(-ENOMEM);
 
        ret = -ENOMEM;
        bt->sequence = alloc_percpu(unsigned long);
@@ -572,7 +561,7 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
        if (bdev && !bdev_is_partition(bdev))
                dir = q->debugfs_dir;
        else
-               bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root);
+               bt->dir = dir = debugfs_create_dir(name, blk_debugfs_root);
 
        /*
         * As blktrace relies on debugfs for its interface the debugfs directory
@@ -580,8 +569,7 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
         * files or directories.
         */
        if (IS_ERR_OR_NULL(dir)) {
-               pr_warn("debugfs_dir not present for %s so skipping\n",
-                       buts->name);
+               pr_warn("debugfs_dir not present for %s so skipping\n", name);
                ret = -ENOENT;
                goto err;
        }
@@ -593,17 +581,38 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
        debugfs_create_file("dropped", 0444, dir, bt, &blk_dropped_fops);
        debugfs_create_file("msg", 0222, dir, bt, &blk_msg_fops);
 
-       bt->rchan = relay_open("trace", dir, buts->buf_size,
-                               buts->buf_nr, &blk_relay_callbacks, bt);
+       bt->rchan = relay_open("trace", dir, buf_size, buf_nr,
+                              &blk_relay_callbacks, bt);
        if (!bt->rchan)
                goto err;
 
+       blk_trace_setup_lba(bt, bdev);
+
+       return bt;
+
+err:
+       blk_trace_free(q, bt);
+
+       return ERR_PTR(ret);
+}
+
+static void blk_trace_setup_finalize(struct request_queue *q,
+                                    char *name, struct blk_trace *bt,
+                                    struct blk_user_trace_setup *buts)
+
+{
+       strscpy_pad(buts->name, name, BLKTRACE_BDEV_SIZE);
+
+       /*
+        * some device names have larger paths - convert the slashes
+        * to underscores for this to work as expected
+        */
+       strreplace(buts->name, '/', '_');
+
        bt->act_mask = buts->act_mask;
        if (!bt->act_mask)
                bt->act_mask = (u16) -1;
 
-       blk_trace_setup_lba(bt, bdev);
-
        /* overwrite with user settings */
        if (buts->start_lba)
                bt->start_lba = buts->start_lba;
@@ -615,12 +624,6 @@ static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
 
        rcu_assign_pointer(q->blk_trace, bt);
        get_probe_ref();
-
-       ret = 0;
-err:
-       if (ret)
-               blk_trace_free(q, bt);
-       return ret;
 }
 
 int blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
@@ -628,17 +631,25 @@ int blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
                    char __user *arg)
 {
        struct blk_user_trace_setup buts;
+       struct blk_trace *bt;
        int ret;
 
        ret = copy_from_user(&buts, arg, sizeof(buts));
        if (ret)
                return -EFAULT;
 
+       if (!buts.buf_size || !buts.buf_nr)
+               return -EINVAL;
+
        mutex_lock(&q->debugfs_mutex);
-       ret = do_blk_trace_setup(q, name, dev, bdev, &buts);
+       bt = blk_trace_setup_prepare(q, name, dev, buts.buf_size, buts.buf_nr,
+                                    bdev);
+       if (IS_ERR(bt)) {
+               mutex_unlock(&q->debugfs_mutex);
+               return PTR_ERR(bt);
+       }
+       blk_trace_setup_finalize(q, name, bt, &buts);
        mutex_unlock(&q->debugfs_mutex);
-       if (ret)
-               return ret;
 
        if (copy_to_user(arg, &buts, sizeof(buts))) {
                blk_trace_remove(q);
@@ -655,11 +666,14 @@ static int compat_blk_trace_setup(struct request_queue *q, char *name,
 {
        struct blk_user_trace_setup buts;
        struct compat_blk_user_trace_setup cbuts;
-       int ret;
+       struct blk_trace *bt;
 
        if (copy_from_user(&cbuts, arg, sizeof(cbuts)))
                return -EFAULT;
 
+       if (!cbuts.buf_size || !cbuts.buf_nr)
+               return -EINVAL;
+
        buts = (struct blk_user_trace_setup) {
                .act_mask = cbuts.act_mask,
                .buf_size = cbuts.buf_size,
@@ -670,10 +684,14 @@ static int compat_blk_trace_setup(struct request_queue *q, char *name,
        };
 
        mutex_lock(&q->debugfs_mutex);
-       ret = do_blk_trace_setup(q, name, dev, bdev, &buts);
+       bt = blk_trace_setup_prepare(q, name, dev, buts.buf_size, buts.buf_nr,
+                                    bdev);
+       if (IS_ERR(bt)) {
+               mutex_unlock(&q->debugfs_mutex);
+               return PTR_ERR(bt);
+       }
+       blk_trace_setup_finalize(q, name, bt, &buts);
        mutex_unlock(&q->debugfs_mutex);
-       if (ret)
-               return ret;
 
        if (copy_to_user(arg, &buts.name, ARRAY_SIZE(buts.name))) {
                blk_trace_remove(q);