msm: ipc: Reorganize server hash table
QMI server(<service_id:instance_id>) info is maintained using a hash
table. The server's instance_id is used as the key to index into that
hash table to perform maintenance operations(add, lookup, delete).
The service_id values are well distributed than the instance_id values
and hence the operations on the hash table are not optimized. Optimize
the hash table operations by using server's service_id as the key to
index into the hash table.
While accessing the server hash table, the mutexes are locked and
unlocked inside the access functions. This introduces some race
conditions once the access functions return. So lock and unlock the
server hash table mutex outside the access functions.
CRs-Fixed: 415158
Change-Id: Id1ce6bbfc677df26ff209fd62dca8553c7d2b4a5
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@codeaurora.org>
diff --git a/arch/arm/mach-msm/ipc_router.c b/arch/arm/mach-msm/ipc_router.c
index 1f2448d..0b0823050 100644
--- a/arch/arm/mach-msm/ipc_router.c
+++ b/arch/arm/mach-msm/ipc_router.c
@@ -96,6 +96,13 @@
static struct list_head local_ports[LP_HASH_SIZE];
static DEFINE_MUTEX(local_ports_lock);
+/*
+ * Server info is organized as a hash table. The server's service ID is
+ * used to index into the hash table. The instance ID of most of the servers
+ * are 1 or 2. The service IDs are well distributed compared to the instance
+ * IDs and hence choosing service ID to index into this hash table optimizes
+ * the hash table operations like add, lookup, destroy.
+ */
#define SRV_HASH_SIZE 32
static struct list_head server_list[SRV_HASH_SIZE];
static DEFINE_MUTEX(server_list_lock);
@@ -581,6 +588,20 @@
return;
}
+/**
+ * msm_ipc_router_lookup_server() - Lookup server information
+ * @service: Service ID of the server info to be looked up.
+ * @instance: Instance ID of the server info to be looked up.
+ * @node_id: Node/Processor ID in which the server is hosted.
+ * @port_id: Port ID within the node in which the server is hosted.
+ *
+ * @return: If found Pointer to server structure, else NULL.
+ *
+ * Note1: Lock the server_list_lock before accessing this function.
+ * Note2: If the <node_id:port_id> are <0:0>, then the lookup is restricted
+ * to <service:instance>. Used only when a client wants to send a
+ * message to any QMI server.
+ */
static struct msm_ipc_server *msm_ipc_router_lookup_server(
uint32_t service,
uint32_t instance,
@@ -589,30 +610,39 @@
{
struct msm_ipc_server *server;
struct msm_ipc_server_port *server_port;
- int key = (instance & (SRV_HASH_SIZE - 1));
+ int key = (service & (SRV_HASH_SIZE - 1));
- mutex_lock(&server_list_lock);
list_for_each_entry(server, &server_list[key], list) {
if ((server->name.service != service) ||
(server->name.instance != instance))
continue;
- if ((node_id == 0) && (port_id == 0)) {
- mutex_unlock(&server_list_lock);
+ if ((node_id == 0) && (port_id == 0))
return server;
- }
list_for_each_entry(server_port, &server->server_port_list,
list) {
if ((server_port->server_addr.node_id == node_id) &&
- (server_port->server_addr.port_id == port_id)) {
- mutex_unlock(&server_list_lock);
+ (server_port->server_addr.port_id == port_id))
return server;
- }
}
}
- mutex_unlock(&server_list_lock);
return NULL;
}
+/**
+ * msm_ipc_router_create_server() - Add server info to hash table
+ * @service: Service ID of the server info to be created.
+ * @instance: Instance ID of the server info to be created.
+ * @node_id: Node/Processor ID in which the server is hosted.
+ * @port_id: Port ID within the node in which the server is hosted.
+ * @xprt_info: XPRT through which the node hosting the server is reached.
+ *
+ * @return: Pointer to server structure on success, else NULL.
+ *
+ * This function adds the server info to the hash table. If the same
+ * server(i.e. <service_id:instance_id>) is hosted in different nodes,
+ * they are maintained as list of "server_port" under "server" structure.
+ * Note: Lock the server_list_lock before accessing this function.
+ */
static struct msm_ipc_server *msm_ipc_router_create_server(
uint32_t service,
uint32_t instance,
@@ -622,9 +652,8 @@
{
struct msm_ipc_server *server = NULL;
struct msm_ipc_server_port *server_port;
- int key = (instance & (SRV_HASH_SIZE - 1));
+ int key = (service & (SRV_HASH_SIZE - 1));
- mutex_lock(&server_list_lock);
list_for_each_entry(server, &server_list[key], list) {
if ((server->name.service == service) &&
(server->name.instance == instance))
@@ -633,7 +662,6 @@
server = kmalloc(sizeof(struct msm_ipc_server), GFP_KERNEL);
if (!server) {
- mutex_unlock(&server_list_lock);
pr_err("%s: Server allocation failed\n", __func__);
return NULL;
}
@@ -649,7 +677,6 @@
list_del(&server->list);
kfree(server);
}
- mutex_unlock(&server_list_lock);
pr_err("%s: Server Port allocation failed\n", __func__);
return NULL;
}
@@ -657,11 +684,22 @@
server_port->server_addr.port_id = port_id;
server_port->xprt_info = xprt_info;
list_add_tail(&server_port->list, &server->server_port_list);
- mutex_unlock(&server_list_lock);
return server;
}
+/**
+ * msm_ipc_router_destroy_server() - Remove server info from hash table
+ * @server: Server info to be removed.
+ * @node_id: Node/Processor ID in which the server is hosted.
+ * @port_id: Port ID within the node in which the server is hosted.
+ *
+ * This function removes the server_port identified using <node_id:port_id>
+ * from the server structure. If the server_port list under server structure
+ * is empty after removal, then remove the server structure from the server
+ * hash table.
+ * Note: Lock the server_list_lock before accessing this function.
+ */
static void msm_ipc_router_destroy_server(struct msm_ipc_server *server,
uint32_t node_id, uint32_t port_id)
{
@@ -670,7 +708,6 @@
if (!server)
return;
- mutex_lock(&server_list_lock);
list_for_each_entry(server_port, &server->server_port_list, list) {
if ((server_port->server_addr.node_id == node_id) &&
(server_port->server_addr.port_id == port_id))
@@ -684,7 +721,6 @@
list_del(&server->list);
kfree(server);
}
- mutex_unlock(&server_list_lock);
return;
}
@@ -1319,6 +1355,7 @@
}
mutex_unlock(&routing_table_lock);
+ mutex_lock(&server_list_lock);
server = msm_ipc_router_lookup_server(msg->srv.service,
msg->srv.instance,
msg->srv.node_id,
@@ -1328,6 +1365,7 @@
msg->srv.service, msg->srv.instance,
msg->srv.node_id, msg->srv.port_id, xprt_info);
if (!server) {
+ mutex_unlock(&server_list_lock);
pr_err("%s: Server Create failed\n", __func__);
return -ENOMEM;
}
@@ -1342,6 +1380,7 @@
}
wake_up(&newserver_wait);
}
+ mutex_unlock(&server_list_lock);
relay_msg(xprt_info, pkt);
post_control_ports(pkt);
@@ -1349,6 +1388,7 @@
case IPC_ROUTER_CTRL_CMD_REMOVE_SERVER:
RR("o REMOVE_SERVER service=%08x:%d\n",
msg->srv.service, msg->srv.instance);
+ mutex_lock(&server_list_lock);
server = msm_ipc_router_lookup_server(msg->srv.service,
msg->srv.instance,
msg->srv.node_id,
@@ -1360,6 +1400,7 @@
relay_msg(xprt_info, pkt);
post_control_ports(pkt);
}
+ mutex_unlock(&server_list_lock);
break;
case IPC_ROUTER_CTRL_CMD_REMOVE_CLIENT:
RR("o REMOVE_CLIENT id=%d:%08x\n",
@@ -1544,11 +1585,13 @@
if (name->addrtype != MSM_IPC_ADDR_NAME)
return -EINVAL;
+ mutex_lock(&server_list_lock);
server = msm_ipc_router_lookup_server(name->addr.port_name.service,
name->addr.port_name.instance,
IPC_ROUTER_NID_LOCAL,
port_ptr->this_port.port_id);
if (server) {
+ mutex_unlock(&server_list_lock);
pr_err("%s: Server already present\n", __func__);
return -EINVAL;
}
@@ -1559,6 +1602,7 @@
port_ptr->this_port.port_id,
NULL);
if (!server) {
+ mutex_unlock(&server_list_lock);
pr_err("%s: Server Creation failed\n", __func__);
return -EINVAL;
}
@@ -1568,6 +1612,7 @@
ctl.srv.instance = server->name.instance;
ctl.srv.node_id = IPC_ROUTER_NID_LOCAL;
ctl.srv.port_id = port_ptr->this_port.port_id;
+ mutex_unlock(&server_list_lock);
broadcast_ctl_msg(&ctl);
spin_lock_irqsave(&port_ptr->port_lock, flags);
port_ptr->type = SERVER_PORT;
@@ -1598,11 +1643,13 @@
return -EINVAL;
}
+ mutex_lock(&server_list_lock);
server = msm_ipc_router_lookup_server(port_ptr->port_name.service,
port_ptr->port_name.instance,
port_ptr->this_port.node_id,
port_ptr->this_port.port_id);
if (!server) {
+ mutex_unlock(&server_list_lock);
pr_err("%s: Server lookup failed\n", __func__);
return -ENODEV;
}
@@ -1612,9 +1659,10 @@
ctl.srv.instance = server->name.instance;
ctl.srv.node_id = IPC_ROUTER_NID_LOCAL;
ctl.srv.port_id = port_ptr->this_port.port_id;
- broadcast_ctl_msg(&ctl);
msm_ipc_router_destroy_server(server, port_ptr->this_port.node_id,
port_ptr->this_port.port_id);
+ mutex_unlock(&server_list_lock);
+ broadcast_ctl_msg(&ctl);
spin_lock_irqsave(&port_ptr->port_lock, flags);
port_ptr->type = CLIENT_PORT;
spin_unlock_irqrestore(&port_ptr->port_lock, flags);
@@ -1813,15 +1861,16 @@
dst_node_id = dest->addr.port_addr.node_id;
dst_port_id = dest->addr.port_addr.port_id;
} else if (dest->addrtype == MSM_IPC_ADDR_NAME) {
+ mutex_lock(&server_list_lock);
server = msm_ipc_router_lookup_server(
dest->addr.port_name.service,
dest->addr.port_name.instance,
0, 0);
if (!server) {
+ mutex_unlock(&server_list_lock);
pr_err("%s: Destination not reachable\n", __func__);
return -ENODEV;
}
- mutex_lock(&server_list_lock);
server_port = list_first_entry(&server->server_port_list,
struct msm_ipc_server_port,
list);
@@ -2005,6 +2054,7 @@
mutex_unlock(&port_ptr->port_rx_q_lock);
if (port_ptr->type == SERVER_PORT) {
+ mutex_lock(&server_list_lock);
server = msm_ipc_router_lookup_server(
port_ptr->port_name.service,
port_ptr->port_name.instance,
@@ -2014,6 +2064,7 @@
msm_ipc_router_destroy_server(server,
port_ptr->this_port.node_id,
port_ptr->this_port.port_id);
+ mutex_unlock(&server_list_lock);
}
wake_lock_destroy(&port_ptr->port_rx_wake_lock);
@@ -2078,27 +2129,24 @@
mutex_lock(&server_list_lock);
if (!lookup_mask)
lookup_mask = 0xFFFFFFFF;
- for (key = 0; key < SRV_HASH_SIZE; key++) {
- list_for_each_entry(server, &server_list[key], list) {
- if ((server->name.service != srv_name->service) ||
- ((server->name.instance & lookup_mask) !=
- srv_name->instance))
- continue;
+ key = (srv_name->service & (SRV_HASH_SIZE - 1));
+ list_for_each_entry(server, &server_list[key], list) {
+ if ((server->name.service != srv_name->service) ||
+ ((server->name.instance & lookup_mask) !=
+ srv_name->instance))
+ continue;
- list_for_each_entry(server_port,
- &server->server_port_list, list) {
- if (i < num_entries_in_array) {
- srv_info[i].node_id =
+ list_for_each_entry(server_port,
+ &server->server_port_list, list) {
+ if (i < num_entries_in_array) {
+ srv_info[i].node_id =
server_port->server_addr.node_id;
- srv_info[i].port_id =
+ srv_info[i].port_id =
server_port->server_addr.port_id;
- srv_info[i].service =
- server->name.service;
- srv_info[i].instance =
- server->name.instance;
- }
- i++;
+ srv_info[i].service = server->name.service;
+ srv_info[i].instance = server->name.instance;
}
+ i++;
}
}
mutex_unlock(&server_list_lock);