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]
Message-Id: <1423557587-21254-16-git-send-email-tomas.winkler@intel.com>
Date:	Tue, 10 Feb 2015 10:39:46 +0200
From:	Tomas Winkler <tomas.winkler@...el.com>
To:	gregkh@...uxfoundation.org
Cc:	arnd@...db.de, linux-kernel@...r.kernel.org,
	Tomas Winkler <tomas.winkler@...el.com>
Subject: [char-misc-next 17/18] mei: allow read concurrency

Replace clunky read state machine with read stack
implemented as per client read list, this is important
mostly for mei drivers with unsolicited reads

Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
---
 drivers/misc/mei/bus.c       | 36 ++++++++++---------
 drivers/misc/mei/client.c    | 84 ++++++++++++++++++++++++++++----------------
 drivers/misc/mei/client.h    |  7 ++--
 drivers/misc/mei/debugfs.c   |  2 +-
 drivers/misc/mei/init.c      |  1 -
 drivers/misc/mei/interrupt.c | 35 +++++-------------
 drivers/misc/mei/main.c      | 42 +++++-----------------
 drivers/misc/mei/mei_dev.h   | 10 +++---
 8 files changed, 98 insertions(+), 119 deletions(-)

diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index b5385372693d..17ca7e20fb6a 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -288,19 +288,20 @@ ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
 
 	mutex_lock(&dev->device_lock);
 
-	if (!cl->read_cb) {
-		rets = mei_cl_read_start(cl, length, NULL);
-		if (rets < 0)
-			goto out;
-	}
+	cb = mei_cl_read_cb(cl, NULL);
+	if (cb)
+		goto copy;
+
+	rets = mei_cl_read_start(cl, length, NULL);
+	if (rets && rets != -EBUSY)
+		goto out;
 
-	if (cl->reading_state != MEI_READ_COMPLETE &&
-	    !waitqueue_active(&cl->rx_wait)) {
+	if (list_empty(&cl->rd_completed) && !waitqueue_active(&cl->rx_wait)) {
 
 		mutex_unlock(&dev->device_lock);
 
 		if (wait_event_interruptible(cl->rx_wait,
-				cl->reading_state == MEI_READ_COMPLETE  ||
+				(!list_empty(&cl->rd_completed)) ||
 				mei_cl_is_transitioning(cl))) {
 
 			if (signal_pending(current))
@@ -309,15 +310,20 @@ ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
 		}
 
 		mutex_lock(&dev->device_lock);
-	}
 
+		if (mei_cl_is_transitioning(cl)) {
+			rets = -EBUSY;
+			goto out;
+		}
+	}
 
-	if (cl->reading_state != MEI_READ_COMPLETE) {
+	cb = mei_cl_read_cb(cl, NULL);
+	if (!cb) {
 		rets = 0;
 		goto out;
 	}
 
-	cb = cl->read_cb;
+copy:
 	if (cb->status) {
 		rets = cb->status;
 		goto free;
@@ -329,9 +335,6 @@ ssize_t __mei_cl_recv(struct mei_cl *cl, u8 *buf, size_t length)
 
 free:
 	mei_io_cb_free(cb);
-	cl->read_cb = NULL;
-	cl->reading_state = MEI_IDLE;
-
 out:
 	mutex_unlock(&dev->device_lock);
 
@@ -443,7 +446,7 @@ int mei_cl_enable_device(struct mei_cl_device *device)
 
 	mutex_unlock(&dev->device_lock);
 
-	if (device->event_cb && !cl->read_cb)
+	if (device->event_cb)
 		mei_cl_read_start(device->cl, 0, NULL);
 
 	if (!device->ops || !device->ops->enable)
@@ -485,8 +488,7 @@ int mei_cl_disable_device(struct mei_cl_device *device)
 	}
 
 	/* Flush queues and remove any pending read */
-	mei_cl_flush_queues(cl);
-	mei_io_cb_free(cl->read_cb);
+	mei_cl_flush_queues(cl, NULL);
 
 	device->event_cb = NULL;
 
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index f28eed12e7ab..e220bef0173c 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -458,13 +458,55 @@ struct mei_cl_cb *mei_cl_alloc_cb(struct mei_cl *cl, size_t length,
 }
 
 /**
+ * mei_cl_read_cb - find this cl's callback in the read list
+ *     for a specific file
+ *
+ * @cl: host client
+ * @fp: file pointer (matching cb file object), may be NULL
+ *
+ * Return: cb on success, NULL if cb is not found
+ */
+struct mei_cl_cb *mei_cl_read_cb(const struct mei_cl *cl, const struct file *fp)
+{
+	struct mei_cl_cb *cb;
+
+	list_for_each_entry(cb, &cl->rd_completed, list)
+		if (!fp || fp == cb->file_object)
+			return cb;
+
+	return NULL;
+}
+
+/**
+ * mei_cl_read_cb_flush - free client's read pending and completed cbs
+ *   for a specific file
+ *
+ * @cl: host client
+ * @fp: file pointer (matching cb file object), may be NULL
+ */
+void mei_cl_read_cb_flush(const struct mei_cl *cl, const struct file *fp)
+{
+	struct mei_cl_cb *cb, *next;
+
+	list_for_each_entry_safe(cb, next, &cl->rd_completed, list)
+		if (!fp || fp == cb->file_object)
+			mei_io_cb_free(cb);
+
+
+	list_for_each_entry_safe(cb, next, &cl->rd_pending, list)
+		if (!fp || fp == cb->file_object)
+			mei_io_cb_free(cb);
+}
+
+/**
  * mei_cl_flush_queues - flushes queue lists belonging to cl.
  *
  * @cl: host client
+ * @fp: file pointer (matching cb file object), may be NULL
  *
  * Return: 0 on success, -EINVAL if cl or cl->dev is NULL.
  */
-int mei_cl_flush_queues(struct mei_cl *cl)
+int mei_cl_flush_queues(struct mei_cl *cl, const struct file *fp)
 {
 	struct mei_device *dev;
 
@@ -474,13 +516,15 @@ int mei_cl_flush_queues(struct mei_cl *cl)
 	dev = cl->dev;
 
 	cl_dbg(dev, cl, "remove list entry belonging to cl\n");
-	mei_io_list_flush(&cl->dev->read_list, cl);
 	mei_io_list_free(&cl->dev->write_list, cl);
 	mei_io_list_free(&cl->dev->write_waiting_list, cl);
 	mei_io_list_flush(&cl->dev->ctrl_wr_list, cl);
 	mei_io_list_flush(&cl->dev->ctrl_rd_list, cl);
 	mei_io_list_flush(&cl->dev->amthif_cmd_list, cl);
 	mei_io_list_flush(&cl->dev->amthif_rd_complete_list, cl);
+
+	mei_cl_read_cb_flush(cl, fp);
+
 	return 0;
 }
 
@@ -497,9 +541,10 @@ void mei_cl_init(struct mei_cl *cl, struct mei_device *dev)
 	init_waitqueue_head(&cl->wait);
 	init_waitqueue_head(&cl->rx_wait);
 	init_waitqueue_head(&cl->tx_wait);
+	INIT_LIST_HEAD(&cl->rd_completed);
+	INIT_LIST_HEAD(&cl->rd_pending);
 	INIT_LIST_HEAD(&cl->link);
 	INIT_LIST_HEAD(&cl->device_link);
-	cl->reading_state = MEI_IDLE;
 	cl->writing_state = MEI_IDLE;
 	cl->dev = dev;
 }
@@ -524,24 +569,6 @@ struct mei_cl *mei_cl_allocate(struct mei_device *dev)
 }
 
 /**
- * mei_cl_find_read_cb - find this cl's callback in the read list
- *
- * @cl: host client
- *
- * Return: cb on success, NULL on error
- */
-struct mei_cl_cb *mei_cl_find_read_cb(struct mei_cl *cl)
-{
-	struct mei_device *dev = cl->dev;
-	struct mei_cl_cb *cb;
-
-	list_for_each_entry(cb, &dev->read_list.list, list)
-		if (mei_cl_cmp_id(cl, cb->cl))
-			return cb;
-	return NULL;
-}
-
-/**
  * mei_cl_link - allocate host id in the host map
  *
  * @cl: host client
@@ -1006,10 +1033,10 @@ int mei_cl_read_start(struct mei_cl *cl, size_t length, struct file *fp)
 	if (!mei_cl_is_connected(cl))
 		return -ENODEV;
 
-	if (cl->read_cb) {
-		cl_dbg(dev, cl, "read is pending.\n");
+	/* HW currently supports only one pending read */
+	if (!list_empty(&cl->rd_pending))
 		return -EBUSY;
-	}
+
 	me_cl = mei_me_cl_by_uuid_id(dev, &cl->cl_uuid, cl->me_client_id);
 	if (!me_cl) {
 		cl_err(dev, cl, "no such me client %d\n", cl->me_client_id);
@@ -1036,13 +1063,11 @@ int mei_cl_read_start(struct mei_cl *cl, size_t length, struct file *fp)
 		if (rets < 0)
 			goto out;
 
-		list_add_tail(&cb->list, &dev->read_list.list);
+		list_add_tail(&cb->list, &cl->rd_pending);
 	} else {
 		list_add_tail(&cb->list, &dev->ctrl_wr_list.list);
 	}
 
-	cl->read_cb = cb;
-
 out:
 	cl_dbg(dev, cl, "rpm: autosuspend\n");
 	pm_runtime_mark_last_busy(dev->dev);
@@ -1267,9 +1292,8 @@ void mei_cl_complete(struct mei_cl *cl, struct mei_cl_cb *cb)
 		if (waitqueue_active(&cl->tx_wait))
 			wake_up_interruptible(&cl->tx_wait);
 
-	} else if (cb->fop_type == MEI_FOP_READ &&
-			MEI_READING == cl->reading_state) {
-		cl->reading_state = MEI_READ_COMPLETE;
+	} else if (cb->fop_type == MEI_FOP_READ) {
+		list_add_tail(&cb->list, &cl->rd_completed);
 		if (waitqueue_active(&cl->rx_wait))
 			wake_up_interruptible(&cl->rx_wait);
 		else
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index c3d0e200a642..eb02f34b2fe0 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -77,11 +77,12 @@ int mei_cl_unlink(struct mei_cl *cl);
 
 struct mei_cl *mei_cl_alloc_linked(struct mei_device *dev, int id);
 
-int mei_cl_flush_queues(struct mei_cl *cl);
-struct mei_cl_cb *mei_cl_find_read_cb(struct mei_cl *cl);
-
+struct mei_cl_cb *mei_cl_read_cb(const struct mei_cl *cl,
+				 const struct file *fp);
+void mei_cl_read_cb_flush(const struct mei_cl *cl, const struct file *fp);
 struct mei_cl_cb *mei_cl_alloc_cb(struct mei_cl *cl, size_t length,
 				  enum mei_cb_file_ops type, struct file *fp);
+int mei_cl_flush_queues(struct mei_cl *cl, const struct file *fp);
 
 int mei_cl_flow_ctrl_creds(struct mei_cl *cl);
 
diff --git a/drivers/misc/mei/debugfs.c b/drivers/misc/mei/debugfs.c
index 50fc6635fab1..d9cd7e6ee484 100644
--- a/drivers/misc/mei/debugfs.c
+++ b/drivers/misc/mei/debugfs.c
@@ -117,7 +117,7 @@ static ssize_t mei_dbgfs_read_active(struct file *fp, char __user *ubuf,
 		pos += scnprintf(buf + pos, bufsz - pos,
 			"%2d|%2d|%4d|%5d|%2d|%2d|\n",
 			i, cl->me_client_id, cl->host_client_id, cl->state,
-			cl->reading_state, cl->writing_state);
+			!list_empty(&cl->rd_completed), cl->writing_state);
 		i++;
 	}
 out:
diff --git a/drivers/misc/mei/init.c b/drivers/misc/mei/init.c
index ad18b0d16f61..97353cf8d9b6 100644
--- a/drivers/misc/mei/init.c
+++ b/drivers/misc/mei/init.c
@@ -397,7 +397,6 @@ void mei_device_init(struct mei_device *dev,
 	dev->dev_state = MEI_DEV_INITIALIZING;
 	dev->reset_count = 0;
 
-	mei_io_list_init(&dev->read_list);
 	mei_io_list_init(&dev->write_list);
 	mei_io_list_init(&dev->write_waiting_list);
 	mei_io_list_init(&dev->ctrl_wr_list);
diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 1e2f3c774853..3f23629759db 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -68,18 +68,6 @@ static inline int mei_cl_hbm_equal(struct mei_cl *cl,
 	return cl->host_client_id == mei_hdr->host_addr &&
 		cl->me_client_id == mei_hdr->me_addr;
 }
-/**
- * mei_cl_is_reading - checks if the client is in reading state
- *
- * @cl: mei client
- *
- * Return: true if the client is reading
- */
-static bool mei_cl_is_reading(struct mei_cl *cl)
-{
-	return cl->state == MEI_FILE_CONNECTED &&
-		cl->reading_state != MEI_READ_COMPLETE;
-}
 
 /**
  * mei_irq_discard_msg  - discard received message
@@ -116,24 +104,18 @@ int mei_cl_irq_read_msg(struct mei_cl *cl,
 	struct mei_cl_cb *cb;
 	unsigned char *buffer = NULL;
 
-	list_for_each_entry(cb, &dev->read_list.list, list) {
-		if (cl == cb->cl)
-			break;
-	}
-
-	if (&cb->list == &dev->read_list.list) {
-		dev_err(dev->dev, "no reader found\n");
+	cb = list_first_entry_or_null(&cl->rd_pending, struct mei_cl_cb, list);
+	if (!cb) {
+		cl_err(dev, cl, "pending read cb not found\n");
 		goto out;
 	}
 
-	if (!mei_cl_is_reading(cl)) {
-		cl_err(dev, cl, "cl is not reading state=%d reading state=%d\n",
-			cl->state, cl->reading_state);
+	if (cl->state != MEI_FILE_CONNECTED) {
+		cl_dbg(dev, cl, "not connected\n");
+		cb->status = -ENODEV;
 		goto out;
 	}
 
-	cl->reading_state = MEI_READING;
-
 	if (cb->buf.size == 0 || cb->buf.data == NULL) {
 		cl_err(dev, cl, "response buffer is not allocated.\n");
 		list_move_tail(&cb->list, &complete_list->list);
@@ -163,8 +145,7 @@ int mei_cl_irq_read_msg(struct mei_cl *cl,
 
 	if (mei_hdr->msg_complete) {
 		cb->read_time = jiffies;
-		cl_dbg(dev, cl, "completed read length = %lu\n",
-			cb->buf_idx);
+		cl_dbg(dev, cl, "completed read length = %lu\n", cb->buf_idx);
 		list_move_tail(&cb->list, &complete_list->list);
 	}
 
@@ -281,7 +262,7 @@ static int mei_cl_irq_read(struct mei_cl *cl, struct mei_cl_cb *cb,
 		return ret;
 	}
 
-	list_move_tail(&cb->list, &dev->read_list.list);
+	list_move_tail(&cb->list, &cl->rd_pending);
 
 	return 0;
 }
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index c34853be963f..d80867e0d803 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -112,14 +112,11 @@ static int mei_release(struct inode *inode, struct file *file)
 		cl_dbg(dev, cl, "disconnecting\n");
 		rets = mei_cl_disconnect(cl);
 	}
-	mei_cl_flush_queues(cl);
+	mei_cl_flush_queues(cl, file);
 	cl_dbg(dev, cl, "removing\n");
 
 	mei_cl_unlink(cl);
 
-	mei_io_cb_free(cl->read_cb);
-	cl->read_cb = NULL;
-
 	file->private_data = NULL;
 
 	kfree(cl);
@@ -143,8 +140,8 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
 			size_t length, loff_t *offset)
 {
 	struct mei_cl *cl = file->private_data;
-	struct mei_cl_cb *cb = NULL;
 	struct mei_device *dev;
+	struct mei_cl_cb *cb = NULL;
 	int rets;
 	int err;
 
@@ -171,7 +168,7 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
 		goto out;
 	}
 
-	cb = cl->read_cb;
+	cb = mei_cl_read_cb(cl, file);
 	if (cb) {
 		/* read what left */
 		if (cb->buf_idx > *offset)
@@ -196,9 +193,7 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
 		goto out;
 	}
 
-	if (MEI_READ_COMPLETE != cl->reading_state &&
-		!waitqueue_active(&cl->rx_wait)) {
-
+	if (list_empty(&cl->rd_completed) && !waitqueue_active(&cl->rx_wait)) {
 		if (file->f_flags & O_NONBLOCK) {
 			rets = -EAGAIN;
 			goto out;
@@ -207,7 +202,7 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
 		mutex_unlock(&dev->device_lock);
 
 		if (wait_event_interruptible(cl->rx_wait,
-				MEI_READ_COMPLETE == cl->reading_state ||
+				(!list_empty(&cl->rd_completed)) ||
 				mei_cl_is_transitioning(cl))) {
 
 			if (signal_pending(current))
@@ -222,14 +217,8 @@ static ssize_t mei_read(struct file *file, char __user *ubuf,
 		}
 	}
 
-	cb = cl->read_cb;
-
+	cb = mei_cl_read_cb(cl, file);
 	if (!cb) {
-		rets = -ENODEV;
-		goto out;
-	}
-
-	if (cl->reading_state != MEI_READ_COMPLETE) {
 		rets = 0;
 		goto out;
 	}
@@ -266,9 +255,7 @@ copy_buffer:
 
 free:
 	mei_io_cb_free(cb);
-	cl->read_cb = NULL;
 
-	cl->reading_state = MEI_IDLE;
 out:
 	dev_dbg(dev->dev, "end mei read rets= %d\n", rets);
 	mutex_unlock(&dev->device_lock);
@@ -335,8 +322,7 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
 			timeout = write_cb->read_time +
 				mei_secs_to_jiffies(MEI_IAMTHIF_READ_TIMER);
 
-			if (time_after(jiffies, timeout) ||
-			    cl->reading_state == MEI_READ_COMPLETE) {
+			if (time_after(jiffies, timeout)) {
 				*offset = 0;
 				mei_io_cb_free(write_cb);
 				write_cb = NULL;
@@ -344,19 +330,7 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
 		}
 	}
 
-	/* free entry used in read */
-	if (cl->reading_state == MEI_READ_COMPLETE) {
-		*offset = 0;
-		write_cb = mei_cl_find_read_cb(cl);
-		if (write_cb) {
-			mei_io_cb_free(write_cb);
-			write_cb = NULL;
-			cl->read_cb = NULL;
-			cl->reading_state = MEI_IDLE;
-		}
-	} else if (cl->reading_state == MEI_IDLE)
-		*offset = 0;
-
+	*offset = 0;
 	write_cb = mei_cl_alloc_cb(cl, length, MEI_FOP_WRITE, file);
 	if (!write_cb) {
 		rets = -ENOMEM;
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 1a0f6e9588b6..f066ecd71939 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -231,9 +231,9 @@ struct mei_cl_cb {
  * @me_client_id: me/fw id
  * @mei_flow_ctrl_creds: transmit flow credentials
  * @timer_count:  watchdog timer for operation completion
- * @reading_state: state of the rx
  * @writing_state: state of the tx
- * @read_cb: current pending reading callback
+ * @rd_pending: pending read credits
+ * @rd_completed: completed read
  *
  * @device: device on the mei client bus
  * @device_link:  link to bus clients
@@ -251,9 +251,9 @@ struct mei_cl {
 	u8 me_client_id;
 	u8 mei_flow_ctrl_creds;
 	u8 timer_count;
-	enum mei_file_transaction_states reading_state;
 	enum mei_file_transaction_states writing_state;
-	struct mei_cl_cb *read_cb;
+	struct list_head rd_pending;
+	struct list_head rd_completed;
 
 	/* MEI CL bus data */
 	struct mei_cl_device *device;
@@ -425,7 +425,6 @@ const char *mei_pg_state_str(enum mei_pg_state state);
  * @cdev        : character device
  * @minor       : minor number allocated for device
  *
- * @read_list   : read completion list
  * @write_list  : write pending list
  * @write_waiting_list : write completion list
  * @ctrl_wr_list : pending control write list
@@ -501,7 +500,6 @@ struct mei_device {
 	struct cdev cdev;
 	int minor;
 
-	struct mei_cl_cb read_list;
 	struct mei_cl_cb write_list;
 	struct mei_cl_cb write_waiting_list;
 	struct mei_cl_cb ctrl_wr_list;
-- 
1.9.3

--
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