[ovs-dev] [rwlock 1/5] ofproto-dpif-monitor: Change global rwlock into mutex.

[Alex Wang]

Looks good to me, thx for the refine.


On Thu, Jan 16, 2014 at 10:07 AM, Ben Pfaff <blp at nicira.com> wrote:

> Nothing ever took monitor_rwlock's read lock, so it might as well be a
> mutex.
>
> Signed-off-by: Ben Pfaff <blp at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140116/5ef4a3bd/attachment-0003.html>