[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