Ben,
Minor comments below,
Jarno
Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
On May 8, 2014, at 4:50 PM, Ben Pfaff <b...@nicira.com> wrote:
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/dpif-netdev.c | 163 +++++++++++++++++++++++++++--------------------------
> 1 file changed, 82 insertions(+), 81 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 55712dd..b682876 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -32,6 +32,7 @@
(snip)
> @@ -671,7 +669,7 @@ dp_netdev_reload_pmd_threads(struct dp_netdev *dp)
> static int
> do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
> odp_port_t port_no)
> - OVS_REQ_WRLOCK(dp->port_rwlock)
> + OVS_REQUIRES(dp->port_mutex)
> {
> struct netdev_saved_flags *sf;
> struct dp_netdev_port *port;
> @@ -733,7 +731,7 @@ do_add_port(struct dp_netdev *dp, const char *devname,
> const char *type,
> }
> ovs_refcount_init(&port->ref_cnt);
>
> - hmap_insert(&dp->ports, &port->node, hash_int(odp_to_u32(port_no), 0));
> + cmap_insert(&dp->ports, &port->node, hash_int(odp_to_u32(port_no), 0));
Could use the new hash_port_no() here.
> seq_change(dp->port_seq);
>
> return 0;
> @@ -749,7 +747,7 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev
> *netdev,
> odp_port_t port_no;
> int error;
>
> - ovs_rwlock_wrlock(&dp->port_rwlock);
> + ovs_mutex_lock(&dp->port_mutex);
> dpif_port = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
> if (*port_nop != ODPP_NONE) {
> port_no = *port_nop;
> @@ -762,7 +760,7 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev
> *netdev,
> *port_nop = port_no;
> error = do_add_port(dp, dpif_port, netdev_get_type(netdev), port_no);
> }
> - ovs_rwlock_unlock(&dp->port_rwlock);
> + ovs_mutex_unlock(&dp->port_mutex);
>
> return error;
> }
> @@ -773,9 +771,9 @@ dpif_netdev_port_del(struct dpif *dpif, odp_port_t
> port_no)
> struct dp_netdev *dp = get_dp_netdev(dpif);
> int error;
>
> - ovs_rwlock_wrlock(&dp->port_rwlock);
> + ovs_mutex_lock(&dp->port_mutex);
> error = port_no == ODPP_LOCAL ? EINVAL : do_del_port(dp, port_no);
> - ovs_rwlock_unlock(&dp->port_rwlock);
> + ovs_mutex_unlock(&dp->port_mutex);
>
> return error;
> }
> @@ -786,14 +784,18 @@ is_valid_port_number(odp_port_t port_no)
> return port_no != ODPP_NONE;
> }
>
> +static uint32_t
> +hash_port_no(odp_port_t port_no)
> +{
> + return hash_int(odp_to_u32(port_no), 0);
> +}
> +
(snip)
> @@ -1783,10 +1784,9 @@ dpif_netdev_run(struct dpif *dpif)
> {
> struct dp_netdev_port *port;
> struct dp_netdev *dp = get_dp_netdev(dpif);
> + struct cmap_cursor cursor;
>
> - ovs_rwlock_rdlock(&dp->port_rwlock);
> -
Unless it is OK to sometimes skip a port (due a concurrent insert, etc.) then
we should still exclude writers here.
> - HMAP_FOR_EACH (port, node, &dp->ports) {
> + CMAP_FOR_EACH (port, node, &cursor, &dp->ports) {
> if (!netdev_is_pmd(port->netdev)) {
> int i;
>
> @@ -1795,8 +1795,6 @@ dpif_netdev_run(struct dpif *dpif)
> }
> }
> }
> -
> - ovs_rwlock_unlock(&dp->port_rwlock);
> }
>
> static void
> @@ -1804,10 +1802,9 @@ dpif_netdev_wait(struct dpif *dpif)
> {
> struct dp_netdev_port *port;
> struct dp_netdev *dp = get_dp_netdev(dpif);
> + struct cmap_cursor cursor;
>
> - ovs_rwlock_rdlock(&dp->port_rwlock);
> -
Same here.
> - HMAP_FOR_EACH (port, node, &dp->ports) {
> + CMAP_FOR_EACH (port, node, &cursor, &dp->ports) {
> if (!netdev_is_pmd(port->netdev)) {
> int i;
>
> @@ -1816,7 +1813,6 @@ dpif_netdev_wait(struct dpif *dpif)
> }
> }
> }
> - ovs_rwlock_unlock(&dp->port_rwlock);
> }
>
> struct rxq_poll {
> @@ -1831,12 +1827,12 @@ pmd_load_queues(struct pmd_thread *f,
> struct dp_netdev *dp = f->dp;
> struct rxq_poll *poll_list = *ppoll_list;
> struct dp_netdev_port *port;
> + struct cmap_cursor cursor;
> int id = f->id;
> int index;
> int i;
>
> /* Simple scheduler for netdev rx polling. */
> - ovs_rwlock_rdlock(&dp->port_rwlock);
And here.
> for (i = 0; i < poll_cnt; i++) {
> port_unref(poll_list[i].port);
> }
> @@ -1844,7 +1840,7 @@ pmd_load_queues(struct pmd_thread *f,
> poll_cnt = 0;
> index = 0;
>
> - HMAP_FOR_EACH (port, node, &f->dp->ports) {
> + CMAP_FOR_EACH (port, node, &cursor, &f->dp->ports) {
> if (netdev_is_pmd(port->netdev)) {
> int i;
>
> @@ -1862,7 +1858,6 @@ pmd_load_queues(struct pmd_thread *f,
> }
> }
>
> - ovs_rwlock_unlock(&dp->port_rwlock);
> *ppoll_list = poll_list;
> return poll_cnt;
> }
(snip)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
