mei: revamp me clients list handling

1. Use rw lock to access the me_clients list

2. Reuse already defined find functions also when
removing particular me client

3. Add wrappers for addition  and deletion

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/misc/mei/client.c

@@ -48,14 +48,14 @@ void mei_me_cl_init(struct mei_me_client *me_cl)
  */
 struct mei_me_client *mei_me_cl_get(struct mei_me_client *me_cl)
 {
-	if (me_cl)
+	if (me_cl && kref_get_unless_zero(&me_cl->refcnt))
-		kref_get(&me_cl->refcnt);
+		return me_cl;
 
-	return me_cl;
+	return NULL;
 }
 
 /**
- * mei_me_cl_release - unlink and free me client
+ * mei_me_cl_release - free me client
  *
  * Locking: called under "dev->device_lock" lock
  *
@@ -65,9 +65,10 @@ 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
  *
@@ -81,6 +82,65 @@ void mei_me_cl_put(struct mei_me_client *me_cl)
 		kref_put(&me_cl->refcnt, mei_me_cl_release);
 }
 
+/**
+ * __mei_me_cl_del  - delete me client form the list and decrease
+ *     reference counter
+ *
+ * @dev: mei device
+ * @me_cl: me client
+ *
+ * Locking: dev->me_clients_rwsem
+ */
+static void __mei_me_cl_del(struct mei_device *dev, struct mei_me_client *me_cl)
+{
+	if (!me_cl)
+		return;
+
+	list_del(&me_cl->list);
+	mei_me_cl_put(me_cl);
+}
+
+/**
+ * mei_me_cl_add - add me client to the list
+ *
+ * @dev: mei device
+ * @me_cl: me client
+ */
+void mei_me_cl_add(struct mei_device *dev, struct mei_me_client *me_cl)
+{
+	down_write(&dev->me_clients_rwsem);
+	list_add(&me_cl->list, &dev->me_clients);
+	up_write(&dev->me_clients_rwsem);
+}
+
+/**
+ * __mei_me_cl_by_uuid - locate me client by uuid
+ *	increases ref count
+ *
+ * @dev: mei device
+ * @uuid: me client uuid
+ *
+ * Return: me client or NULL if not found
+ *
+ * Locking: dev->me_clients_rwsem
+ */
+static struct mei_me_client *__mei_me_cl_by_uuid(struct mei_device *dev,
+					const uuid_le *uuid)
+{
+	struct mei_me_client *me_cl;
+	const uuid_le *pn;
+
+	WARN_ON(!rwsem_is_locked(&dev->me_clients_rwsem));
+
+	list_for_each_entry(me_cl, &dev->me_clients, list) {
+		pn = &me_cl->props.protocol_name;
+		if (uuid_le_cmp(*uuid, *pn) == 0)
+			return mei_me_cl_get(me_cl);
+	}
+
+	return NULL;
+}
+
 /**
  * mei_me_cl_by_uuid - locate me client by uuid
  *	increases ref count
@@ -88,20 +148,20 @@ void mei_me_cl_put(struct mei_me_client *me_cl)
  * @dev: mei device
  * @uuid: me client uuid
  *
- * Locking: called under "dev->device_lock" lock
- *
  * Return: me client or NULL if not found
+ *
+ * Locking: dev->me_clients_rwsem
  */
-struct mei_me_client *mei_me_cl_by_uuid(const struct mei_device *dev,
+struct mei_me_client *mei_me_cl_by_uuid(struct mei_device *dev,
 					const uuid_le *uuid)
 {
 	struct mei_me_client *me_cl;
 
-	list_for_each_entry(me_cl, &dev->me_clients, list)
+	down_read(&dev->me_clients_rwsem);
-		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0)
+	me_cl = __mei_me_cl_by_uuid(dev, uuid);
-			return mei_me_cl_get(me_cl);
+	up_read(&dev->me_clients_rwsem);
 
-	return NULL;
+	return me_cl;
 }
 
 /**
@@ -111,22 +171,58 @@ struct mei_me_client *mei_me_cl_by_uuid(const struct mei_device *dev,
  * @dev: the device structure
  * @client_id: me client id
  *
- * Locking: called under "dev->device_lock" lock
- *
  * Return: me client or NULL if not found
+ *
+ * Locking: dev->me_clients_rwsem
  */
 struct mei_me_client *mei_me_cl_by_id(struct mei_device *dev, u8 client_id)
 {
 
-	struct mei_me_client *me_cl;
+	struct mei_me_client *__me_cl, *me_cl = NULL;
 
-	list_for_each_entry(me_cl, &dev->me_clients, list)
+	down_read(&dev->me_clients_rwsem);
-		if (me_cl->client_id == client_id)
+	list_for_each_entry(__me_cl, &dev->me_clients, list) {
+		if (__me_cl->client_id == client_id) {
+			me_cl = mei_me_cl_get(__me_cl);
+			break;
+		}
+	}
+	up_read(&dev->me_clients_rwsem);
+
+	return me_cl;
+}
+
+/**
+ * __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
+ * @client_id: me client id
+ *
+ * Return: me client or null if not found
+ *
+ * Locking: dev->me_clients_rwsem
+ */
+static struct mei_me_client *__mei_me_cl_by_uuid_id(struct mei_device *dev,
+					   const uuid_le *uuid, u8 client_id)
+{
+	struct mei_me_client *me_cl;
+	const uuid_le *pn;
+
+	WARN_ON(!rwsem_is_locked(&dev->me_clients_rwsem));
+
+	list_for_each_entry(me_cl, &dev->me_clients, list) {
+		pn = &me_cl->props.protocol_name;
+		if (uuid_le_cmp(*uuid, *pn) == 0 &&
+		    me_cl->client_id == client_id)
 			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
@@ -135,21 +231,18 @@ struct mei_me_client *mei_me_cl_by_id(struct mei_device *dev, u8 client_id)
  * @uuid: me client uuid
  * @client_id: me client id
  *
- * Locking: called under "dev->device_lock" lock
+ * Return: me client or null if not found
- *
- * Return: me client or NULL if not found
  */
 struct mei_me_client *mei_me_cl_by_uuid_id(struct mei_device *dev,
 					   const uuid_le *uuid, u8 client_id)
 {
 	struct mei_me_client *me_cl;
 
-	list_for_each_entry(me_cl, &dev->me_clients, list)
+	down_read(&dev->me_clients_rwsem);
-		if (uuid_le_cmp(*uuid, me_cl->props.protocol_name) == 0 &&
+	me_cl = __mei_me_cl_by_uuid_id(dev, uuid, client_id);
-		    me_cl->client_id == client_id)
+	up_read(&dev->me_clients_rwsem);
-			return mei_me_cl_get(me_cl);
 
-	return NULL;
+	return me_cl;
 }
 
 /**
@@ -162,12 +255,14 @@ struct mei_me_client *mei_me_cl_by_uuid_id(struct mei_device *dev,
  */
 void mei_me_cl_rm_by_uuid(struct mei_device *dev, const uuid_le *uuid)
 {
-	struct mei_me_client *me_cl, *next;
+	struct mei_me_client *me_cl;
 
 	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)
+	down_write(&dev->me_clients_rwsem);
-			mei_me_cl_put(me_cl);
+	me_cl = __mei_me_cl_by_uuid(dev, uuid);
+	__mei_me_cl_del(dev, me_cl);
+	up_write(&dev->me_clients_rwsem);
 }
 
 /**
@@ -181,15 +276,14 @@ 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)
 {
-	struct mei_me_client *me_cl, *next;
+	struct mei_me_client *me_cl;
-	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) {
+
-		pn =  &me_cl->props.protocol_name;
+	down_write(&dev->me_clients_rwsem);
-		if (me_cl->client_id == id && uuid_le_cmp(*uuid, *pn) == 0)
+	me_cl = __mei_me_cl_by_uuid_id(dev, uuid, id);
-			mei_me_cl_put(me_cl);
+	__mei_me_cl_del(dev, me_cl);
-	}
+	up_write(&dev->me_clients_rwsem);
 }
 
 /**
@@ -203,12 +297,12 @@ void mei_me_cl_rm_all(struct mei_device *dev)
 {
 	struct mei_me_client *me_cl, *next;
 
+	down_write(&dev->me_clients_rwsem);
 	list_for_each_entry_safe(me_cl, next, &dev->me_clients, list)
-			mei_me_cl_put(me_cl);
+		__mei_me_cl_del(dev, me_cl);
+	up_write(&dev->me_clients_rwsem);
 }
 
-
-
 /**
  * mei_cl_cmp_id - tells if the clients are the same
  *
@@ -535,28 +629,28 @@ int mei_cl_unlink(struct mei_cl *cl)
 
 void mei_host_client_init(struct work_struct *work)
 {
-	struct mei_device *dev = container_of(work,
+	struct mei_device *dev =
-					      struct mei_device, init_work);
+		container_of(work, struct mei_device, init_work);
 	struct mei_me_client *me_cl;
-	struct mei_client_properties *props;
 
 	mutex_lock(&dev->device_lock);
 
-	list_for_each_entry(me_cl, &dev->me_clients, list) {
-		props = &me_cl->props;
 
-		if (!uuid_le_cmp(props->protocol_name, mei_amthif_guid))
+	me_cl = mei_me_cl_by_uuid(dev, &mei_amthif_guid);
-			mei_amthif_host_init(dev);
+	if (me_cl)
-		else if (!uuid_le_cmp(props->protocol_name, mei_wd_guid))
+		mei_amthif_host_init(dev);
-			mei_wd_host_init(dev);
+
-		else if (!uuid_le_cmp(props->protocol_name, mei_nfc_guid))
+	me_cl = mei_me_cl_by_uuid(dev, &mei_wd_guid);
-			mei_nfc_host_init(dev);
+	if (me_cl)
+		mei_wd_host_init(dev);
+
+	me_cl = mei_me_cl_by_uuid(dev, &mei_nfc_guid);
+	if (me_cl)
+		mei_nfc_host_init(dev);
 
-	}
 
 	dev->dev_state = MEI_DEV_ENABLED;
 	dev->reset_count = 0;
-
 	mutex_unlock(&dev->device_lock);
 
 	pm_runtime_mark_last_busy(dev->dev);

drivers/misc/mei/client.h

@@ -31,7 +31,10 @@ 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,
+void mei_me_cl_add(struct mei_device *dev, struct mei_me_client *me_cl);
+void mei_me_cl_del(struct mei_device *dev, struct mei_me_client *me_cl);
+
+struct mei_me_client *mei_me_cl_by_uuid(struct mei_device *dev,
 					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,

drivers/misc/mei/debugfs.c

@@ -28,7 +28,7 @@ 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, *n;
+	struct mei_me_client *me_cl;
 	size_t bufsz = 1;
 	char *buf;
 	int i = 0;
@@ -38,15 +38,14 @@ static ssize_t mei_dbgfs_read_meclients(struct file *fp, char __user *ubuf,
 #define HDR \
 "  |id|fix|         UUID                       |con|msg len|sb|refc|\n"
 
-	mutex_lock(&dev->device_lock);
+	down_read(&dev->me_clients_rwsem);
-
 	list_for_each_entry(me_cl, &dev->me_clients, list)
 		bufsz++;
 
 	bufsz *= sizeof(HDR) + 1;
 	buf = kzalloc(bufsz, GFP_KERNEL);
 	if (!buf) {
-		mutex_unlock(&dev->device_lock);
+		up_read(&dev->me_clients_rwsem);
 		return -ENOMEM;
 	}
 
@@ -56,10 +55,9 @@ 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_safe(me_cl, n, &dev->me_clients, list) {
+	list_for_each_entry(me_cl, &dev->me_clients, list) {
 
-		me_cl = mei_me_cl_get(me_cl);
+		if (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,
@@ -69,12 +67,13 @@ static ssize_t mei_dbgfs_read_meclients(struct file *fp, char __user *ubuf,
 				me_cl->props.max_msg_length,
 				me_cl->props.single_recv_buf,
 				atomic_read(&me_cl->refcnt.refcount));
-		}
 
-		mei_me_cl_put(me_cl);
+			mei_me_cl_put(me_cl);
+		}
 	}
+
 out:
-	mutex_unlock(&dev->device_lock);
+	up_read(&dev->me_clients_rwsem);
 	ret = simple_read_from_buffer(ubuf, cnt, ppos, buf, pos);
 	kfree(buf);
 	return ret;

drivers/misc/mei/hbm.c

@@ -338,7 +338,8 @@ static int mei_hbm_me_cl_add(struct mei_device *dev,
 	me_cl->client_id = res->me_addr;
 	me_cl->mei_flow_ctrl_creds = 0;
 
-	list_add(&me_cl->list, &dev->me_clients);
+	mei_me_cl_add(dev, me_cl);
+
 	return 0;
 }
 

drivers/misc/mei/init.c

@@ -387,6 +387,7 @@ void mei_device_init(struct mei_device *dev,
 	INIT_LIST_HEAD(&dev->device_list);
 	INIT_LIST_HEAD(&dev->me_clients);
 	mutex_init(&dev->device_lock);
+	init_rwsem(&dev->me_clients_rwsem);
 	init_waitqueue_head(&dev->wait_hw_ready);
 	init_waitqueue_head(&dev->wait_pg);
 	init_waitqueue_head(&dev->wait_hbm_start);

drivers/misc/mei/mei_dev.h

@@ -460,6 +460,7 @@ const char *mei_pg_state_str(enum mei_pg_state state);
  * @version     : HBM protocol version in use
  * @hbm_f_pg_supported : hbm feature pgi protocol
  *
+ * @me_clients_rwsem: rw lock over me_clients list
  * @me_clients  : list of FW clients
  * @me_clients_map : FW clients bit map
  * @host_clients_map : host clients id pool
@@ -556,6 +557,7 @@ struct mei_device {
 	struct hbm_version version;
 	unsigned int hbm_f_pg_supported:1;
 
+	struct rw_semaphore me_clients_rwsem;
 	struct list_head me_clients;
 	DECLARE_BITMAP(me_clients_map, MEI_CLIENTS_MAX);
 	DECLARE_BITMAP(host_clients_map, MEI_CLIENTS_MAX);