[ovs-dev] [RFC 4/5] dpif-netdev: Only reload static tx qid when needed.

[Eelco Chaudron]

Patch looks good to me, however need to fix the relaxed atomic 
operations.

On 30 Apr 2019, at 14:18, David Marchand wrote:

> pmd->static_tx_qid is allocated under a mutex by the different pmd
> threads.
> Unconditionally reallocating it will make those pmd threads sleep
> when contention occurs.
> During "normal" reloads like for rebalancing queues between pmd 
> threads,
> this can make pmd threads waste time on this.
> Reallocating the tx qid is only needed when removing other pmd threads
> as it is the only situation when the qid pool can become uncontiguous.
>
> Add a flag to instruct the pmd to reload tx qid for this case which is
> Step 1 in current code.
>
> Signed-off-by: David Marchand <david.marchand at redhat.com>
> ---
>  lib/dpif-netdev.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index d67cdb5..45993a3 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -685,6 +685,7 @@ struct dp_netdev_pmd_thread {
>      uint64_t last_reload_seq;
>      atomic_bool reload;             /* Do we need to reload ports? */
>      atomic_bool wait_for_reload;    /* Can we busy wait for the next 
> reload? */
> +    atomic_bool reload_tx_qid;      /* Do we need to reload 
> static_tx_qid? */
>      atomic_bool exit;               /* For terminating the pmd 
> thread. */
>      pthread_t thread;
>      unsigned core_id;               /* CPU core id of this pmd 
> thread. */
> @@ -4717,6 +4718,7 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>                                                      pmd->core_id)) {
>              hmapx_add(&to_delete, pmd);
>          } else if (need_to_adjust_static_tx_qids) {
> +            atomic_store_relaxed(&pmd->reload_tx_qid, true);
>              pmd->need_reload = true;
>          }
>      }
> @@ -5439,6 +5441,7 @@ pmd_thread_main(void *f_)
>      unsigned int lc = 0;
>      struct polled_queue *poll_list;
>      bool wait_for_reload = false;
> +    bool reload_tx_qid;
>      bool exiting;
>      bool reload;
>      int poll_cnt;
> @@ -5453,9 +5456,9 @@ pmd_thread_main(void *f_)
>      dpdk_set_lcore_id(pmd->core_id);
>      poll_cnt = pmd_load_queues_and_ports(pmd, &poll_list);
>      dfc_cache_init(&pmd->flow_cache);
> -reload:
>      pmd_alloc_static_tx_qid(pmd);
>
> +reload:
>      atomic_count_init(&pmd->pmd_overloaded, 0);
>
>      /* List port/core affinity */
> @@ -5536,17 +5539,22 @@ reload:
>
>      poll_cnt = pmd_load_queues_and_ports(pmd, &poll_list);
>      atomic_read_relaxed(&pmd->wait_for_reload, &wait_for_reload);
> +    atomic_read_relaxed(&pmd->reload_tx_qid, &reload_tx_qid);
>      atomic_read_relaxed(&pmd->exit, &exiting);
>      /* Signal here to make sure the pmd finishes
>       * reloading the updated configuration. */
>      dp_netdev_pmd_reload_done(pmd);
>
> -    pmd_free_static_tx_qid(pmd);
> +    if (reload_tx_qid) {
> +        pmd_free_static_tx_qid(pmd);
> +        pmd_alloc_static_tx_qid(pmd);
> +    }
>
>      if (!exiting) {
>          goto reload;
>      }
>
> +    pmd_free_static_tx_qid(pmd);
>      dfc_cache_uninit(&pmd->flow_cache);
>      free(poll_list);
>      pmd_free_cached_ports(pmd);
> @@ -5871,6 +5879,7 @@ static void
>  dp_netdev_pmd_reload_done(struct dp_netdev_pmd_thread *pmd)
>  {
>      atomic_store_relaxed(&pmd->wait_for_reload, false);
> +    atomic_store_relaxed(&pmd->reload_tx_qid, false);
>      atomic_store_relaxed(&pmd->reload, false);
>      pmd->last_reload_seq = seq_read(pmd->reload_seq);
>      atomic_count_dec(&pmd->dp->reloading_pmds);
> -- 
> 1.8.3.1