[ovs-dev] [PATCH] lib/dpif-netdev: Clean-up pmd thread signaling.

Jarno Rajahalme jrajahalme at nicira.com
Fri Aug 15 23:41:44 UTC 2014


Pushed,

  Jarno

On Aug 15, 2014, at 4:15 PM, Pravin Shelar <pshelar at nicira.com> wrote:

> On Fri, Aug 15, 2014 at 3:17 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>> It could be possible that the thread misses a signal when it reads the
>> change_seq again after reload.  Also, the counter has no dependent
>> data, so the memory model for the atomic read can be relaxed.
>> 
>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> Good catch.
> Acked-by: Pravin B Shelar <pshelar at nicira.com>
> 
>> ---
>> lib/dpif-netdev.c |   27 ++++++++++++++-------------
>> 1 file changed, 14 insertions(+), 13 deletions(-)
>> 
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index a458f24..7401293 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -297,6 +297,8 @@ struct pmd_thread {
>>     atomic_uint change_seq;
>> };
>> 
>> +#define PMD_INITIAL_SEQ 1
>> +
>> /* Interface to netdev-based datapath. */
>> struct dpif_netdev {
>>     struct dpif dpif;
>> @@ -613,10 +615,10 @@ dp_netdev_reload_pmd_threads(struct dp_netdev *dp)
>> 
>>     for (i = 0; i < dp->n_pmd_threads; i++) {
>>         struct pmd_thread *f = &dp->pmd_threads[i];
>> -        int id;
>> +        int old_seq;
>> 
>> -        atomic_add(&f->change_seq, 1, &id);
>> -   }
>> +        atomic_add_explicit(&f->change_seq, 1, &old_seq, memory_order_relaxed);
>> +    }
>> }
>> 
>> static uint32_t
>> @@ -1701,7 +1703,7 @@ pmd_thread_main(void *f_)
>>     struct dp_netdev *dp = f->dp;
>>     unsigned int lc = 0;
>>     struct rxq_poll *poll_list;
>> -    unsigned int port_seq;
>> +    unsigned int port_seq = PMD_INITIAL_SEQ;
>>     int poll_cnt;
>>     int i;
>> 
>> @@ -1711,10 +1713,8 @@ pmd_thread_main(void *f_)
>>     pmd_thread_setaffinity_cpu(f->id);
>> reload:
>>     poll_cnt = pmd_load_queues(f, &poll_list, poll_cnt);
>> -    atomic_read(&f->change_seq, &port_seq);
>> 
>>     for (;;) {
>> -        unsigned int c_port_seq;
>>         int i;
>> 
>>         for (i = 0; i < poll_cnt; i++) {
>> @@ -1722,14 +1722,15 @@ reload:
>>         }
>> 
>>         if (lc++ > 1024) {
>> -            ovsrcu_quiesce();
>> +            unsigned int seq;
>> 
>> -            /* XXX: need completely userspace based signaling method.
>> -             * to keep this thread entirely in userspace.
>> -             * For now using atomic counter. */
>>             lc = 0;
>> -            atomic_read_explicit(&f->change_seq, &c_port_seq, memory_order_consume);
>> -            if (c_port_seq != port_seq) {
>> +
>> +            ovsrcu_quiesce();
>> +
>> +            atomic_read_explicit(&f->change_seq, &seq, memory_order_relaxed);
>> +            if (seq != port_seq) {
>> +                port_seq = seq;
>>                 break;
>>             }
>>         }
>> @@ -1806,7 +1807,7 @@ dp_netdev_set_pmd_threads(struct dp_netdev *dp, int n)
>> 
>>         f->dp = dp;
>>         f->id = i;
>> -        atomic_store(&f->change_seq, 1);
>> +        atomic_init(&f->change_seq, PMD_INITIAL_SEQ);
>> 
>>         /* Each thread will distribute all devices rx-queues among
>>          * themselves. */
>> --
>> 1.7.10.4
>> 
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list