[ovs-dev] [PATCH 3/5] dpif-netdev: Do not sleep when swapping queues.

[David Marchand]

On Wed, Jun 19, 2019 at 3:40 PM Ian Stokes <ian.stokes at intel.com> wrote:

> On 5/23/2019 3:23 PM, David Marchand wrote:
> > When swapping queues from a pmd thread to another (q0 polled by pmd0/q1
> > polled by pmd1 -> q1 polled by pmd0/q0 polled by pmd1), the current
> > "Step 5" puts both pmds to sleep waiting for the control thread to wake
> > them up later.
> >
> > Prefer to make them spin in such a case to avoid sleeping an
> > undeterministic amount of time.
> >
> > Signed-off-by: David Marchand <david.marchand at redhat.com>
> > Acked-by: Eelco Chaudron <echaudro at redhat.com>
> > ---
> >   lib/dpif-netdev.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
> >   1 file changed, 42 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 23cf6a6..243c1ce 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -683,6 +683,7 @@ struct dp_netdev_pmd_thread {
> >       struct seq *reload_seq;
> >       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 exit;               /* For terminating the pmd thread.
> */
> >       pthread_t thread;
> >       unsigned core_id;               /* CPU core id of this pmd thread.
> */
> > @@ -4896,6 +4897,33 @@ reconfigure_datapath(struct dp_netdev *dp)
> >           HMAP_FOR_EACH_SAFE (poll, poll_next, node, &pmd->poll_list) {
> >               if (poll->rxq->pmd != pmd) {
> >                   dp_netdev_del_rxq_from_pmd(pmd, poll);
>
> I'm a little confused by the block below.
>
> > +
> > +                /* This pmd might sleep after this step reload if it
> has no
> > +                 * rxq remaining. Can we tell it to busy wait for new
> rxq at
> > +                 * Step 6 ? */
>
> So whats the typical cases we target here, I would think this would
> occur if PMDs have been isolated and there are no non isolated queues
> available for the rxq to be assigned to?
>

It can happen in any situation when the rebalance code decides to reassign
rxqs in a way that, for a given pmd, the old polling list intersection with
the new polling list is null.
Because of this null intersection, between step 5 and step 6 reload
notifications, the pmd sleeps until it is woken on the poll_block().



> > +                if (hmap_count(&pmd->poll_list) == 0) {
> > +                    HMAP_FOR_EACH (port, node, &dp->ports) {
> > +                        int qid;
> > +
> > +                        if (!netdev_is_pmd(port->netdev)) {
> > +                            continue;
> > +                        }
> > +
> > +                        for (qid = 0; qid < port->n_rxq; qid++) {
> > +                            struct dp_netdev_rxq *q = &port->rxqs[qid];
> > +
> > +                            if (q->pmd == pmd) {
> > +
> atomic_store_relaxed(&q->pmd->wait_for_reload,
> > +                                                     true);
>
> I was a little confused here, are we marking wait_for_reload true here
> so that reload in step 6 will handle any new assignment? Does this not
> put it in the busy-wait state already here in step 5? It's just from
> reading the comment it looked like busy wait status was expected in step
> 6 rather than here.
>

Nop, we are in step 5, so yes, the comment should say "until" instead of
"at" ?


-- 
David Marchand