Patchwork [v2,2/5] net: hub use lock to protect ports list

login
register
mail settings
Submitter pingfan liu
Date March 7, 2013, 2:53 a.m.
Message ID <1362624800-10682-3-git-send-email-qemulist@gmail.com>
Download mbox | patch
Permalink /patch/225710/
State New
Headers show

Comments

pingfan liu - March 7, 2013, 2:53 a.m.
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 net/hub.c |   27 ++++++++++++++++++++++++++-
 1 files changed, 26 insertions(+), 1 deletions(-)
Paolo Bonzini - March 12, 2013, 8:56 a.m.
Il 07/03/2013 03:53, Liu Ping Fan ha scritto:
> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> 
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
>  net/hub.c |   27 ++++++++++++++++++++++++++-
>  1 files changed, 26 insertions(+), 1 deletions(-)

You are introducing three locks.  Please document for each one how can
concurrent access to the lock happen, and also the lock hierarchy.

Paolo

Patch

diff --git a/net/hub.c b/net/hub.c
index 73c1f26..47fe72c 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -39,6 +39,8 @@  struct NetHub {
     int id;
     QLIST_ENTRY(NetHub) next;
     int num_ports;
+    /* protect ports */
+    QemuMutex lock;
     QLIST_HEAD(, NetHubPort) ports;
 };
 
@@ -49,6 +51,7 @@  static ssize_t net_hub_receive(NetHub *hub, NetHubPort *source_port,
 {
     NetHubPort *port;
 
+    qemu_mutex_lock(&hub->lock);
     QLIST_FOREACH(port, &hub->ports, next) {
         if (port == source_port) {
             continue;
@@ -58,6 +61,7 @@  static ssize_t net_hub_receive(NetHub *hub, NetHubPort *source_port,
                             QEMU_NET_PACKET_FLAG_NONE, buf, len, NULL);
         event_notifier_set(&port->e);
     }
+    qemu_mutex_unlock(&hub->lock);
     return len;
 }
 
@@ -74,6 +78,7 @@  static ssize_t net_hub_receive_iov(NetHub *hub, NetHubPort *source_port,
     NetHubPort *port;
     ssize_t len = iov_size(iov, iovcnt);
 
+    qemu_mutex_lock(&hub->lock);
     QLIST_FOREACH(port, &hub->ports, next) {
         if (port == source_port) {
             continue;
@@ -83,6 +88,7 @@  static ssize_t net_hub_receive_iov(NetHub *hub, NetHubPort *source_port,
                             QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, NULL);
         event_notifier_set(&port->e);
     }
+    qemu_mutex_unlock(&hub->lock);
     return len;
 }
 
@@ -93,6 +99,7 @@  static NetHub *net_hub_new(int id)
     hub = g_malloc(sizeof(*hub));
     hub->id = id;
     hub->num_ports = 0;
+    qemu_mutex_init(&hub->lock);
     QLIST_INIT(&hub->ports);
 
     QLIST_INSERT_HEAD(&hubs, hub, next);
@@ -106,16 +113,19 @@  static int net_hub_port_can_receive(NetClientState *nc)
     NetHubPort *src_port = DO_UPCAST(NetHubPort, nc, nc);
     NetHub *hub = src_port->hub;
 
+    qemu_mutex_lock(&hub->lock);
     QLIST_FOREACH(port, &hub->ports, next) {
         if (port == src_port) {
             continue;
         }
 
         if (qemu_can_send_packet(&port->nc)) {
+            qemu_mutex_unlock(&hub->lock);
             return 1;
         }
     }
 
+    qemu_mutex_unlock(&hub->lock);
     return 0;
 }
 
@@ -183,7 +193,9 @@  static NetHubPort *net_hub_port_new(NetHub *hub, const char *name)
     port->id = id;
     port->hub = hub;
     event_notifier_init(&port->e, 0);
+    qemu_mutex_lock(&hub->lock);
     QLIST_INSERT_HEAD(&hub->ports, port, next);
+    qemu_mutex_unlock(&hub->lock);
 
     return port;
 }
@@ -224,13 +236,16 @@  NetClientState *net_hub_find_client_by_name(int hub_id, const char *name)
 
     QLIST_FOREACH(hub, &hubs, next) {
         if (hub->id == hub_id) {
+            qemu_mutex_lock(&hub->lock);
             QLIST_FOREACH(port, &hub->ports, next) {
                 peer = port->nc.peer;
 
                 if (peer && strcmp(peer->name, name) == 0) {
+                    qemu_mutex_unlock(&hub->lock);
                     return peer;
                 }
             }
+            qemu_mutex_unlock(&hub->lock);
         }
     }
     return NULL;
@@ -247,12 +262,15 @@  NetClientState *net_hub_port_find(int hub_id)
 
     QLIST_FOREACH(hub, &hubs, next) {
         if (hub->id == hub_id) {
+            qemu_mutex_lock(&hub->lock);
             QLIST_FOREACH(port, &hub->ports, next) {
                 nc = port->nc.peer;
                 if (!nc) {
+                    qemu_mutex_unlock(&hub->lock);
                     return &(port->nc);
                 }
             }
+            qemu_mutex_unlock(&hub->lock);
             break;
         }
     }
@@ -271,12 +289,14 @@  void net_hub_info(Monitor *mon)
 
     QLIST_FOREACH(hub, &hubs, next) {
         monitor_printf(mon, "hub %d\n", hub->id);
+        qemu_mutex_lock(&hub->lock);
         QLIST_FOREACH(port, &hub->ports, next) {
             if (port->nc.peer) {
                 monitor_printf(mon, " \\ ");
                 print_net_client(mon, port->nc.peer);
             }
         }
+        qemu_mutex_unlock(&hub->lock);
     }
 }
 
@@ -333,6 +353,7 @@  void net_hub_check_clients(void)
     QLIST_FOREACH(hub, &hubs, next) {
         int has_nic = 0, has_host_dev = 0;
 
+        qemu_mutex_lock(&hub->lock);
         QLIST_FOREACH(port, &hub->ports, next) {
             peer = port->nc.peer;
             if (!peer) {
@@ -355,6 +376,7 @@  void net_hub_check_clients(void)
                 break;
             }
         }
+        qemu_mutex_unlock(&hub->lock);
         if (has_host_dev && !has_nic) {
             fprintf(stderr, "Warning: vlan %d with no nics\n", hub->id);
         }
@@ -370,12 +392,15 @@  bool net_hub_flush(NetClientState *nc)
 {
     NetHubPort *port;
     NetHubPort *source_port = DO_UPCAST(NetHubPort, nc, nc);
+    NetHub *hub = source_port->hub;
     int ret = 0;
 
-    QLIST_FOREACH(port, &source_port->hub->ports, next) {
+    qemu_mutex_lock(&hub->lock);
+    QLIST_FOREACH(port, &hub->ports, next) {
         if (port != source_port) {
             ret += qemu_net_queue_flush(port->nc.send_queue);
         }
     }
+    qemu_mutex_unlock(&hub->lock);
     return ret ? true : false;
 }