1 files changed, 69 insertions, 49 deletions

diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index d9d6022c5..e8d55a153 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
@@ -1164,7 +1172,7 @@ static void msc_mmap_close(struct vm_area_struct *vma)
 	if (!atomic_dec_and_mutex_lock(&msc->mmap_count, &msc->buf_mutex))
 		return;
 
-	/* drop page _counts */
+	/* drop page _refcounts */
 	for (pg = 0; pg < msc->nr_pages; pg++) {
 		struct page *page = msc_buffer_get_page(msc, pg);
 
@@ -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,