linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [char-misc-next V3] mei: add reference counting for me clients
@ 2014-11-03  8:42 Tomas Winkler
  2014-11-03 23:54 ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Tomas Winkler @ 2014-11-03  8:42 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

To support dynamic addition/remove we add reference
counter.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: code style and documentation fixes
V3: 1. Use refcounter also in mei_me_cl_rm_all, 
    2. Replace mei_me_cl_rm_by_id
        with mei_me_cl_rm_by_uuid_id
    3. Few more doc fixes

 drivers/misc/mei/amthif.c  |  14 +++--
 drivers/misc/mei/bus.c     |  39 +++++++------
 drivers/misc/mei/client.c  | 140 +++++++++++++++++++++++++++++++++++++--------
 drivers/misc/mei/client.h  |  17 ++++--
 drivers/misc/mei/debugfs.c |  32 +++++++----
 drivers/misc/mei/hbm.c     |  34 +++++------
 drivers/misc/mei/main.c    |  22 +++----
 drivers/misc/mei/mei_dev.h |   2 +
 drivers/misc/mei/nfc.c     |   2 +
 drivers/misc/mei/wd.c      |   1 +
 10 files changed, 209 insertions(+), 94 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index 56255ffe5a84..cafd9470a856 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -97,23 +97,25 @@ int mei_amthif_host_init(struct mei_device *dev)
 	/* allocate storage for ME message buffer */
 	msg_buf = kcalloc(dev->iamthif_mtu,
 			sizeof(unsigned char), GFP_KERNEL);
-	if (!msg_buf)
-		return -ENOMEM;
+	if (!msg_buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
 	dev->iamthif_msg_buf = msg_buf;
 
 	ret = mei_cl_link(cl, MEI_IAMTHIF_HOST_CLIENT_ID);
-
 	if (ret < 0) {
-		dev_err(dev->dev,
-			"amthif: failed link client %d\n", ret);
-		return ret;
+		dev_err(dev->dev, "amthif: failed cl_link %d\n", ret);
+		goto out;
 	}
 
 	ret = mei_cl_connect(cl, NULL);
 
 	dev->iamthif_state = MEI_IAMTHIF_IDLE;
 
+out:
+	mei_me_cl_put(me_cl);
 	return ret;
 }
 
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index b3a72bca5242..b2de93724f5b 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -228,8 +228,8 @@ static int ___mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
 			bool blocking)
 {
 	struct mei_device *dev;
-	struct mei_me_client *me_cl;
-	struct mei_cl_cb *cb;
+	struct mei_me_client *me_cl = NULL;
+	struct mei_cl_cb *cb = NULL;
 	int rets;
 
 	if (WARN_ON(!cl || !cl->dev))
@@ -237,33 +237,40 @@ static int ___mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
 
 	dev = cl->dev;
 
-	if (cl->state != MEI_FILE_CONNECTED)
-		return -ENODEV;
+	mutex_lock(&dev->device_lock);
+	if (cl->state != MEI_FILE_CONNECTED) {
+		rets = -ENODEV;
+		goto out;
+	}
 
 	/* Check if we have an ME client device */
 	me_cl = mei_me_cl_by_uuid_id(dev, &cl->cl_uuid, cl->me_client_id);
-	if (!me_cl)
-		return -ENOTTY;
+	if (!me_cl) {
+		rets = -ENOTTY;
+		goto out;
+	}
 
-	if (length > me_cl->props.max_msg_length)
-		return -EFBIG;
+	if (length > me_cl->props.max_msg_length) {
+		rets = -EFBIG;
+		goto out;
+	}
 
 	cb = mei_io_cb_init(cl, NULL);
-	if (!cb)
-		return -ENOMEM;
+	if (!cb) {
+		rets = -ENOMEM;
+		goto out;
+	}
 
 	rets = mei_io_cb_alloc_req_buf(cb, length);
-	if (rets < 0) {
-		mei_io_cb_free(cb);
-		return rets;
-	}
+	if (rets < 0)
+		goto out;
 
 	memcpy(cb->request_buffer.data, buf, length);
 
-	mutex_lock(&dev->device_lock);
-
 	rets = mei_cl_write(cl, cb, blocking);
 
+out:
+	mei_me_cl_put(me_cl);
 	mutex_unlock(&dev->device_lock);
 	if (rets < 0)
 		mei_io_cb_free(cb);
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 03c95e06ed1b..3a0bac150c73 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -27,7 +27,57 @@
 #include "client.h"
 
 /**
+ * mei_me_cl_init - initialize me client
+ *
+ * @me_cl: me client
+ */
+void mei_me_cl_init(struct mei_me_client *me_cl)
+{
+	INIT_LIST_HEAD(&me_cl->list);
+	kref_init(&me_cl->refcnt);
+}
+
+/**
+ * mei_me_cl_get - increases me client refcount
+ *
+ * @me_cl: me client
+ *
+ * Return: me client or NULL
+ */
+struct mei_me_client *mei_me_cl_get(struct mei_me_client *me_cl)
+{
+	if (me_cl)
+		kref_get(&me_cl->refcnt);
+
+	return me_cl;
+}
+
+/**
+ * mei_me_cl_release - unlink and free me client
+ *
+ * @ref: me_client refcount
+ */
+static void mei_me_cl_release(struct kref *ref)
+{
+	struct mei_me_client *me_cl =
+		container_of(ref, struct mei_me_client, refcnt);
+	list_del(&me_cl->list);
+	kfree(me_cl);
+}
+/**
+ * mei_me_cl_put - decrease me client refcount and free client if necessary
+ *
+ * @me_cl: me client
+ */
+void mei_me_cl_put(struct mei_me_client *me_cl)
+{
+	if (me_cl)
+		kref_put(&me_cl->refcnt, mei_me_cl_release);
+}
+
+/**
  * mei_me_cl_by_uuid - locate me client by uuid
+ *	increases ref count
  *
  * @dev: mei device
  * @uuid: me client uuid
@@ -43,13 +93,14 @@ struct mei_me_client *mei_me_cl_by_uuid(const struct mei_device *dev,
 
 	list_for_each_entry(me_cl, &dev->me_clients, list)
 		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0)
-			return me_cl;
+			return mei_me_cl_get(me_cl);
 
 	return NULL;
 }
 
 /**
  * mei_me_cl_by_id - locate me client by client id
+ *	increases ref count
  *
  * @dev: the device structure
  * @client_id: me client id
@@ -65,12 +116,14 @@ struct mei_me_client *mei_me_cl_by_id(struct mei_device *dev, u8 client_id)
 
 	list_for_each_entry(me_cl, &dev->me_clients, list)
 		if (me_cl->client_id == client_id)
-			return me_cl;
+			return mei_me_cl_get(me_cl);
+
 	return NULL;
 }
 
 /**
  * mei_me_cl_by_uuid_id - locate me client by client id and uuid
+ *	increases ref count
  *
  * @dev: the device structure
  * @uuid: me client uuid
@@ -88,31 +141,61 @@ struct mei_me_client *mei_me_cl_by_uuid_id(struct mei_device *dev,
 	list_for_each_entry(me_cl, &dev->me_clients, list)
 		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0 &&
 		    me_cl->client_id == client_id)
-			return me_cl;
+			return mei_me_cl_get(me_cl);
+
 	return NULL;
 }
 
 /**
- * mei_me_cl_remove - remove me client matching uuid and client_id
+ * mei_me_cl_rm_by_uuid - remove all me clients matching uuid
  *
  * @dev: the device structure
  * @uuid: me client uuid
- * @client_id: me client address
  */
-void mei_me_cl_remove(struct mei_device *dev, const uuid_le *uuid, u8 client_id)
+void mei_me_cl_rm_by_uuid(struct mei_device *dev, const uuid_le *uuid)
 {
 	struct mei_me_client *me_cl, *next;
 
+	dev_dbg(dev->dev, "remove %pUl\n", uuid);
+	list_for_each_entry_safe(me_cl, next, &dev->me_clients, list)
+		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0)
+			mei_me_cl_put(me_cl);
+}
+
+/**
+ * mei_me_cl_rm_by_uuid_id - remove all me clients matching client id
+ *
+ * @dev: the device structure
+ * @uuid: me client uuid
+ * @id: me client id
+ */
+void mei_me_cl_rm_by_uuid_id(struct mei_device *dev, const uuid_le *uuid, u8 id)
+{
+	struct mei_me_client *me_cl, *next;
+	const uuid_le *pn;
+
+	dev_dbg(dev->dev, "remove %pUl %d\n", uuid, id);
 	list_for_each_entry_safe(me_cl, next, &dev->me_clients, list) {
-		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0 &&
-		    me_cl->client_id == client_id) {
-			list_del(&me_cl->list);
-			kfree(me_cl);
-			break;
-		}
+		pn =  &me_cl->props.protocol_name;
+		if (me_cl->client_id == id && uuid_le_cmp(*uuid, *pn) == 0)
+			mei_me_cl_put(me_cl);
 	}
 }
 
+/**
+ * mei_me_cl_rm_all - remove all me clients
+ *
+ * @dev: the device structure
+ */
+void mei_me_cl_rm_all(struct mei_device *dev)
+{
+	struct mei_me_client *me_cl, *next;
+
+	list_for_each_entry_safe(me_cl, next, &dev->me_clients, list)
+			mei_me_cl_put(me_cl);
+}
+
+
 
 /**
  * mei_cl_cmp_id - tells if the clients are the same
@@ -695,6 +778,7 @@ int mei_cl_flow_ctrl_creds(struct mei_cl *cl)
 {
 	struct mei_device *dev;
 	struct mei_me_client *me_cl;
+	int rets = 0;
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -EINVAL;
@@ -710,12 +794,13 @@ int mei_cl_flow_ctrl_creds(struct mei_cl *cl)
 		return -ENOENT;
 	}
 
-	if (me_cl->mei_flow_ctrl_creds) {
+	if (me_cl->mei_flow_ctrl_creds > 0) {
+		rets = 1;
 		if (WARN_ON(me_cl->props.single_recv_buf == 0))
-			return -EINVAL;
-		return 1;
+			rets = -EINVAL;
 	}
-	return 0;
+	mei_me_cl_put(me_cl);
+	return rets;
 }
 
 /**
@@ -732,6 +817,7 @@ int mei_cl_flow_ctrl_reduce(struct mei_cl *cl)
 {
 	struct mei_device *dev;
 	struct mei_me_client *me_cl;
+	int rets;
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -EINVAL;
@@ -745,15 +831,22 @@ int mei_cl_flow_ctrl_reduce(struct mei_cl *cl)
 	}
 
 	if (me_cl->props.single_recv_buf) {
-		if (WARN_ON(me_cl->mei_flow_ctrl_creds <= 0))
-			return -EINVAL;
+		if (WARN_ON(me_cl->mei_flow_ctrl_creds <= 0)) {
+			rets = -EINVAL;
+			goto out;
+		}
 		me_cl->mei_flow_ctrl_creds--;
 	} else {
-		if (WARN_ON(cl->mei_flow_ctrl_creds <= 0))
-			return -EINVAL;
+		if (WARN_ON(cl->mei_flow_ctrl_creds <= 0)) {
+			rets = -EINVAL;
+			goto out;
+		}
 		cl->mei_flow_ctrl_creds--;
 	}
-	return 0;
+	rets = 0;
+out:
+	mei_me_cl_put(me_cl);
+	return rets;
 }
 
 /**
@@ -788,6 +881,9 @@ int mei_cl_read_start(struct mei_cl *cl, size_t length)
 		cl_err(dev, cl, "no such me client %d\n", cl->me_client_id);
 		return  -ENOTTY;
 	}
+	/* always allocate at least client max message */
+	length = max_t(size_t, length, me_cl->props.max_msg_length);
+	mei_me_cl_put(me_cl);
 
 	rets = pm_runtime_get(dev->dev);
 	if (rets < 0 && rets != -EINPROGRESS) {
@@ -802,8 +898,6 @@ int mei_cl_read_start(struct mei_cl *cl, size_t length)
 		goto out;
 	}
 
-	/* always allocate at least client max message */
-	length = max_t(size_t, length, me_cl->props.max_msg_length);
 	rets = mei_io_cb_alloc_resp_buf(cb, length);
 	if (rets)
 		goto out;
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index d9d0c1525259..cfcde8e97fc4 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -24,15 +24,22 @@
 
 #include "mei_dev.h"
 
+/*
+ * reference counting base function
+ */
+void mei_me_cl_init(struct mei_me_client *me_cl);
+void mei_me_cl_put(struct mei_me_client *me_cl);
+struct mei_me_client *mei_me_cl_get(struct mei_me_client *me_cl);
+
 struct mei_me_client *mei_me_cl_by_uuid(const struct mei_device *dev,
-					const uuid_le *cuuid);
+					const uuid_le *uuid);
 struct mei_me_client *mei_me_cl_by_id(struct mei_device *dev, u8 client_id);
-
 struct mei_me_client *mei_me_cl_by_uuid_id(struct mei_device *dev,
 					   const uuid_le *uuid, u8 client_id);
-
-void mei_me_cl_remove(struct mei_device *dev,
-		      const uuid_le *uuid, u8 client_id);
+void mei_me_cl_rm_by_uuid(struct mei_device *dev, const uuid_le *uuid);
+void mei_me_cl_rm_by_uuid_id(struct mei_device *dev,
+			     const uuid_le *uuid, u8 id);
+void mei_me_cl_rm_all(struct mei_device *dev);
 
 /*
  * MEI IO Functions
diff --git a/drivers/misc/mei/debugfs.c b/drivers/misc/mei/debugfs.c
index b60b4263cf0f..b125380ee871 100644
--- a/drivers/misc/mei/debugfs.c
+++ b/drivers/misc/mei/debugfs.c
@@ -21,20 +21,22 @@
 #include <linux/mei.h>
 
 #include "mei_dev.h"
+#include "client.h"
 #include "hw.h"
 
 static ssize_t mei_dbgfs_read_meclients(struct file *fp, char __user *ubuf,
 					size_t cnt, loff_t *ppos)
 {
 	struct mei_device *dev = fp->private_data;
-	struct mei_me_client *me_cl;
+	struct mei_me_client *me_cl, *n;
 	size_t bufsz = 1;
 	char *buf;
 	int i = 0;
 	int pos = 0;
 	int ret;
 
-#define HDR "  |id|fix|         UUID                       |con|msg len|sb|\n"
+#define HDR \
+"  |id|fix|         UUID                       |con|msg len|sb|refc|\n"
 
 	mutex_lock(&dev->device_lock);
 
@@ -54,16 +56,22 @@ static ssize_t mei_dbgfs_read_meclients(struct file *fp, char __user *ubuf,
 	if (dev->dev_state != MEI_DEV_ENABLED)
 		goto out;
 
-	list_for_each_entry(me_cl, &dev->me_clients, list) {
-
-		pos += scnprintf(buf + pos, bufsz - pos,
-			"%2d|%2d|%3d|%pUl|%3d|%7d|%2d|\n",
-			i++, me_cl->client_id,
-			me_cl->props.fixed_address,
-			&me_cl->props.protocol_name,
-			me_cl->props.max_number_of_connections,
-			me_cl->props.max_msg_length,
-			me_cl->props.single_recv_buf);
+	list_for_each_entry_safe(me_cl, n, &dev->me_clients, list) {
+
+		me_cl = mei_me_cl_get(me_cl);
+		if (me_cl) {
+			pos += scnprintf(buf + pos, bufsz - pos,
+				"%2d|%2d|%3d|%pUl|%3d|%7d|%2d|%4d|\n",
+				i++, me_cl->client_id,
+				me_cl->props.fixed_address,
+				&me_cl->props.protocol_name,
+				me_cl->props.max_number_of_connections,
+				me_cl->props.max_msg_length,
+				me_cl->props.single_recv_buf,
+				atomic_read(&me_cl->refcnt.refcount));
+		}
+
+		mei_me_cl_put(me_cl);
 	}
 out:
 	mutex_unlock(&dev->device_lock);
diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index f1ea0cee714d..cdcead8b1690 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -105,21 +105,6 @@ void mei_hbm_idle(struct mei_device *dev)
 }
 
 /**
- * mei_me_cl_remove_all - remove all me clients
- *
- * @dev: the device structure
- */
-static void mei_me_cl_remove_all(struct mei_device *dev)
-{
-	struct mei_me_client *me_cl, *next;
-
-	list_for_each_entry_safe(me_cl, next, &dev->me_clients, list) {
-			list_del(&me_cl->list);
-			kfree(me_cl);
-	}
-}
-
-/**
  * mei_hbm_reset - reset hbm counters and book keeping data structurs
  *
  * @dev: the device structure
@@ -128,7 +113,7 @@ void mei_hbm_reset(struct mei_device *dev)
 {
 	dev->me_client_index = 0;
 
-	mei_me_cl_remove_all(dev);
+	mei_me_cl_rm_all(dev);
 
 	mei_hbm_idle(dev);
 }
@@ -339,11 +324,16 @@ static int mei_hbm_me_cl_add(struct mei_device *dev,
 			     struct hbm_props_response *res)
 {
 	struct mei_me_client *me_cl;
+	const uuid_le *uuid = &res->client_properties.protocol_name;
+
+	mei_me_cl_rm_by_uuid(dev, uuid);
 
 	me_cl = kzalloc(sizeof(struct mei_me_client), GFP_KERNEL);
 	if (!me_cl)
 		return -ENOMEM;
 
+	mei_me_cl_init(me_cl);
+
 	me_cl->props = res->client_properties;
 	me_cl->client_id = res->me_addr;
 	me_cl->mei_flow_ctrl_creds = 0;
@@ -484,6 +474,7 @@ static int mei_hbm_add_single_flow_creds(struct mei_device *dev,
 				  struct hbm_flow_control *flow)
 {
 	struct mei_me_client *me_cl;
+	int rets;
 
 	me_cl = mei_me_cl_by_id(dev, flow->me_addr);
 	if (!me_cl) {
@@ -492,14 +483,19 @@ static int mei_hbm_add_single_flow_creds(struct mei_device *dev,
 		return -ENOENT;
 	}
 
-	if (WARN_ON(me_cl->props.single_recv_buf == 0))
-		return -EINVAL;
+	if (WARN_ON(me_cl->props.single_recv_buf == 0)) {
+		rets = -EINVAL;
+		goto out;
+	}
 
 	me_cl->mei_flow_ctrl_creds++;
 	dev_dbg(dev->dev, "recv flow ctrl msg ME %d (single) creds = %d.\n",
 	    flow->me_addr, me_cl->mei_flow_ctrl_creds);
 
-	return 0;
+	rets = 0;
+out:
+	mei_me_cl_put(me_cl);
+	return rets;
 }
 
 /**
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index beedc91f03a6..5d6a15c3619a 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -303,7 +303,7 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
 			 size_t length, loff_t *offset)
 {
 	struct mei_cl *cl = file->private_data;
-	struct mei_me_client *me_cl;
+	struct mei_me_client *me_cl = NULL;
 	struct mei_cl_cb *write_cb = NULL;
 	struct mei_device *dev;
 	unsigned long timeout = 0;
@@ -399,12 +399,14 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
 				"amthif write failed with status = %d\n", rets);
 			goto out;
 		}
+		mei_me_cl_put(me_cl);
 		mutex_unlock(&dev->device_lock);
 		return length;
 	}
 
 	rets = mei_cl_write(cl, write_cb, false);
 out:
+	mei_me_cl_put(me_cl);
 	mutex_unlock(&dev->device_lock);
 	if (rets < 0)
 		mei_io_cb_free(write_cb);
@@ -433,24 +435,19 @@ static int mei_ioctl_connect_client(struct file *file,
 	cl = file->private_data;
 	dev = cl->dev;
 
-	if (dev->dev_state != MEI_DEV_ENABLED) {
-		rets = -ENODEV;
-		goto end;
-	}
+	if (dev->dev_state != MEI_DEV_ENABLED)
+		return -ENODEV;
 
 	if (cl->state != MEI_FILE_INITIALIZING &&
-	    cl->state != MEI_FILE_DISCONNECTED) {
-		rets = -EBUSY;
-		goto end;
-	}
+	    cl->state != MEI_FILE_DISCONNECTED)
+		return  -EBUSY;
 
 	/* find ME client we're trying to connect to */
 	me_cl = mei_me_cl_by_uuid(dev, &data->in_client_uuid);
 	if (!me_cl || me_cl->props.fixed_address) {
 		dev_dbg(dev->dev, "Cannot connect to FW Client UUID = %pUl\n",
 				&data->in_client_uuid);
-		rets = -ENOTTY;
-		goto end;
+		return  -ENOTTY;
 	}
 
 	cl->me_client_id = me_cl->client_id;
@@ -487,17 +484,16 @@ static int mei_ioctl_connect_client(struct file *file,
 		goto end;
 	}
 
-
 	/* prepare the output buffer */
 	client = &data->out_client_properties;
 	client->max_msg_length = me_cl->props.max_msg_length;
 	client->protocol_version = me_cl->props.protocol_version;
 	dev_dbg(dev->dev, "Can connect?\n");
 
-
 	rets = mei_cl_connect(cl, file);
 
 end:
+	mei_me_cl_put(me_cl);
 	return rets;
 }
 
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 1288a5cf60b9..7d3a1de0b051 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -169,12 +169,14 @@ struct mei_fw_status {
  * struct mei_me_client - representation of me (fw) client
  *
  * @list: link in me client list
+ * @refcnt: struct reference count
  * @props: client properties
  * @client_id: me client id
  * @mei_flow_ctrl_creds: flow control credits
  */
 struct mei_me_client {
 	struct list_head list;
+	struct kref refcnt;
 	struct mei_client_properties props;
 	u8 client_id;
 	u8 mei_flow_ctrl_creds;
diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
index 82f38613c550..a10f3250aa2c 100644
--- a/drivers/misc/mei/nfc.c
+++ b/drivers/misc/mei/nfc.c
@@ -513,6 +513,7 @@ int mei_nfc_host_init(struct mei_device *dev)
 
 	cl_info->me_client_id = me_cl->client_id;
 	cl_info->cl_uuid = me_cl->props.protocol_name;
+	mei_me_cl_put(me_cl);
 
 	ret = mei_cl_link(cl_info, MEI_HOST_CLIENT_ID_ANY);
 	if (ret)
@@ -531,6 +532,7 @@ int mei_nfc_host_init(struct mei_device *dev)
 
 	cl->me_client_id = me_cl->client_id;
 	cl->cl_uuid = me_cl->props.protocol_name;
+	mei_me_cl_put(me_cl);
 
 	ret = mei_cl_link(cl, MEI_HOST_CLIENT_ID_ANY);
 	if (ret)
diff --git a/drivers/misc/mei/wd.c b/drivers/misc/mei/wd.c
index b1d892cea94d..475f1dea45bf 100644
--- a/drivers/misc/mei/wd.c
+++ b/drivers/misc/mei/wd.c
@@ -76,6 +76,7 @@ int mei_wd_host_init(struct mei_device *dev)
 
 	cl->me_client_id = me_cl->client_id;
 	cl->cl_uuid = me_cl->props.protocol_name;
+	mei_me_cl_put(me_cl);
 
 	ret = mei_cl_link(cl, MEI_WD_HOST_CLIENT_ID);
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [char-misc-next V3] mei: add reference counting for me clients
  2014-11-03  8:42 [char-misc-next V3] mei: add reference counting for me clients Tomas Winkler
@ 2014-11-03 23:54 ` Greg KH
  2014-11-04  5:22   ` Winkler, Tomas
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2014-11-03 23:54 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: arnd, linux-kernel

On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote:
> To support dynamic addition/remove we add reference
> counter.

What is keeping two different threads / cpus from grabbing a reference
at the same time the other one is dropping it?

You have a list here, with no locking, right?  You also don't have any
locking for your kref, which isn't good at all...

Please audit and fix up.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [char-misc-next V3] mei: add reference counting for me clients
  2014-11-03 23:54 ` Greg KH
@ 2014-11-04  5:22   ` Winkler, Tomas
  2014-11-06  2:20     ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Winkler, Tomas @ 2014-11-04  5:22 UTC (permalink / raw)
  To: Greg KH; +Cc: arnd, linux-kernel

> 
> On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote:
> > To support dynamic addition/remove we add reference
> > counter.
> 
> What is keeping two different threads / cpus from grabbing a reference
> at the same time the other one is dropping it?
> 
> You have a list here, with no locking, right?  You also don't have any
> locking for your kref, which isn't good at all...
> 
> Please audit and fix up.

Of course there is a lock, it wouldn't work at all. It is not explicit but we run under device_lock mutex. 

Tomas


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [char-misc-next V3] mei: add reference counting for me clients
  2014-11-04  5:22   ` Winkler, Tomas
@ 2014-11-06  2:20     ` Greg KH
  2014-11-06  8:40       ` Winkler, Tomas
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2014-11-06  2:20 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: arnd, linux-kernel

On Tue, Nov 04, 2014 at 05:22:51AM +0000, Winkler, Tomas wrote:
> > 
> > On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote:
> > > To support dynamic addition/remove we add reference
> > > counter.
> > 
> > What is keeping two different threads / cpus from grabbing a reference
> > at the same time the other one is dropping it?
> > 
> > You have a list here, with no locking, right?  You also don't have any
> > locking for your kref, which isn't good at all...
> > 
> > Please audit and fix up.
> 
> Of course there is a lock, it wouldn't work at all. It is not explicit
> but we run under device_lock mutex. 

Please make it explicit :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [char-misc-next V3] mei: add reference counting for me clients
  2014-11-06  2:20     ` Greg KH
@ 2014-11-06  8:40       ` Winkler, Tomas
  2014-11-06 15:58         ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: Winkler, Tomas @ 2014-11-06  8:40 UTC (permalink / raw)
  To: Greg KH; +Cc: arnd, linux-kernel

> 
> On Tue, Nov 04, 2014 at 05:22:51AM +0000, Winkler, Tomas wrote:
> > >
> > > On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote:
> > > > To support dynamic addition/remove we add reference
> > > > counter.
> > >
> > > What is keeping two different threads / cpus from grabbing a reference
> > > at the same time the other one is dropping it?
> > >
> > > You have a list here, with no locking, right?  You also don't have any
> > > locking for your kref, which isn't good at all...
> > >
> > > Please audit and fix up.
> >
> > Of course there is a lock, it wouldn't work at all. It is not explicit
> > but we run under device_lock mutex.
> 
> Please make it explicit :)

Not sure what you mean by that? There is a lot of options in this laconic  sentence.

Thanks
Tomas



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [char-misc-next V3] mei: add reference counting for me clients
  2014-11-06  8:40       ` Winkler, Tomas
@ 2014-11-06 15:58         ` Greg KH
  2014-11-06 19:05           ` Winkler, Tomas
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2014-11-06 15:58 UTC (permalink / raw)
  To: Winkler, Tomas; +Cc: arnd, linux-kernel

On Thu, Nov 06, 2014 at 08:40:21AM +0000, Winkler, Tomas wrote:
> > 
> > On Tue, Nov 04, 2014 at 05:22:51AM +0000, Winkler, Tomas wrote:
> > > >
> > > > On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote:
> > > > > To support dynamic addition/remove we add reference
> > > > > counter.
> > > >
> > > > What is keeping two different threads / cpus from grabbing a reference
> > > > at the same time the other one is dropping it?
> > > >
> > > > You have a list here, with no locking, right?  You also don't have any
> > > > locking for your kref, which isn't good at all...
> > > >
> > > > Please audit and fix up.
> > >
> > > Of course there is a lock, it wouldn't work at all. It is not explicit
> > > but we run under device_lock mutex.
> > 
> > Please make it explicit :)
> 
> Not sure what you mean by that? There is a lot of options in this laconic  sentence.

In looking at that patch, it was not obvious to me that any locking was
happening at all, so that needs to be addressed somehow before I can
accept the change.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [char-misc-next V3] mei: add reference counting for me clients
  2014-11-06 15:58         ` Greg KH
@ 2014-11-06 19:05           ` Winkler, Tomas
  0 siblings, 0 replies; 8+ messages in thread
From: Winkler, Tomas @ 2014-11-06 19:05 UTC (permalink / raw)
  To: Greg KH; +Cc: arnd, linux-kernel



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, November 06, 2014 17:58
> To: Winkler, Tomas
> Cc: arnd@arndb.de; linux-kernel@vger.kernel.org
> Subject: Re: [char-misc-next V3] mei: add reference counting for me clients
> 
> On Thu, Nov 06, 2014 at 08:40:21AM +0000, Winkler, Tomas wrote:
> > >
> > > On Tue, Nov 04, 2014 at 05:22:51AM +0000, Winkler, Tomas wrote:
> > > > >
> > > > > On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote:
> > > > > > To support dynamic addition/remove we add reference
> > > > > > counter.
> > > > >
> > > > > What is keeping two different threads / cpus from grabbing a reference
> > > > > at the same time the other one is dropping it?
> > > > >
> > > > > You have a list here, with no locking, right?  You also don't have any
> > > > > locking for your kref, which isn't good at all...
> > > > >
> > > > > Please audit and fix up.
> > > >
> > > > Of course there is a lock, it wouldn't work at all. It is not explicit
> > > > but we run under device_lock mutex.
> > >
> > > Please make it explicit :)
> >
> > Not sure what you mean by that? There is a lot of options in this laconic
> sentence.
> 
> In looking at that patch, it was not obvious to me that any locking was
> happening at all, so that needs to be addressed somehow before I can
> accept the change.

I understand the concern though we are using a BIG driver mutex so no additional locking is required, we are already locked. 
Currently we didn't hit any performance issue that required more fine grained locking but let say this is something we aware and that might  happen in the future. 

Thanks
Tomas


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [char-misc-next V3] mei: add reference counting for me clients
@ 2015-01-10 22:07 Tomas Winkler
  0 siblings, 0 replies; 8+ messages in thread
From: Tomas Winkler @ 2015-01-10 22:07 UTC (permalink / raw)
  To: gregkh; +Cc: arnd, linux-kernel, Tomas Winkler

To support dynamic addition and removal of
me clients we add reference counter.

Update kdoc with locking requirements.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: add locking requirements to kdoc
V3: fix warning: 'cb/me_cl' may be used uninitialized in this function 

 drivers/misc/mei/amthif.c  |  14 +++--
 drivers/misc/mei/bus.c     |  39 +++++++-----
 drivers/misc/mei/client.c  | 152 ++++++++++++++++++++++++++++++++++++++-------
 drivers/misc/mei/client.h  |  17 +++--
 drivers/misc/mei/debugfs.c |  32 ++++++----
 drivers/misc/mei/hbm.c     |  34 +++++-----
 drivers/misc/mei/main.c    |  22 +++----
 drivers/misc/mei/mei_dev.h |   2 +
 drivers/misc/mei/nfc.c     |   2 +
 drivers/misc/mei/wd.c      |   1 +
 10 files changed, 221 insertions(+), 94 deletions(-)

diff --git a/drivers/misc/mei/amthif.c b/drivers/misc/mei/amthif.c
index 79f53941779d..c4cb9a984a5f 100644
--- a/drivers/misc/mei/amthif.c
+++ b/drivers/misc/mei/amthif.c
@@ -97,23 +97,25 @@ int mei_amthif_host_init(struct mei_device *dev)
 	/* allocate storage for ME message buffer */
 	msg_buf = kcalloc(dev->iamthif_mtu,
 			sizeof(unsigned char), GFP_KERNEL);
-	if (!msg_buf)
-		return -ENOMEM;
+	if (!msg_buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
 	dev->iamthif_msg_buf = msg_buf;
 
 	ret = mei_cl_link(cl, MEI_IAMTHIF_HOST_CLIENT_ID);
-
 	if (ret < 0) {
-		dev_err(dev->dev,
-			"amthif: failed link client %d\n", ret);
-		return ret;
+		dev_err(dev->dev, "amthif: failed cl_link %d\n", ret);
+		goto out;
 	}
 
 	ret = mei_cl_connect(cl, NULL);
 
 	dev->iamthif_state = MEI_IAMTHIF_IDLE;
 
+out:
+	mei_me_cl_put(me_cl);
 	return ret;
 }
 
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index 31164dd14bd0..be767f4db26a 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -228,8 +228,8 @@ static ssize_t ___mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
 			bool blocking)
 {
 	struct mei_device *dev;
-	struct mei_me_client *me_cl;
-	struct mei_cl_cb *cb;
+	struct mei_me_client *me_cl = NULL;
+	struct mei_cl_cb *cb = NULL;
 	ssize_t rets;
 
 	if (WARN_ON(!cl || !cl->dev))
@@ -237,33 +237,40 @@ static ssize_t ___mei_cl_send(struct mei_cl *cl, u8 *buf, size_t length,
 
 	dev = cl->dev;
 
-	if (cl->state != MEI_FILE_CONNECTED)
-		return -ENODEV;
+	mutex_lock(&dev->device_lock);
+	if (cl->state != MEI_FILE_CONNECTED) {
+		rets = -ENODEV;
+		goto out;
+	}
 
 	/* Check if we have an ME client device */
 	me_cl = mei_me_cl_by_uuid_id(dev, &cl->cl_uuid, cl->me_client_id);
-	if (!me_cl)
-		return -ENOTTY;
+	if (!me_cl) {
+		rets = -ENOTTY;
+		goto out;
+	}
 
-	if (length > me_cl->props.max_msg_length)
-		return -EFBIG;
+	if (length > me_cl->props.max_msg_length) {
+		rets = -EFBIG;
+		goto out;
+	}
 
 	cb = mei_io_cb_init(cl, NULL);
-	if (!cb)
-		return -ENOMEM;
+	if (!cb) {
+		rets = -ENOMEM;
+		goto out;
+	}
 
 	rets = mei_io_cb_alloc_req_buf(cb, length);
-	if (rets < 0) {
-		mei_io_cb_free(cb);
-		return rets;
-	}
+	if (rets < 0)
+		goto out;
 
 	memcpy(cb->request_buffer.data, buf, length);
 
-	mutex_lock(&dev->device_lock);
-
 	rets = mei_cl_write(cl, cb, blocking);
 
+out:
+	mei_me_cl_put(me_cl);
 	mutex_unlock(&dev->device_lock);
 	if (rets < 0)
 		mei_io_cb_free(cb);
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 28e004b52dd2..9541218c10eb 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -27,7 +27,63 @@
 #include "client.h"
 
 /**
+ * mei_me_cl_init - initialize me client
+ *
+ * @me_cl: me client
+ */
+void mei_me_cl_init(struct mei_me_client *me_cl)
+{
+	INIT_LIST_HEAD(&me_cl->list);
+	kref_init(&me_cl->refcnt);
+}
+
+/**
+ * mei_me_cl_get - increases me client refcount
+ *
+ * @me_cl: me client
+ *
+ * Locking: called under "dev->device_lock" lock
+ *
+ * Return: me client or NULL
+ */
+struct mei_me_client *mei_me_cl_get(struct mei_me_client *me_cl)
+{
+	if (me_cl)
+		kref_get(&me_cl->refcnt);
+
+	return me_cl;
+}
+
+/**
+ * mei_me_cl_release - unlink and free me client
+ *
+ * Locking: called under "dev->device_lock" lock
+ *
+ * @ref: me_client refcount
+ */
+static void mei_me_cl_release(struct kref *ref)
+{
+	struct mei_me_client *me_cl =
+		container_of(ref, struct mei_me_client, refcnt);
+	list_del(&me_cl->list);
+	kfree(me_cl);
+}
+/**
+ * mei_me_cl_put - decrease me client refcount and free client if necessary
+ *
+ * Locking: called under "dev->device_lock" lock
+ *
+ * @me_cl: me client
+ */
+void mei_me_cl_put(struct mei_me_client *me_cl)
+{
+	if (me_cl)
+		kref_put(&me_cl->refcnt, mei_me_cl_release);
+}
+
+/**
  * mei_me_cl_by_uuid - locate me client by uuid
+ *	increases ref count
  *
  * @dev: mei device
  * @uuid: me client uuid
@@ -43,13 +99,14 @@ struct mei_me_client *mei_me_cl_by_uuid(const struct mei_device *dev,
 
 	list_for_each_entry(me_cl, &dev->me_clients, list)
 		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0)
-			return me_cl;
+			return mei_me_cl_get(me_cl);
 
 	return NULL;
 }
 
 /**
  * mei_me_cl_by_id - locate me client by client id
+ *	increases ref count
  *
  * @dev: the device structure
  * @client_id: me client id
@@ -65,12 +122,14 @@ struct mei_me_client *mei_me_cl_by_id(struct mei_device *dev, u8 client_id)
 
 	list_for_each_entry(me_cl, &dev->me_clients, list)
 		if (me_cl->client_id == client_id)
-			return me_cl;
+			return mei_me_cl_get(me_cl);
+
 	return NULL;
 }
 
 /**
  * mei_me_cl_by_uuid_id - locate me client by client id and uuid
+ *	increases ref count
  *
  * @dev: the device structure
  * @uuid: me client uuid
@@ -88,31 +147,67 @@ struct mei_me_client *mei_me_cl_by_uuid_id(struct mei_device *dev,
 	list_for_each_entry(me_cl, &dev->me_clients, list)
 		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0 &&
 		    me_cl->client_id == client_id)
-			return me_cl;
+			return mei_me_cl_get(me_cl);
+
 	return NULL;
 }
 
 /**
- * mei_me_cl_remove - remove me client matching uuid and client_id
+ * mei_me_cl_rm_by_uuid - remove all me clients matching uuid
  *
  * @dev: the device structure
  * @uuid: me client uuid
- * @client_id: me client address
+ *
+ * Locking: called under "dev->device_lock" lock
  */
-void mei_me_cl_remove(struct mei_device *dev, const uuid_le *uuid, u8 client_id)
+void mei_me_cl_rm_by_uuid(struct mei_device *dev, const uuid_le *uuid)
 {
 	struct mei_me_client *me_cl, *next;
 
+	dev_dbg(dev->dev, "remove %pUl\n", uuid);
+	list_for_each_entry_safe(me_cl, next, &dev->me_clients, list)
+		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0)
+			mei_me_cl_put(me_cl);
+}
+
+/**
+ * mei_me_cl_rm_by_uuid_id - remove all me clients matching client id
+ *
+ * @dev: the device structure
+ * @uuid: me client uuid
+ * @id: me client id
+ *
+ * Locking: called under "dev->device_lock" lock
+ */
+void mei_me_cl_rm_by_uuid_id(struct mei_device *dev, const uuid_le *uuid, u8 id)
+{
+	struct mei_me_client *me_cl, *next;
+	const uuid_le *pn;
+
+	dev_dbg(dev->dev, "remove %pUl %d\n", uuid, id);
 	list_for_each_entry_safe(me_cl, next, &dev->me_clients, list) {
-		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0 &&
-		    me_cl->client_id == client_id) {
-			list_del(&me_cl->list);
-			kfree(me_cl);
-			break;
-		}
+		pn =  &me_cl->props.protocol_name;
+		if (me_cl->client_id == id && uuid_le_cmp(*uuid, *pn) == 0)
+			mei_me_cl_put(me_cl);
 	}
 }
 
+/**
+ * mei_me_cl_rm_all - remove all me clients
+ *
+ * @dev: the device structure
+ *
+ * Locking: called under "dev->device_lock" lock
+ */
+void mei_me_cl_rm_all(struct mei_device *dev)
+{
+	struct mei_me_client *me_cl, *next;
+
+	list_for_each_entry_safe(me_cl, next, &dev->me_clients, list)
+			mei_me_cl_put(me_cl);
+}
+
+
 
 /**
  * mei_cl_cmp_id - tells if the clients are the same
@@ -695,6 +790,7 @@ int mei_cl_flow_ctrl_creds(struct mei_cl *cl)
 {
 	struct mei_device *dev;
 	struct mei_me_client *me_cl;
+	int rets = 0;
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -EINVAL;
@@ -710,12 +806,13 @@ int mei_cl_flow_ctrl_creds(struct mei_cl *cl)
 		return -ENOENT;
 	}
 
-	if (me_cl->mei_flow_ctrl_creds) {
+	if (me_cl->mei_flow_ctrl_creds > 0) {
+		rets = 1;
 		if (WARN_ON(me_cl->props.single_recv_buf == 0))
-			return -EINVAL;
-		return 1;
+			rets = -EINVAL;
 	}
-	return 0;
+	mei_me_cl_put(me_cl);
+	return rets;
 }
 
 /**
@@ -732,6 +829,7 @@ int mei_cl_flow_ctrl_reduce(struct mei_cl *cl)
 {
 	struct mei_device *dev;
 	struct mei_me_client *me_cl;
+	int rets;
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -EINVAL;
@@ -745,15 +843,22 @@ int mei_cl_flow_ctrl_reduce(struct mei_cl *cl)
 	}
 
 	if (me_cl->props.single_recv_buf) {
-		if (WARN_ON(me_cl->mei_flow_ctrl_creds <= 0))
-			return -EINVAL;
+		if (WARN_ON(me_cl->mei_flow_ctrl_creds <= 0)) {
+			rets = -EINVAL;
+			goto out;
+		}
 		me_cl->mei_flow_ctrl_creds--;
 	} else {
-		if (WARN_ON(cl->mei_flow_ctrl_creds <= 0))
-			return -EINVAL;
+		if (WARN_ON(cl->mei_flow_ctrl_creds <= 0)) {
+			rets = -EINVAL;
+			goto out;
+		}
 		cl->mei_flow_ctrl_creds--;
 	}
-	return 0;
+	rets = 0;
+out:
+	mei_me_cl_put(me_cl);
+	return rets;
 }
 
 /**
@@ -788,6 +893,9 @@ int mei_cl_read_start(struct mei_cl *cl, size_t length)
 		cl_err(dev, cl, "no such me client %d\n", cl->me_client_id);
 		return  -ENOTTY;
 	}
+	/* always allocate at least client max message */
+	length = max_t(size_t, length, me_cl->props.max_msg_length);
+	mei_me_cl_put(me_cl);
 
 	rets = pm_runtime_get(dev->dev);
 	if (rets < 0 && rets != -EINPROGRESS) {
@@ -802,8 +910,6 @@ int mei_cl_read_start(struct mei_cl *cl, size_t length)
 		goto out;
 	}
 
-	/* always allocate at least client max message */
-	length = max_t(size_t, length, me_cl->props.max_msg_length);
 	rets = mei_io_cb_alloc_resp_buf(cb, length);
 	if (rets)
 		goto out;
diff --git a/drivers/misc/mei/client.h b/drivers/misc/mei/client.h
index d9d0c1525259..cfcde8e97fc4 100644
--- a/drivers/misc/mei/client.h
+++ b/drivers/misc/mei/client.h
@@ -24,15 +24,22 @@
 
 #include "mei_dev.h"
 
+/*
+ * reference counting base function
+ */
+void mei_me_cl_init(struct mei_me_client *me_cl);
+void mei_me_cl_put(struct mei_me_client *me_cl);
+struct mei_me_client *mei_me_cl_get(struct mei_me_client *me_cl);
+
 struct mei_me_client *mei_me_cl_by_uuid(const struct mei_device *dev,
-					const uuid_le *cuuid);
+					const uuid_le *uuid);
 struct mei_me_client *mei_me_cl_by_id(struct mei_device *dev, u8 client_id);
-
 struct mei_me_client *mei_me_cl_by_uuid_id(struct mei_device *dev,
 					   const uuid_le *uuid, u8 client_id);
-
-void mei_me_cl_remove(struct mei_device *dev,
-		      const uuid_le *uuid, u8 client_id);
+void mei_me_cl_rm_by_uuid(struct mei_device *dev, const uuid_le *uuid);
+void mei_me_cl_rm_by_uuid_id(struct mei_device *dev,
+			     const uuid_le *uuid, u8 id);
+void mei_me_cl_rm_all(struct mei_device *dev);
 
 /*
  * MEI IO Functions
diff --git a/drivers/misc/mei/debugfs.c b/drivers/misc/mei/debugfs.c
index b60b4263cf0f..b125380ee871 100644
--- a/drivers/misc/mei/debugfs.c
+++ b/drivers/misc/mei/debugfs.c
@@ -21,20 +21,22 @@
 #include <linux/mei.h>
 
 #include "mei_dev.h"
+#include "client.h"
 #include "hw.h"
 
 static ssize_t mei_dbgfs_read_meclients(struct file *fp, char __user *ubuf,
 					size_t cnt, loff_t *ppos)
 {
 	struct mei_device *dev = fp->private_data;
-	struct mei_me_client *me_cl;
+	struct mei_me_client *me_cl, *n;
 	size_t bufsz = 1;
 	char *buf;
 	int i = 0;
 	int pos = 0;
 	int ret;
 
-#define HDR "  |id|fix|         UUID                       |con|msg len|sb|\n"
+#define HDR \
+"  |id|fix|         UUID                       |con|msg len|sb|refc|\n"
 
 	mutex_lock(&dev->device_lock);
 
@@ -54,16 +56,22 @@ static ssize_t mei_dbgfs_read_meclients(struct file *fp, char __user *ubuf,
 	if (dev->dev_state != MEI_DEV_ENABLED)
 		goto out;
 
-	list_for_each_entry(me_cl, &dev->me_clients, list) {
-
-		pos += scnprintf(buf + pos, bufsz - pos,
-			"%2d|%2d|%3d|%pUl|%3d|%7d|%2d|\n",
-			i++, me_cl->client_id,
-			me_cl->props.fixed_address,
-			&me_cl->props.protocol_name,
-			me_cl->props.max_number_of_connections,
-			me_cl->props.max_msg_length,
-			me_cl->props.single_recv_buf);
+	list_for_each_entry_safe(me_cl, n, &dev->me_clients, list) {
+
+		me_cl = mei_me_cl_get(me_cl);
+		if (me_cl) {
+			pos += scnprintf(buf + pos, bufsz - pos,
+				"%2d|%2d|%3d|%pUl|%3d|%7d|%2d|%4d|\n",
+				i++, me_cl->client_id,
+				me_cl->props.fixed_address,
+				&me_cl->props.protocol_name,
+				me_cl->props.max_number_of_connections,
+				me_cl->props.max_msg_length,
+				me_cl->props.single_recv_buf,
+				atomic_read(&me_cl->refcnt.refcount));
+		}
+
+		mei_me_cl_put(me_cl);
 	}
 out:
 	mutex_unlock(&dev->device_lock);
diff --git a/drivers/misc/mei/hbm.c b/drivers/misc/mei/hbm.c
index 239d7f5d6a92..c8412d41e4f1 100644
--- a/drivers/misc/mei/hbm.c
+++ b/drivers/misc/mei/hbm.c
@@ -105,21 +105,6 @@ void mei_hbm_idle(struct mei_device *dev)
 }
 
 /**
- * mei_me_cl_remove_all - remove all me clients
- *
- * @dev: the device structure
- */
-static void mei_me_cl_remove_all(struct mei_device *dev)
-{
-	struct mei_me_client *me_cl, *next;
-
-	list_for_each_entry_safe(me_cl, next, &dev->me_clients, list) {
-			list_del(&me_cl->list);
-			kfree(me_cl);
-	}
-}
-
-/**
  * mei_hbm_reset - reset hbm counters and book keeping data structurs
  *
  * @dev: the device structure
@@ -128,7 +113,7 @@ void mei_hbm_reset(struct mei_device *dev)
 {
 	dev->me_client_index = 0;
 
-	mei_me_cl_remove_all(dev);
+	mei_me_cl_rm_all(dev);
 
 	mei_hbm_idle(dev);
 }
@@ -339,11 +324,16 @@ static int mei_hbm_me_cl_add(struct mei_device *dev,
 			     struct hbm_props_response *res)
 {
 	struct mei_me_client *me_cl;
+	const uuid_le *uuid = &res->client_properties.protocol_name;
+
+	mei_me_cl_rm_by_uuid(dev, uuid);
 
 	me_cl = kzalloc(sizeof(struct mei_me_client), GFP_KERNEL);
 	if (!me_cl)
 		return -ENOMEM;
 
+	mei_me_cl_init(me_cl);
+
 	me_cl->props = res->client_properties;
 	me_cl->client_id = res->me_addr;
 	me_cl->mei_flow_ctrl_creds = 0;
@@ -484,6 +474,7 @@ static int mei_hbm_add_single_flow_creds(struct mei_device *dev,
 				  struct hbm_flow_control *flow)
 {
 	struct mei_me_client *me_cl;
+	int rets;
 
 	me_cl = mei_me_cl_by_id(dev, flow->me_addr);
 	if (!me_cl) {
@@ -492,14 +483,19 @@ static int mei_hbm_add_single_flow_creds(struct mei_device *dev,
 		return -ENOENT;
 	}
 
-	if (WARN_ON(me_cl->props.single_recv_buf == 0))
-		return -EINVAL;
+	if (WARN_ON(me_cl->props.single_recv_buf == 0)) {
+		rets = -EINVAL;
+		goto out;
+	}
 
 	me_cl->mei_flow_ctrl_creds++;
 	dev_dbg(dev->dev, "recv flow ctrl msg ME %d (single) creds = %d.\n",
 	    flow->me_addr, me_cl->mei_flow_ctrl_creds);
 
-	return 0;
+	rets = 0;
+out:
+	mei_me_cl_put(me_cl);
+	return rets;
 }
 
 /**
diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
index ae56ba6ca0e3..3c019c0e60eb 100644
--- a/drivers/misc/mei/main.c
+++ b/drivers/misc/mei/main.c
@@ -303,7 +303,7 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
 			 size_t length, loff_t *offset)
 {
 	struct mei_cl *cl = file->private_data;
-	struct mei_me_client *me_cl;
+	struct mei_me_client *me_cl = NULL;
 	struct mei_cl_cb *write_cb = NULL;
 	struct mei_device *dev;
 	unsigned long timeout = 0;
@@ -399,12 +399,14 @@ static ssize_t mei_write(struct file *file, const char __user *ubuf,
 				"amthif write failed with status = %d\n", rets);
 			goto out;
 		}
+		mei_me_cl_put(me_cl);
 		mutex_unlock(&dev->device_lock);
 		return length;
 	}
 
 	rets = mei_cl_write(cl, write_cb, false);
 out:
+	mei_me_cl_put(me_cl);
 	mutex_unlock(&dev->device_lock);
 	if (rets < 0)
 		mei_io_cb_free(write_cb);
@@ -433,24 +435,19 @@ static int mei_ioctl_connect_client(struct file *file,
 	cl = file->private_data;
 	dev = cl->dev;
 
-	if (dev->dev_state != MEI_DEV_ENABLED) {
-		rets = -ENODEV;
-		goto end;
-	}
+	if (dev->dev_state != MEI_DEV_ENABLED)
+		return -ENODEV;
 
 	if (cl->state != MEI_FILE_INITIALIZING &&
-	    cl->state != MEI_FILE_DISCONNECTED) {
-		rets = -EBUSY;
-		goto end;
-	}
+	    cl->state != MEI_FILE_DISCONNECTED)
+		return  -EBUSY;
 
 	/* find ME client we're trying to connect to */
 	me_cl = mei_me_cl_by_uuid(dev, &data->in_client_uuid);
 	if (!me_cl || me_cl->props.fixed_address) {
 		dev_dbg(dev->dev, "Cannot connect to FW Client UUID = %pUl\n",
 				&data->in_client_uuid);
-		rets = -ENOTTY;
-		goto end;
+		return  -ENOTTY;
 	}
 
 	cl->me_client_id = me_cl->client_id;
@@ -487,17 +484,16 @@ static int mei_ioctl_connect_client(struct file *file,
 		goto end;
 	}
 
-
 	/* prepare the output buffer */
 	client = &data->out_client_properties;
 	client->max_msg_length = me_cl->props.max_msg_length;
 	client->protocol_version = me_cl->props.protocol_version;
 	dev_dbg(dev->dev, "Can connect?\n");
 
-
 	rets = mei_cl_connect(cl, file);
 
 end:
+	mei_me_cl_put(me_cl);
 	return rets;
 }
 
diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 7f350af2ee10..6c6ce9381535 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -172,12 +172,14 @@ struct mei_fw_status {
  * struct mei_me_client - representation of me (fw) client
  *
  * @list: link in me client list
+ * @refcnt: struct reference count
  * @props: client properties
  * @client_id: me client id
  * @mei_flow_ctrl_creds: flow control credits
  */
 struct mei_me_client {
 	struct list_head list;
+	struct kref refcnt;
 	struct mei_client_properties props;
 	u8 client_id;
 	u8 mei_flow_ctrl_creds;
diff --git a/drivers/misc/mei/nfc.c b/drivers/misc/mei/nfc.c
index 60ca9240368e..bb61a119b8bb 100644
--- a/drivers/misc/mei/nfc.c
+++ b/drivers/misc/mei/nfc.c
@@ -521,6 +521,7 @@ int mei_nfc_host_init(struct mei_device *dev)
 
 	cl_info->me_client_id = me_cl->client_id;
 	cl_info->cl_uuid = me_cl->props.protocol_name;
+	mei_me_cl_put(me_cl);
 
 	ret = mei_cl_link(cl_info, MEI_HOST_CLIENT_ID_ANY);
 	if (ret)
@@ -539,6 +540,7 @@ int mei_nfc_host_init(struct mei_device *dev)
 
 	cl->me_client_id = me_cl->client_id;
 	cl->cl_uuid = me_cl->props.protocol_name;
+	mei_me_cl_put(me_cl);
 
 	ret = mei_cl_link(cl, MEI_HOST_CLIENT_ID_ANY);
 	if (ret)
diff --git a/drivers/misc/mei/wd.c b/drivers/misc/mei/wd.c
index b1d892cea94d..475f1dea45bf 100644
--- a/drivers/misc/mei/wd.c
+++ b/drivers/misc/mei/wd.c
@@ -76,6 +76,7 @@ int mei_wd_host_init(struct mei_device *dev)
 
 	cl->me_client_id = me_cl->client_id;
 	cl->cl_uuid = me_cl->props.protocol_name;
+	mei_me_cl_put(me_cl);
 
 	ret = mei_cl_link(cl, MEI_WD_HOST_CLIENT_ID);
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-01-10 22:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-03  8:42 [char-misc-next V3] mei: add reference counting for me clients Tomas Winkler
2014-11-03 23:54 ` Greg KH
2014-11-04  5:22   ` Winkler, Tomas
2014-11-06  2:20     ` Greg KH
2014-11-06  8:40       ` Winkler, Tomas
2014-11-06 15:58         ` Greg KH
2014-11-06 19:05           ` Winkler, Tomas
2015-01-10 22:07 Tomas Winkler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).