[ovs-dev] [RFC 1/5] dpif-netdev: Convert exit latch to flag.

David Marchand david.marchand at redhat.com
Tue May 14 07:38:05 UTC 2019


Hello Ilya,


On Mon, May 6, 2019 at 5:37 PM Ilya Maximets <i.maximets at samsung.com> wrote:

> On 30.04.2019 15:17, David Marchand wrote:
> > No need for a latch here since we don't have to wait.
> > A simple boolean flag is enough.
> >
> > Fixes: e4cfed38b159 ("dpif-netdev: Add poll-mode-device thread.")
> > Signed-off-by: David Marchand <david.marchand at redhat.com>
> > ---
> >  lib/dpif-netdev.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index f1422b2..30774ed 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -681,10 +681,10 @@ struct dp_netdev_pmd_thread {
> >      /* Current context of the PMD thread. */
> >      struct dp_netdev_pmd_thread_ctx ctx;
> >
> > -    struct latch exit_latch;        /* For terminating the pmd thread.
> */
> >      struct seq *reload_seq;
> >      uint64_t last_reload_seq;
> >      atomic_bool reload;             /* Do we need to reload ports? */
> > +    atomic_bool exit;               /* For terminating the pmd thread.
> */
> >      pthread_t thread;
> >      unsigned core_id;               /* CPU core id of this pmd thread.
> */
> >      int numa_id;                    /* numa node id of this pmd thread.
> */
> > @@ -5479,7 +5479,7 @@ reload:
> >      ovs_mutex_unlock(&pmd->perf_stats.stats_mutex);
> >
> >      poll_cnt = pmd_load_queues_and_ports(pmd, &poll_list);
> > -    exiting = latch_is_set(&pmd->exit_latch);
> > +    atomic_read_relaxed(&pmd->exit, &exiting);
>
> I'm afraid that relaxed memory model is not suitable here.
> You need to change memory models for both 'reload' and 'exit'
> or put ack_rel thread fence between them. Otherwise reads/writes
> could be reordered resulting in missed exit and main thread
> hang on join.
>

Indeed, I can not use relaxed memory model on both atomics.

I have been reading some articles and I am a bit puzzled :-).
I don't understand why I would need to update both atomics memory model.

On the pmd thread side:
atomic_read_explicit(&pmd->reload, &reload, memory_model_acquire);
atomic_read_relaxed(&pmd->exit, &exiting);

On the control side:
atomic_store_relaxed(&pmd->exit, true);
atomic_store_explicit(&pmd->reload, true, memory_order_release);

Would not it be enough to have those threads share the same view by
synchronising on reload ?


-- 
David Marchand


More information about the dev mailing list