lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 9 Oct 2015 13:48:00 -0700
From:	Bjorn Andersson <bjorn.andersson@...ymobile.com>
To:	Andy Gross <agross@...eaurora.org>
CC:	Fengwei Yin <fengwei.yin@...aro.org>,
	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
	<linux-arm-msm@...r.kernel.org>, <linux-soc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: [PATCH v2 2/7] soc: qcom: smd: Split discovery and state change work

Split the two steps of channel discovery and state change handling into
two different workers. This allows for new channels to be found while
we're are probing, which is required as we introduce multi-channel
support.

Signed-off-by: Bjorn Andersson <bjorn.andersson@...ymobile.com>
---

Changes since v1:
- New patch

 drivers/soc/qcom/smd.c | 58 +++++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/soc/qcom/smd.c b/drivers/soc/qcom/smd.c
index 8b401d89b0d0..fb5f91efd0da 100644
--- a/drivers/soc/qcom/smd.c
+++ b/drivers/soc/qcom/smd.c
@@ -106,9 +106,9 @@ static const struct {
  * @channels:		list of all channels detected on this edge
  * @channels_lock:	guard for modifications of @channels
  * @allocated:		array of bitmaps representing already allocated channels
- * @need_rescan:	flag that the @work needs to scan smem for new channels
  * @smem_available:	last available amount of smem triggering a channel scan
- * @work:		work item for edge house keeping
+ * @scan_work:		work item for discovering new channels
+ * @state_work:		work item for edge state changes
  */
 struct qcom_smd_edge {
 	struct qcom_smd *smd;
@@ -123,14 +123,14 @@ struct qcom_smd_edge {
 	int ipc_bit;
 
 	struct list_head channels;
-	spinlock_t channels_lock;
+	rwlock_t channels_lock;
 
 	DECLARE_BITMAP(allocated[SMD_ALLOC_TBL_COUNT], SMD_ALLOC_TBL_SIZE);
 
-	bool need_rescan;
 	unsigned smem_available;
 
-	struct work_struct work;
+	struct work_struct scan_work;
+	struct work_struct state_work;
 };
 
 /*
@@ -624,13 +624,13 @@ static irqreturn_t qcom_smd_edge_intr(int irq, void *data)
 	/*
 	 * Handle state changes or data on each of the channels on this edge
 	 */
-	spin_lock(&edge->channels_lock);
+	read_lock(&edge->channels_lock);
 	list_for_each_entry(channel, &edge->channels, list) {
 		spin_lock(&channel->recv_lock);
 		kick_worker |= qcom_smd_channel_intr(channel);
 		spin_unlock(&channel->recv_lock);
 	}
-	spin_unlock(&edge->channels_lock);
+	read_unlock(&edge->channels_lock);
 
 	/*
 	 * Creating a new channel requires allocating an smem entry, so we only
@@ -640,12 +640,11 @@ static irqreturn_t qcom_smd_edge_intr(int irq, void *data)
 	available = qcom_smem_get_free_space(edge->remote_pid);
 	if (available != edge->smem_available) {
 		edge->smem_available = available;
-		edge->need_rescan = true;
 		kick_worker = true;
 	}
 
 	if (kick_worker)
-		schedule_work(&edge->work);
+		schedule_work(&edge->scan_work);
 
 	return IRQ_HANDLED;
 }
@@ -1101,8 +1100,9 @@ free_name_and_channel:
  * qcom_smd_create_channel() to create representations of these and add
  * them to the edge's list of channels.
  */
-static void qcom_discover_channels(struct qcom_smd_edge *edge)
+static void qcom_channel_scan_worker(struct work_struct *work)
 {
+	struct qcom_smd_edge *edge = container_of(work, struct qcom_smd_edge, scan_work);
 	struct qcom_smd_alloc_entry *alloc_tbl;
 	struct qcom_smd_alloc_entry *entry;
 	struct qcom_smd_channel *channel;
@@ -1146,16 +1146,16 @@ static void qcom_discover_channels(struct qcom_smd_edge *edge)
 			if (IS_ERR(channel))
 				continue;
 
-			spin_lock_irqsave(&edge->channels_lock, flags);
+			write_lock_irqsave(&edge->channels_lock, flags);
 			list_add(&channel->list, &edge->channels);
-			spin_unlock_irqrestore(&edge->channels_lock, flags);
+			write_unlock_irqrestore(&edge->channels_lock, flags);
 
 			dev_dbg(smd->dev, "new channel found: '%s'\n", channel->name);
 			set_bit(i, edge->allocated[tbl]);
 		}
 	}
 
-	schedule_work(&edge->work);
+	schedule_work(&edge->state_work);
 }
 
 /*
@@ -1163,29 +1163,22 @@ static void qcom_discover_channels(struct qcom_smd_edge *edge)
  * then scans all registered channels for state changes that should be handled
  * by creating or destroying smd client devices for the registered channels.
  *
- * LOCKING: edge->channels_lock is not needed to be held during the traversal
- * of the channels list as it's done synchronously with the only writer.
+ * LOCKING: edge->channels_lock only needs to cover the list operations, as the
+ * worker is killed before any channels are deallocated
  */
 static void qcom_channel_state_worker(struct work_struct *work)
 {
 	struct qcom_smd_channel *channel;
 	struct qcom_smd_edge *edge = container_of(work,
 						  struct qcom_smd_edge,
-						  work);
+						  state_work);
 	unsigned remote_state;
 
 	/*
-	 * Rescan smem if we have reason to belive that there are new channels.
-	 */
-	if (edge->need_rescan) {
-		edge->need_rescan = false;
-		qcom_discover_channels(edge);
-	}
-
-	/*
 	 * Register a device for any closed channel where the remote processor
 	 * is showing interest in opening the channel.
 	 */
+	read_lock(&edge->channels_lock);
 	list_for_each_entry(channel, &edge->channels, list) {
 		if (channel->state != SMD_CHANNEL_CLOSED)
 			continue;
@@ -1195,7 +1188,9 @@ static void qcom_channel_state_worker(struct work_struct *work)
 		    remote_state != SMD_CHANNEL_OPENED)
 			continue;
 
+		read_unlock(&edge->channels_lock);
 		qcom_smd_create_device(channel);
+		read_lock(&edge->channels_lock);
 	}
 
 	/*
@@ -1212,8 +1207,11 @@ static void qcom_channel_state_worker(struct work_struct *work)
 		    remote_state == SMD_CHANNEL_OPENED)
 			continue;
 
+		read_unlock(&edge->channels_lock);
 		qcom_smd_destroy_device(channel);
+		read_lock(&edge->channels_lock);
 	}
+	read_unlock(&edge->channels_lock);
 }
 
 /*
@@ -1229,9 +1227,10 @@ static int qcom_smd_parse_edge(struct device *dev,
 	int ret;
 
 	INIT_LIST_HEAD(&edge->channels);
-	spin_lock_init(&edge->channels_lock);
+	rwlock_init(&edge->channels_lock);
 
-	INIT_WORK(&edge->work, qcom_channel_state_worker);
+	INIT_WORK(&edge->scan_work, qcom_channel_scan_worker);
+	INIT_WORK(&edge->state_work, qcom_channel_state_worker);
 
 	edge->of_node = of_node_get(node);
 
@@ -1320,8 +1319,7 @@ static int qcom_smd_probe(struct platform_device *pdev)
 		if (ret)
 			continue;
 
-		edge->need_rescan = true;
-		schedule_work(&edge->work);
+		schedule_work(&edge->scan_work);
 	}
 
 	platform_set_drvdata(pdev, smd);
@@ -1344,8 +1342,10 @@ static int qcom_smd_remove(struct platform_device *pdev)
 		edge = &smd->edges[i];
 
 		disable_irq(edge->irq);
-		cancel_work_sync(&edge->work);
+		cancel_work_sync(&edge->scan_work);
+		cancel_work_sync(&edge->state_work);
 
+		/* No need to lock here, because the writer is gone */
 		list_for_each_entry(channel, &edge->channels, list) {
 			if (!channel->qsdev)
 				continue;
-- 
2.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ