Pushed,
Jarno
On Aug 15, 2014, at 4:15 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Fri, Aug 15, 2014 at 3:17 PM, Jarno Rajahalme <jrajaha...@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 <jrajaha...@nicira.com>
> Good catch.
> Acked-by: Pravin B Shelar <pshe...@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@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
