[ovs-dev] [PATCH] lib/dpif-netdev: Clean-up pmd thread signaling.

Pravin Shelar pshelar at nicira.com
Fri Aug 15 23:15:52 UTC 2014


On Fri, Aug 15, 2014 at 3:17 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
> It could be possible that the thread misses a signal when it reads the
> change_seq again after reload.  Also, the counter has no dependent
> data, so the memory model for the atomic read can be relaxed.
>
> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
Good catch.
Acked-by: Pravin B Shelar <pshelar at nicira.com>

> ---
>  lib/dpif-netdev.c |   27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index a458f24..7401293 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -297,6 +297,8 @@ struct pmd_thread {
>      atomic_uint change_seq;
>  };
>
> +#define PMD_INITIAL_SEQ 1
> +
>  /* Interface to netdev-based datapath. */
>  struct dpif_netdev {
>      struct dpif dpif;
> @@ -613,10 +615,10 @@ dp_netdev_reload_pmd_threads(struct dp_netdev *dp)
>
>      for (i = 0; i < dp->n_pmd_threads; i++) {
>          struct pmd_thread *f = &dp->pmd_threads[i];
> -        int id;
> +        int old_seq;
>
> -        atomic_add(&f->change_seq, 1, &id);
> -   }
> +        atomic_add_explicit(&f->change_seq, 1, &old_seq, memory_order_relaxed);
> +    }
>  }
>
>  static uint32_t
> @@ -1701,7 +1703,7 @@ pmd_thread_main(void *f_)
>      struct dp_netdev *dp = f->dp;
>      unsigned int lc = 0;
>      struct rxq_poll *poll_list;
> -    unsigned int port_seq;
> +    unsigned int port_seq = PMD_INITIAL_SEQ;
>      int poll_cnt;
>      int i;
>
> @@ -1711,10 +1713,8 @@ pmd_thread_main(void *f_)
>      pmd_thread_setaffinity_cpu(f->id);
>  reload:
>      poll_cnt = pmd_load_queues(f, &poll_list, poll_cnt);
> -    atomic_read(&f->change_seq, &port_seq);
>
>      for (;;) {
> -        unsigned int c_port_seq;
>          int i;
>
>          for (i = 0; i < poll_cnt; i++) {
> @@ -1722,14 +1722,15 @@ reload:
>          }
>
>          if (lc++ > 1024) {
> -            ovsrcu_quiesce();
> +            unsigned int seq;
>
> -            /* XXX: need completely userspace based signaling method.
> -             * to keep this thread entirely in userspace.
> -             * For now using atomic counter. */
>              lc = 0;
> -            atomic_read_explicit(&f->change_seq, &c_port_seq, memory_order_consume);
> -            if (c_port_seq != port_seq) {
> +
> +            ovsrcu_quiesce();
> +
> +            atomic_read_explicit(&f->change_seq, &seq, memory_order_relaxed);
> +            if (seq != port_seq) {
> +                port_seq = seq;
>                  break;
>              }
>          }
> @@ -1806,7 +1807,7 @@ dp_netdev_set_pmd_threads(struct dp_netdev *dp, int n)
>
>          f->dp = dp;
>          f->id = i;
> -        atomic_store(&f->change_seq, 1);
> +        atomic_init(&f->change_seq, PMD_INITIAL_SEQ);
>
>          /* Each thread will distribute all devices rx-queues among
>           * themselves. */
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list