[ovs-dev] [dpif-netdev 09/15] ovs-atomic: Add atomic_destroy() and use everywhere it is needed.

Pravin Shelar pshelar at nicira.com
Wed Jan 8 17:48:45 UTC 2014


I found atomic_init without destroy for following var.
clock->slow_path
rule->ref_count

otherwise looks good.

On Fri, Dec 27, 2013 at 8:03 PM, Ben Pfaff <blp at nicira.com> wrote:
> C11 is able to require that atomics don't need to be destroyed, but some
> of the OVS implementations do.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/bfd.c                     |    1 +
>  lib/cfm.c                     |    5 +++++
>  lib/lacp.c                    |    1 +
>  lib/mac-learning.c            |    3 ++-
>  lib/netdev.c                  |    1 +
>  lib/ovs-atomic-clang.h        |    1 +
>  lib/ovs-atomic-gcc4+.h        |    1 +
>  lib/ovs-atomic-gcc4.7+.h      |    1 +
>  lib/ovs-atomic-pthreads.h     |    3 +++
>  lib/ovs-atomic.h              |   13 +++++++++++--
>  lib/stp.c                     |    1 +
>  ofproto/bond.c                |    1 +
>  ofproto/netflow.c             |    3 ++-
>  ofproto/ofproto-dpif-ipfix.c  |    3 ++-
>  ofproto/ofproto-dpif-sflow.c  |    1 +
>  ofproto/ofproto-dpif-upcall.c |    2 ++
>  ofproto/ofproto.c             |    2 ++
>  tests/test-atomic.c           |    2 ++
>  18 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/lib/bfd.c b/lib/bfd.c
> index b8574d5..ad2d753 100644
> --- a/lib/bfd.c
> +++ b/lib/bfd.c
> @@ -460,6 +460,7 @@ bfd_unref(struct bfd *bfd) OVS_EXCLUDED(mutex)
>              hmap_remove(all_bfds, &bfd->node);
>              netdev_close(bfd->netdev);
>              free(bfd->name);
> +            atomic_destroy(&bfd->ref_cnt);
>              free(bfd);
>              ovs_mutex_unlock(&mutex);
>          }
> diff --git a/lib/cfm.c b/lib/cfm.c
> index 5ee94d5..fc0ef78 100644
> --- a/lib/cfm.c
> +++ b/lib/cfm.c
> @@ -374,6 +374,11 @@ cfm_unref(struct cfm *cfm) OVS_EXCLUDED(mutex)
>      hmap_destroy(&cfm->remote_mps);
>      netdev_close(cfm->netdev);
>      free(cfm->rmps_array);
> +
> +    atomic_destroy(&cfm->extended);
> +    atomic_destroy(&cfm->check_tnl_key);
> +    atomic_destroy(&cfm->ref_cnt);
> +
>      free(cfm);
>  }
>
> diff --git a/lib/lacp.c b/lib/lacp.c
> index 9eec1f9..2c17e22 100644
> --- a/lib/lacp.c
> +++ b/lib/lacp.c
> @@ -259,6 +259,7 @@ lacp_unref(struct lacp *lacp) OVS_EXCLUDED(mutex)
>          hmap_destroy(&lacp->slaves);
>          list_remove(&lacp->node);
>          free(lacp->name);
> +        atomic_destroy(&lacp->ref_cnt);
>          free(lacp);
>          ovs_mutex_unlock(&mutex);
>      }
> diff --git a/lib/mac-learning.c b/lib/mac-learning.c
> index c9c1ae9..f540d6d 100644
> --- a/lib/mac-learning.c
> +++ b/lib/mac-learning.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -151,6 +151,7 @@ mac_learning_unref(struct mac_learning *ml)
>
>          bitmap_free(ml->flood_vlans);
>          ovs_rwlock_destroy(&ml->rwlock);
> +        atomic_destroy(&ml->ref_cnt);
>          free(ml);
>      }
>  }
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 1bcd80f..575227c 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -224,6 +224,7 @@ netdev_unregister_provider(const char *type)
>          atomic_read(&rc->ref_cnt, &ref_cnt);
>          if (!ref_cnt) {
>              hmap_remove(&netdev_classes, &rc->hmap_node);
> +            atomic_destroy(&rc->ref_cnt);
>              free(rc);
>              error = 0;
>          } else {
> diff --git a/lib/ovs-atomic-clang.h b/lib/ovs-atomic-clang.h
> index dbec956..7449428 100644
> --- a/lib/ovs-atomic-clang.h
> +++ b/lib/ovs-atomic-clang.h
> @@ -63,6 +63,7 @@ typedef _Atomic(int64_t)   atomic_int64_t;
>  #define ATOMIC_VAR_INIT(VALUE) (VALUE)
>
>  #define atomic_init(OBJECT, VALUE) __c11_atomic_init(OBJECT, VALUE)
> +#define atomic_destroy(OBJECT) ((void) (OBJECT))
>
>  /* Clang hard-codes these exact values internally but does not appear to
>   * export any names for them. */
> diff --git a/lib/ovs-atomic-gcc4+.h b/lib/ovs-atomic-gcc4+.h
> index 4476162..0bf17c6 100644
> --- a/lib/ovs-atomic-gcc4+.h
> +++ b/lib/ovs-atomic-gcc4+.h
> @@ -150,6 +150,7 @@ int64_t locked_int64_and(struct locked_int64 *, int64_t arg);
>
>  #define ATOMIC_VAR_INIT(VALUE) { .value = (VALUE) }
>  #define atomic_init(OBJECT, VALUE) ((OBJECT)->value = (VALUE), (void) 0)
> +#define atomic_destroy(OBJECT) ((void) (OBJECT))
>
>  static inline void
>  atomic_thread_fence(memory_order order)
> diff --git a/lib/ovs-atomic-gcc4.7+.h b/lib/ovs-atomic-gcc4.7+.h
> index 52e167f..56d265f 100644
> --- a/lib/ovs-atomic-gcc4.7+.h
> +++ b/lib/ovs-atomic-gcc4.7+.h
> @@ -71,6 +71,7 @@ typedef enum {
>
>  #define ATOMIC_VAR_INIT(VALUE) (VALUE)
>  #define atomic_init(OBJECT, VALUE) (*(OBJECT) = (VALUE), (void) 0)
> +#define atomic_destroy(OBJECT) ((void) (OBJECT))
>
>  #define atomic_thread_fence __atomic_thread_fence
>  #define atomic_signal_fence __atomic_signal_fence
> diff --git a/lib/ovs-atomic-pthreads.h b/lib/ovs-atomic-pthreads.h
> index 840c7a6..dc8f498 100644
> --- a/lib/ovs-atomic-pthreads.h
> +++ b/lib/ovs-atomic-pthreads.h
> @@ -85,6 +85,9 @@ typedef enum {
>      ((OBJECT)->value = (VALUE),                         \
>       pthread_mutex_init(&(OBJECT)->mutex, NULL),        \
>       (void) 0)
> +#define atomic_destroy(OBJECT)                  \
> +    (pthread_mutex_destroy(&(OBJECT)->mutex),   \
> +     (void) 0)
>
>  static inline void
>  atomic_thread_fence(memory_order order OVS_UNUSED)
> diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
> index e714114..2c07138 100644
> --- a/lib/ovs-atomic.h
> +++ b/lib/ovs-atomic.h
> @@ -84,8 +84,8 @@
>   * that.
>   *
>   *
> - * Initialization
> - * ==============
> + * Life Cycle
> + * ==========
>   *
>   * To initialize an atomic variable at its point of definition, use
>   * ATOMIC_VAR_INIT:
> @@ -98,6 +98,15 @@
>   * ...
>   *     atomic_init(&ai, 123);
>   *
> + * C11 does not hav an destruction function for atomic types, but some
> + * implementations of the OVS atomics do need them.  Thus, the following
> + * function is provided for destroying non-static atomic objects (A is any
> + * atomic type):
> + *
> + *     void atomic_destroy(A *object);
> + *
> + *         Destroys 'object'.
> + *
>   *
>   * Barriers
>   * ========
> diff --git a/lib/stp.c b/lib/stp.c
> index 8140263..818f2ca 100644
> --- a/lib/stp.c
> +++ b/lib/stp.c
> @@ -342,6 +342,7 @@ stp_unref(struct stp *stp)
>          list_remove(&stp->node);
>          ovs_mutex_unlock(&mutex);
>          free(stp->name);
> +        atomic_destroy(&stp->ref_cnt);
>          free(stp);
>      }
>  }
> diff --git a/ofproto/bond.c b/ofproto/bond.c
> index ff050f1..3b0c11c 100644
> --- a/ofproto/bond.c
> +++ b/ofproto/bond.c
> @@ -231,6 +231,7 @@ bond_unref(struct bond *bond)
>
>      free(bond->hash);
>      free(bond->name);
> +    atomic_destroy(&bond->ref_cnt);
>      free(bond);
>  }
>
> diff --git a/ofproto/netflow.c b/ofproto/netflow.c
> index f79ad6a..3aa0630 100644
> --- a/ofproto/netflow.c
> +++ b/ofproto/netflow.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -438,6 +438,7 @@ netflow_unref(struct netflow *nf)
>          atomic_sub(&netflow_count, 1, &orig);
>          collectors_destroy(nf->collectors);
>          ofpbuf_uninit(&nf->packet);
> +        atomic_destroy(&nf->ref_cnt);
>          free(nf);
>      }
>  }
> diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
> index a9cc73e..55544cc 100644
> --- a/ofproto/ofproto-dpif-ipfix.c
> +++ b/ofproto/ofproto-dpif-ipfix.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2012 Nicira, Inc.
> + * Copyright (c) 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -696,6 +696,7 @@ dpif_ipfix_unref(struct dpif_ipfix *di) OVS_EXCLUDED(mutex)
>          dpif_ipfix_clear(di);
>          dpif_ipfix_bridge_exporter_destroy(&di->bridge_exporter);
>          hmap_destroy(&di->flow_exporter_map);
> +        atomic_destroy(&di->ref_cnt);
>          free(di);
>          ovs_mutex_unlock(&mutex);
>      }
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index 158887f..d45eada 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -377,6 +377,7 @@ dpif_sflow_unref(struct dpif_sflow *ds) OVS_EXCLUDED(mutex)
>              dpif_sflow_del_port__(ds, dsp);
>          }
>          hmap_destroy(&ds->ports);
> +        atomic_destroy(&ds->ref_cnt);
>          free(ds);
>      }
>  }
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 37171be..0238d2c 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -273,6 +273,8 @@ udpif_destroy(struct udpif *udpif)
>      latch_destroy(&udpif->exit_latch);
>      seq_destroy(udpif->reval_seq);
>      seq_destroy(udpif->dump_seq);
> +    atomic_destroy(&udpif->max_idle);
> +    atomic_destroy(&udpif->flow_limit);
>      free(udpif);
>  }
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 676a6cb..028b07a 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2625,6 +2625,7 @@ rule_actions_unref(struct rule_actions *actions)
>
>          atomic_sub(&actions->ref_count, 1, &orig);
>          if (orig == 1) {
> +            atomic_destroy(&actions->ref_count);
>              free(actions->ofpacts);
>              free(actions);
>          } else {
> @@ -6665,6 +6666,7 @@ oftable_destroy(struct oftable *table)
>      oftable_disable_eviction(table);
>      classifier_destroy(&table->cls);
>      free(table->name);
> +    atomic_destroy(&table->config);
>  }
>
>  /* Changes the name of 'table' to 'name'.  If 'name' is NULL or the empty
> diff --git a/tests/test-atomic.c b/tests/test-atomic.c
> index e6df1cd..bc00df1 100644
> --- a/tests/test-atomic.c
> +++ b/tests/test-atomic.c
> @@ -59,6 +59,8 @@
>          ovs_assert(orig == 2);                          \
>          atomic_read(&x, &value);                        \
>          ovs_assert(value == 8);                         \
> +                                                        \
> +        atomic_destroy(&x);                             \
>      }
>
>  static void
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list