1 files changed, 68 insertions, 48 deletions

diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index d2209147dc89..e8d55a153a65 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -122,7 +122,6 @@ struct msc {
         atomic_t                mmap_count;
         struct mutex            buf_mutex;
 
-        struct mutex            iter_mutex;
         struct list_head        iter_list;
 
         /* config */
@@ -257,23 +256,37 @@ static struct msc_iter *msc_iter_install(struct msc *msc)
 
         iter = kzalloc(sizeof(*iter), GFP_KERNEL);
         if (!iter)
-                return NULL;
+                return ERR_PTR(-ENOMEM);
+
+        mutex_lock(&msc->buf_mutex);
+
+        /*
+         * Reading and tracing are mutually exclusive; if msc is
+         * enabled, open() will fail; otherwise existing readers
+         * will prevent enabling the msc and the rest of fops don't
+         * need to worry about it.
+         */
+        if (msc->enabled) {
+                kfree(iter);
+                iter = ERR_PTR(-EBUSY);
+                goto unlock;
+        }
 
         msc_iter_init(iter);
         iter->msc = msc;
 
-        mutex_lock(&msc->iter_mutex);
         list_add_tail(&iter->entry, &msc->iter_list);
-        mutex_unlock(&msc->iter_mutex);
+unlock:
+        mutex_unlock(&msc->buf_mutex);
 
         return iter;
 }
 
 static void msc_iter_remove(struct msc_iter *iter, struct msc *msc)
 {
-        mutex_lock(&msc->iter_mutex);
+        mutex_lock(&msc->buf_mutex);
         list_del(&iter->entry);
-        mutex_unlock(&msc->iter_mutex);
+        mutex_unlock(&msc->buf_mutex);
 
         kfree(iter);
 }
@@ -454,7 +467,6 @@ static void msc_buffer_clear_hw_header(struct msc *msc)
 {
         struct msc_window *win;
 
-        mutex_lock(&msc->buf_mutex);
         list_for_each_entry(win, &msc->win_list, entry) {
                 unsigned int blk;
                 size_t hw_sz = sizeof(struct msc_block_desc) -
@@ -466,7 +478,6 @@ static void msc_buffer_clear_hw_header(struct msc *msc)
                         memset(&bdesc->hw_tag, 0, hw_sz);
                 }
         }
-        mutex_unlock(&msc->buf_mutex);
 }
 
 /**
@@ -474,12 +485,15 @@ static void msc_buffer_clear_hw_header(struct msc *msc)
  * @msc:        the MSC device to configure
  *
  * Program storage mode, wrapping, burst length and trace buffer address
- * into a given MSC. If msc::enabled is set, enable the trace, too.
+ * into a given MSC. Then, enable tracing and set msc::enabled.
+ * The latter is serialized on msc::buf_mutex, so make sure to hold it.
  */
 static int msc_configure(struct msc *msc)
 {
         u32 reg;
 
+        lockdep_assert_held(&msc->buf_mutex);
+
         if (msc->mode > MSC_MODE_MULTI)
                 return -ENOTSUPP;
 
@@ -497,21 +511,19 @@ static int msc_configure(struct msc *msc)
         reg = ioread32(msc->reg_base + REG_MSU_MSC0CTL);
         reg &= ~(MSC_MODE | MSC_WRAPEN | MSC_EN | MSC_RD_HDR_OVRD);
 
+        reg |= MSC_EN;
         reg |= msc->mode << __ffs(MSC_MODE);
         reg |= msc->burst_len << __ffs(MSC_LEN);
-        /*if (msc->mode == MSC_MODE_MULTI)
-          reg |= MSC_RD_HDR_OVRD; */
+
         if (msc->wrap)
                 reg |= MSC_WRAPEN;
-        if (msc->enabled)
-                reg |= MSC_EN;
 
         iowrite32(reg, msc->reg_base + REG_MSU_MSC0CTL);
 
-        if (msc->enabled) {
-                msc->thdev->output.multiblock = msc->mode == MSC_MODE_MULTI;
-                intel_th_trace_enable(msc->thdev);
-        }
+        msc->thdev->output.multiblock = msc->mode == MSC_MODE_MULTI;
+        intel_th_trace_enable(msc->thdev);
+        msc->enabled = 1;
+
 
         return 0;
 }
@@ -521,15 +533,14 @@ static int msc_configure(struct msc *msc)
  * @msc:        MSC device to disable
  *
  * If @msc is enabled, disable tracing on the switch and then disable MSC
- * storage.
+ * storage. Caller must hold msc::buf_mutex.
  */
 static void msc_disable(struct msc *msc)
 {
         unsigned long count;
         u32 reg;
 
-        if (!msc->enabled)
-                return;
+        lockdep_assert_held(&msc->buf_mutex);
 
         intel_th_trace_disable(msc->thdev);
 
@@ -569,33 +580,35 @@ static void msc_disable(struct msc *msc)
 static int intel_th_msc_activate(struct intel_th_device *thdev)
 {
         struct msc *msc = dev_get_drvdata(&thdev->dev);
-        int ret = 0;
+        int ret = -EBUSY;
 
         if (!atomic_inc_unless_negative(&msc->user_count))
                 return -ENODEV;
 
-        mutex_lock(&msc->iter_mutex);
-        if (!list_empty(&msc->iter_list))
-                ret = -EBUSY;
-        mutex_unlock(&msc->iter_mutex);
+        mutex_lock(&msc->buf_mutex);
 
-        if (ret) {
-                atomic_dec(&msc->user_count);
-                return ret;
-        }
+        /* if there are readers, refuse */
+        if (list_empty(&msc->iter_list))
+                ret = msc_configure(msc);
 
-        msc->enabled = 1;
+        mutex_unlock(&msc->buf_mutex);
+
+        if (ret)
+                atomic_dec(&msc->user_count);
 
-        return msc_configure(msc);
+        return ret;
 }
 
 static void intel_th_msc_deactivate(struct intel_th_device *thdev)
 {
         struct msc *msc = dev_get_drvdata(&thdev->dev);
 
-        msc_disable(msc);
-
-        atomic_dec(&msc->user_count);
+        mutex_lock(&msc->buf_mutex);
+        if (msc->enabled) {
+                msc_disable(msc);
+                atomic_dec(&msc->user_count);
+        }
+        mutex_unlock(&msc->buf_mutex);
 }
 
 /**
@@ -1035,8 +1048,8 @@ static int intel_th_msc_open(struct inode *inode, struct file *file)
                 return -EPERM;
 
         iter = msc_iter_install(msc);
-        if (!iter)
-                return -ENOMEM;
+        if (IS_ERR(iter))
+                return PTR_ERR(iter);
 
         file->private_data = iter;
 
@@ -1101,11 +1114,6 @@ static ssize_t intel_th_msc_read(struct file *file, char __user *buf,
         if (!atomic_inc_unless_negative(&msc->user_count))
                 return 0;
 
-        if (msc->enabled) {
-                ret = -EBUSY;
-                goto put_count;
-        }
-
         if (msc->mode == MSC_MODE_SINGLE && !msc->single_wrap)
                 size = msc->single_sz;
         else
@@ -1245,6 +1253,7 @@ static const struct file_operations intel_th_msc_fops = {
         .read           = intel_th_msc_read,
         .mmap           = intel_th_msc_mmap,
         .llseek         = no_llseek,
+        .owner          = THIS_MODULE,
 };
 
 static int intel_th_msc_init(struct msc *msc)
@@ -1254,8 +1263,6 @@ static int intel_th_msc_init(struct msc *msc)
         msc->mode = MSC_MODE_MULTI;
         mutex_init(&msc->buf_mutex);
         INIT_LIST_HEAD(&msc->win_list);
-
-        mutex_init(&msc->iter_mutex);
         INIT_LIST_HEAD(&msc->iter_list);
 
         msc->burst_len =
@@ -1393,6 +1400,11 @@ nr_pages_store(struct device *dev, struct device_attribute *attr,
         do {
                 end = memchr(p, ',', len);
                 s = kstrndup(p, end ? end - p : len, GFP_KERNEL);
+                if (!s) {
+                        ret = -ENOMEM;
+                        goto free_win;
+                }
+
                 ret = kstrtoul(s, 10, &val);
                 kfree(s);
 
@@ -1473,10 +1485,6 @@ static int intel_th_msc_probe(struct intel_th_device *thdev)
         if (err)
                 return err;
 
-        err = sysfs_create_group(&dev->kobj, &msc_output_group);
-        if (err)
-                return err;
-
         dev_set_drvdata(dev, msc);
 
         return 0;
@@ -1484,7 +1492,18 @@ static int intel_th_msc_probe(struct intel_th_device *thdev)
 
 static void intel_th_msc_remove(struct intel_th_device *thdev)
 {
-        sysfs_remove_group(&thdev->dev.kobj, &msc_output_group);
+        struct msc *msc = dev_get_drvdata(&thdev->dev);
+        int ret;
+
+        intel_th_msc_deactivate(thdev);
+
+        /*
+         * Buffers should not be used at this point except if the
+         * output character device is still open and the parent
+         * device gets detached from its bus, which is a FIXME.
+         */
+        ret = msc_buffer_free_unless_used(msc);
+        WARN_ON_ONCE(ret);
 }
 
 static struct intel_th_driver intel_th_msc_driver = {
@@ -1493,6 +1512,7 @@ static struct intel_th_driver intel_th_msc_driver = {
         .activate       = intel_th_msc_activate,
         .deactivate     = intel_th_msc_deactivate,
         .fops   = &intel_th_msc_fops,
+        .attr_group     = &msc_output_group,
         .driver = {
                 .name   = "msc",
                 .owner  = THIS_MODULE,