On 6/20/2019 3:31 PM, David Marchand wrote:
On Wed, Jun 19, 2019 at 3:40 PM Ian Stokes <ian.sto...@intel.com <mailto:ian.sto...@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.march...@redhat.com <mailto:david.march...@redhat.com>> > Acked-by: Eelco Chaudron <echau...@redhat.com <mailto:echau...@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().
Ah, ok, now that makes a little more sense. From what i've seen, the rest of the series look ok, I just want to run a few more tests with this scenario in mind.
> + 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" ?
Perfect. Thanks Ian
-- David Marchand
_______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
