Looks good to me, thx for the refine.
On Thu, Jan 16, 2014 at 10:07 AM, Ben Pfaff <b...@nicira.com> wrote:
> Nothing ever took monitor_rwlock's read lock, so it might as well be a
> mutex.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ofproto/ofproto-dpif-monitor.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-monitor.c
> b/ofproto/ofproto-dpif-monitor.c
> index 33115f3..b521735 100644
> --- a/ofproto/ofproto-dpif-monitor.c
> +++ b/ofproto/ofproto-dpif-monitor.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -64,25 +64,25 @@ static pthread_t monitor_tid;
> static bool monitor_running;
>
> static struct latch monitor_exit_latch;
> -static struct ovs_rwlock monitor_rwlock = OVS_RWLOCK_INITIALIZER;
> +static struct ovs_mutex monitor_mutex = OVS_MUTEX_INITIALIZER;
>
> static void *monitor_main(void *);
> static void monitor_run(void);
>
> static void mport_register(const struct ofport_dpif *, struct bfd *,
> struct cfm *, uint8_t[ETH_ADDR_LEN])
> - OVS_REQ_WRLOCK(monitor_rwlock);
> + OVS_REQUIRES(monitor_mutex);
> static void mport_unregister(const struct ofport_dpif *)
> - OVS_REQ_WRLOCK(monitor_rwlock);
> + OVS_REQUIRES(monitor_mutex);
> static void mport_update(struct mport *, struct bfd *, struct cfm *,
> - uint8_t[ETH_ADDR_LEN])
> OVS_REQ_WRLOCK(monitor_rwlock);
> + uint8_t[ETH_ADDR_LEN])
> OVS_REQUIRES(monitor_mutex);
> static struct mport *mport_find(const struct ofport_dpif *)
> - OVS_REQ_WRLOCK(monitor_rwlock);
> + OVS_REQUIRES(monitor_mutex);
>
> /* Tries finding and returning the 'mport' from the monitor_hmap.
> * If there is no such 'mport', returns NULL. */
> static struct mport *
> -mport_find(const struct ofport_dpif *ofport)
> OVS_REQ_WRLOCK(monitor_rwlock)
> +mport_find(const struct ofport_dpif *ofport) OVS_REQUIRES(monitor_mutex)
> {
> struct mport *node;
>
> @@ -100,7 +100,7 @@ mport_find(const struct ofport_dpif *ofport)
> OVS_REQ_WRLOCK(monitor_rwlock)
> static void
> mport_register(const struct ofport_dpif *ofport, struct bfd *bfd,
> struct cfm *cfm, uint8_t *hw_addr)
> - OVS_REQ_WRLOCK(monitor_rwlock)
> + OVS_REQUIRES(monitor_mutex)
> {
> struct mport *mport = mport_find(ofport);
>
> @@ -116,7 +116,7 @@ mport_register(const struct ofport_dpif *ofport,
> struct bfd *bfd,
> /* Removes mport from monitor_hmap and monitor_heap and frees it. */
> static void
> mport_unregister(const struct ofport_dpif *ofport)
> - OVS_REQ_WRLOCK(monitor_rwlock)
> + OVS_REQUIRES(monitor_mutex)
> {
> struct mport *mport = mport_find(ofport);
>
> @@ -131,7 +131,7 @@ mport_unregister(const struct ofport_dpif *ofport)
> /* Updates the fields of an existing mport struct. */
> static void
> mport_update(struct mport *mport, struct bfd *bfd, struct cfm *cfm,
> - uint8_t hw_addr[ETH_ADDR_LEN]) OVS_REQ_WRLOCK(monitor_rwlock)
> + uint8_t hw_addr[ETH_ADDR_LEN]) OVS_REQUIRES(monitor_mutex)
> {
> ovs_assert(mport);
>
> @@ -184,7 +184,7 @@ monitor_run(void)
> struct ofpbuf packet;
>
> ofpbuf_use_stub(&packet, stub, sizeof stub);
> - ovs_rwlock_wrlock(&monitor_rwlock);
> + ovs_mutex_lock(&monitor_mutex);
> prio_now = MSEC_TO_PRIO(time_msec());
> /* Peeks the top of heap and checks if we should run this mport. */
> while (!heap_is_empty(&monitor_heap)
> @@ -226,7 +226,7 @@ monitor_run(void)
> next_mport_wakeup =
> PRIO_TO_MSEC(heap_max(&monitor_heap)->priority);
> poll_timer_wait_until(MIN(next_timeout, next_mport_wakeup));
> }
> - ovs_rwlock_unlock(&monitor_rwlock);
> + ovs_mutex_unlock(&monitor_mutex);
> ofpbuf_uninit(&packet);
> }
>
> @@ -240,13 +240,13 @@ ofproto_dpif_monitor_port_update(const struct
> ofport_dpif *ofport,
> struct bfd *bfd, struct cfm *cfm,
> uint8_t hw_addr[ETH_ADDR_LEN])
> {
> - ovs_rwlock_wrlock(&monitor_rwlock);
> + ovs_mutex_lock(&monitor_mutex);
> if (!cfm && !bfd) {
> mport_unregister(ofport);
> } else {
> mport_register(ofport, bfd, cfm, hw_addr);
> }
> - ovs_rwlock_unlock(&monitor_rwlock);
> + ovs_mutex_unlock(&monitor_mutex);
>
> /* If the monitor thread is not running and the hmap
> * is not empty, starts it. If it is and the hmap is empty,
> @@ -269,14 +269,14 @@ ofproto_dpif_monitor_port_update(const struct
> ofport_dpif *ofport,
> void
> ofproto_dpif_monitor_port_send_soon_safe(const struct ofport_dpif *ofport)
> {
> - ovs_rwlock_wrlock(&monitor_rwlock);
> + ovs_mutex_lock(&monitor_mutex);
> ofproto_dpif_monitor_port_send_soon(ofport);
> - ovs_rwlock_unlock(&monitor_rwlock);
> + ovs_mutex_unlock(&monitor_mutex);
> }
>
> void
> ofproto_dpif_monitor_port_send_soon(const struct ofport_dpif *ofport)
> - OVS_REQ_WRLOCK(monitor_rwlock)
> + OVS_REQUIRES(monitor_mutex)
> {
> struct mport *mport;
>
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
