PMD threads can't wait on 'dp->port_mutex' because of possible deadlock with main thread waiting in cond_wait().
This patch replaces ovs_mutex with fat-rwlock to allow PMD threads using of dp->ports. It is required for future XPS implementation. Signed-off-by: Ilya Maximets <[email protected]> --- lib/dpif-netdev.c | 103 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 58 insertions(+), 45 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index e0107b7..3fb1942 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -183,7 +183,7 @@ static bool dpcls_lookup(const struct dpcls *cls, * Acquisition order is, from outermost to innermost: * * dp_netdev_mutex (global) - * port_mutex + * port_rwlock * non_pmd_mutex */ struct dp_netdev { @@ -196,8 +196,8 @@ struct dp_netdev { /* Ports. * * Any lookup into 'ports' or any access to the dp_netdev_ports found - * through 'ports' requires taking 'port_mutex'. */ - struct ovs_mutex port_mutex; + * through 'ports' requires taking 'port_rwlock'. */ + struct fat_rwlock port_rwlock; struct hmap ports; struct seq *port_seq; /* Incremented whenever a port changes. */ @@ -232,7 +232,7 @@ struct dp_netdev { static struct dp_netdev_port *dp_netdev_lookup_port(const struct dp_netdev *dp, odp_port_t) - OVS_REQUIRES(dp->port_mutex); + OVS_REQ_RDLOCK(dp->port_rwlock); enum dp_stat_type { DP_STAT_EXACT_HIT, /* Packets that had an exact match (emc). */ @@ -481,17 +481,17 @@ struct dpif_netdev { static int get_port_by_number(struct dp_netdev *dp, odp_port_t port_no, struct dp_netdev_port **portp) - OVS_REQUIRES(dp->port_mutex); + OVS_REQ_RDLOCK(dp->port_rwlock); static int get_port_by_name(struct dp_netdev *dp, const char *devname, struct dp_netdev_port **portp) - OVS_REQUIRES(dp->port_mutex); + OVS_REQ_RDLOCK(dp->port_rwlock); static void dp_netdev_free(struct dp_netdev *) OVS_REQUIRES(dp_netdev_mutex); static int do_add_port(struct dp_netdev *dp, const char *devname, const char *type, odp_port_t port_no) - OVS_REQUIRES(dp->port_mutex); + OVS_REQ_RDLOCK(dp->port_rwlock); static void do_del_port(struct dp_netdev *dp, struct dp_netdev_port *) - OVS_REQUIRES(dp->port_mutex); + OVS_REQ_RDLOCK(dp->port_rwlock); static int dpif_netdev_open(const struct dpif_class *, const char *name, bool create, struct dpif **); static void dp_netdev_execute_actions(struct dp_netdev_pmd_thread *pmd, @@ -511,7 +511,7 @@ static void dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, int numa_id); static void dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread *pmd); static void dp_netdev_set_nonpmd(struct dp_netdev *dp) - OVS_REQUIRES(dp->port_mutex); + OVS_REQ_RDLOCK(dp->port_rwlock); static struct dp_netdev_pmd_thread *dp_netdev_get_pmd(struct dp_netdev *dp, unsigned core_id); @@ -520,7 +520,7 @@ dp_netdev_pmd_get_next(struct dp_netdev *dp, struct cmap_position *pos); static void dp_netdev_destroy_all_pmds(struct dp_netdev *dp); static void dp_netdev_del_pmds_on_numa(struct dp_netdev *dp, int numa_id); static void dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id) - OVS_REQUIRES(dp->port_mutex); + OVS_REQ_RDLOCK(dp->port_rwlock); static void dp_netdev_pmd_clear_ports(struct dp_netdev_pmd_thread *pmd); static void dp_netdev_del_port_from_all_pmds(struct dp_netdev *dp, struct dp_netdev_port *port); @@ -534,7 +534,7 @@ static void dp_netdev_add_rxq_to_pmd(struct dp_netdev_pmd_thread *pmd, static struct dp_netdev_pmd_thread * dp_netdev_less_loaded_pmd_on_numa(struct dp_netdev *dp, int numa_id); static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp) - OVS_REQUIRES(dp->port_mutex); + OVS_REQ_RDLOCK(dp->port_rwlock); static bool dp_netdev_pmd_try_ref(struct dp_netdev_pmd_thread *pmd); static void dp_netdev_pmd_unref(struct dp_netdev_pmd_thread *pmd); static void dp_netdev_pmd_flow_flush(struct dp_netdev_pmd_thread *pmd); @@ -870,7 +870,7 @@ create_dpif_netdev(struct dp_netdev *dp) * Return ODPP_NONE on failure. */ static odp_port_t choose_port(struct dp_netdev *dp, const char *name) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { uint32_t port_no; @@ -924,7 +924,7 @@ create_dp_netdev(const char *name, const struct dpif_class *class, ovs_refcount_init(&dp->ref_cnt); atomic_flag_clear(&dp->destroyed); - ovs_mutex_init(&dp->port_mutex); + fat_rwlock_init(&dp->port_rwlock); hmap_init(&dp->ports); dp->port_seq = seq_create(); fat_rwlock_init(&dp->upcall_rwlock); @@ -938,11 +938,11 @@ create_dp_netdev(const char *name, const struct dpif_class *class, ovs_mutex_init_recursive(&dp->non_pmd_mutex); ovsthread_key_create(&dp->per_pmd_key, NULL); - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); dp_netdev_set_nonpmd(dp); error = do_add_port(dp, name, "internal", ODPP_LOCAL); - ovs_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); if (error) { dp_netdev_free(dp); return error; @@ -1004,16 +1004,16 @@ dp_netdev_free(struct dp_netdev *dp) ovs_mutex_destroy(&dp->non_pmd_mutex); ovsthread_key_delete(dp->per_pmd_key); - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); HMAP_FOR_EACH_SAFE (port, next, node, &dp->ports) { do_del_port(dp, port); } - ovs_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); cmap_destroy(&dp->poll_threads); seq_destroy(dp->port_seq); hmap_destroy(&dp->ports); - ovs_mutex_destroy(&dp->port_mutex); + fat_rwlock_destroy(&dp->port_rwlock); /* Upcalls must be disabled at this point */ dp_netdev_destroy_upcall_lock(dp); @@ -1223,7 +1223,7 @@ out: static int do_add_port(struct dp_netdev *dp, const char *devname, const char *type, odp_port_t port_no) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { struct dp_netdev_port *port; int error; @@ -1248,7 +1248,9 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type, dp_netdev_add_port_to_pmds(dp, port); + fat_rwlock_upgrade(&dp->port_rwlock); hmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); + fat_rwlock_downgrade(&dp->port_rwlock); seq_change(dp->port_seq); return 0; @@ -1264,7 +1266,7 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev, odp_port_t port_no; int error; - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); dpif_port = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf); if (*port_nop != ODPP_NONE) { port_no = *port_nop; @@ -1277,7 +1279,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_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); return error; } @@ -1288,7 +1290,7 @@ dpif_netdev_port_del(struct dpif *dpif, odp_port_t port_no) struct dp_netdev *dp = get_dp_netdev(dpif); int error; - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); if (port_no == ODPP_LOCAL) { error = EINVAL; } else { @@ -1299,7 +1301,7 @@ dpif_netdev_port_del(struct dpif *dpif, odp_port_t port_no) do_del_port(dp, port); } } - ovs_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); return error; } @@ -1312,7 +1314,7 @@ is_valid_port_number(odp_port_t port_no) static struct dp_netdev_port * dp_netdev_lookup_port(const struct dp_netdev *dp, odp_port_t port_no) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { struct dp_netdev_port *port; @@ -1327,7 +1329,7 @@ dp_netdev_lookup_port(const struct dp_netdev *dp, odp_port_t port_no) static int get_port_by_number(struct dp_netdev *dp, odp_port_t port_no, struct dp_netdev_port **portp) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { if (!is_valid_port_number(port_no)) { *portp = NULL; @@ -1360,7 +1362,7 @@ port_destroy(struct dp_netdev_port *port) static int get_port_by_name(struct dp_netdev *dp, const char *devname, struct dp_netdev_port **portp) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { struct dp_netdev_port *port; @@ -1399,7 +1401,7 @@ get_n_pmd_threads_on_numa(struct dp_netdev *dp, int numa_id) * is on numa node 'numa_id'. */ static bool has_pmd_port_for_numa(struct dp_netdev *dp, int numa_id) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { struct dp_netdev_port *port; @@ -1416,9 +1418,11 @@ has_pmd_port_for_numa(struct dp_netdev *dp, int numa_id) static void do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { + fat_rwlock_upgrade(&dp->port_rwlock); hmap_remove(&dp->ports, &port->node); + fat_rwlock_downgrade(&dp->port_rwlock); seq_change(dp->port_seq); dp_netdev_del_port_from_all_pmds(dp, port); @@ -1455,12 +1459,12 @@ dpif_netdev_port_query_by_number(const struct dpif *dpif, odp_port_t port_no, struct dp_netdev_port *port; int error; - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); error = get_port_by_number(dp, port_no, &port); if (!error && dpif_port) { answer_port_query(port, dpif_port); } - ovs_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); return error; } @@ -1473,12 +1477,12 @@ dpif_netdev_port_query_by_name(const struct dpif *dpif, const char *devname, struct dp_netdev_port *port; int error; - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); error = get_port_by_name(dp, devname, &port); if (!error && dpif_port) { answer_port_query(port, dpif_port); } - ovs_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); return error; } @@ -1563,7 +1567,7 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_, struct hmap_node *node; int retval; - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); node = hmap_at_position(&dp->ports, &state->position); if (node) { struct dp_netdev_port *port; @@ -1580,7 +1584,7 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_, } else { retval = EOF; } - ovs_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); return retval; } @@ -2673,7 +2677,7 @@ port_reconfigure(struct dp_netdev_port *port) static void reconfigure_pmd_threads(struct dp_netdev *dp) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { struct dp_netdev_port *port, *next; @@ -2684,7 +2688,9 @@ reconfigure_pmd_threads(struct dp_netdev *dp) err = port_reconfigure(port); if (err) { + fat_rwlock_upgrade(&dp->port_rwlock); hmap_remove(&dp->ports, &port->node); + fat_rwlock_downgrade(&dp->port_rwlock); seq_change(dp->port_seq); port_destroy(port); } @@ -2703,7 +2709,7 @@ reconfigure_pmd_threads(struct dp_netdev *dp) /* Returns true if one of the netdevs in 'dp' requires a reconfiguration */ static bool ports_require_restart(const struct dp_netdev *dp) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { struct dp_netdev_port *port; @@ -2726,7 +2732,7 @@ dpif_netdev_run(struct dpif *dpif) NON_PMD_CORE_ID); uint64_t new_tnl_seq; - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); ovs_mutex_lock(&dp->non_pmd_mutex); HMAP_FOR_EACH (port, node, &dp->ports) { if (!netdev_is_pmd(port->netdev)) { @@ -2745,7 +2751,7 @@ dpif_netdev_run(struct dpif *dpif) || ports_require_restart(dp)) { reconfigure_pmd_threads(dp); } - ovs_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); tnl_neigh_cache_run(); tnl_port_map_run(); @@ -2765,7 +2771,7 @@ dpif_netdev_wait(struct dpif *dpif) struct dp_netdev *dp = get_dp_netdev(dpif); ovs_mutex_lock(&dp_netdev_mutex); - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); HMAP_FOR_EACH (port, node, &dp->ports) { netdev_wait_reconf_required(port->netdev); if (!netdev_is_pmd(port->netdev)) { @@ -2776,7 +2782,7 @@ dpif_netdev_wait(struct dpif *dpif) } } } - ovs_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); ovs_mutex_unlock(&dp_netdev_mutex); seq_wait(tnl_conf_seq, dp->last_tnl_conf_seq); } @@ -2961,7 +2967,7 @@ dp_netdev_get_pmd(struct dp_netdev *dp, unsigned core_id) /* Sets the 'struct dp_netdev_pmd_thread' for non-pmd threads. */ static void dp_netdev_set_nonpmd(struct dp_netdev *dp) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { struct dp_netdev_pmd_thread *non_pmd; struct dp_netdev_port *port; @@ -3392,7 +3398,7 @@ dp_netdev_add_port_to_pmds(struct dp_netdev *dp, struct dp_netdev_port *port) * The function takes care of filling the threads tx port cache. */ static void dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { int n_pmds; @@ -3442,7 +3448,7 @@ dp_netdev_set_pmds_on_numa(struct dp_netdev *dp, int numa_id) * new configuration. */ static void dp_netdev_reset_pmd_threads(struct dp_netdev *dp) - OVS_REQUIRES(dp->port_mutex) + OVS_REQ_RDLOCK(dp->port_rwlock) { struct hmapx to_reload = HMAPX_INITIALIZER(&to_reload); struct dp_netdev_pmd_thread *pmd; @@ -4282,7 +4288,7 @@ dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED, ovs_refcount_ref(&dp->ref_cnt); ovs_mutex_unlock(&dp_netdev_mutex); - ovs_mutex_lock(&dp->port_mutex); + fat_rwlock_rdlock(&dp->port_rwlock); if (get_port_by_name(dp, argv[2], &port)) { unixctl_command_reply_error(conn, "unknown port"); goto exit; @@ -4299,19 +4305,26 @@ dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED, } /* Remove port. */ + fat_rwlock_upgrade(&dp->port_rwlock); hmap_remove(&dp->ports, &port->node); + fat_rwlock_downgrade(&dp->port_rwlock); + dp_netdev_del_port_from_all_pmds(dp, port); /* Reinsert with new port number. */ port->port_no = port_no; + + fat_rwlock_upgrade(&dp->port_rwlock); hmap_insert(&dp->ports, &port->node, hash_port_no(port_no)); + fat_rwlock_downgrade(&dp->port_rwlock); + dp_netdev_add_port_to_pmds(dp, port); seq_change(dp->port_seq); unixctl_command_reply(conn, NULL); exit: - ovs_mutex_unlock(&dp->port_mutex); + fat_rwlock_unlock(&dp->port_rwlock); dp_netdev_unref(dp); } -- 2.7.4 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
