On 21/11/2016 06:27, "Ilya Maximets" <[email protected]> wrote:
>
>
>On 16.11.2016 03:46, Daniele Di Proietto wrote:
>> There's no reason for a pmd thread to perform its main loop if there are
>> no queues in its poll_list.
>>
>> This commit introduces a seq object on which the pmd thread can be
>> blocked, if there are no queues.
>>
>> When the main thread wants to reload a pmd threads it must now change
>> the seq object (in case it's blocked) and set 'reload' to true.
>>
>> This is useful to avoid wasting CPU cycles and is also necessary for a
>> future commit.
>>
>> Signed-off-by: Daniele Di Proietto <[email protected]>
>> ---
>> lib/dpif-netdev.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 23546b9..6c2ad29 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -485,6 +485,8 @@ struct dp_netdev_pmd_thread {
>> unsigned long long last_cycles;
>>
>> 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? */
>> pthread_t thread;
>> unsigned core_id; /* CPU core id of this pmd thread. */
>> @@ -1210,6 +1212,7 @@ dp_netdev_reload_pmd__(struct dp_netdev_pmd_thread
>> *pmd)
>> }
>>
>> ovs_mutex_lock(&pmd->cond_mutex);
>> + seq_change(pmd->reload_seq);
>> atomic_store_relaxed(&pmd->reload, true);
>> ovs_mutex_cond_wait(&pmd->cond, &pmd->cond_mutex);
>> ovs_mutex_unlock(&pmd->cond_mutex);
>> @@ -3148,6 +3151,13 @@ reload:
>> netdev_rxq_get_queue_id(poll_list[i].rx));
>> }
>>
>> + if (!poll_cnt) {
>> + while (seq_read(pmd->reload_seq) == pmd->last_reload_seq) {
>> + seq_wait(pmd->reload_seq, pmd->last_reload_seq);
>> + poll_block();
>> + }
>> + }
>> +
>
>How about putting the next polling loop under 'else'? This will help
>to avoid 1024 empty cycles inside of it. Or, maybe, we can just set
>'lc' to 1025 after the 'while' above.
I tried putting the polling loop under 'else', but it seems to have a
small negative performance impact, but I don't see any problem with
setting lc to 1025, so I did that.
>
>Another thought: I guess, we should enter a quiescent state before
>'while' loop and end it after.
poll_block() already handles quiescent states, I think that's enough for
our purpose.
>
>Also, I prefer to merge 'reload' and 'reload_seq' somehow, but I don't
>know how to do this yet.
That's a good point. We could use only reload_seq, but seq_read() takes
a mutex, so it's not ideal to call it in the fast path.
We need an object that can be checked with a simple atomic_read_relaxed()
and that can be also used to block.
This (using both 'reload' and 'reload_seq') is the simplest (in terms of
lines of code) solution I came up with, but I'm open to suggestions.
Thanks
>
>> for (;;) {
>> for (i = 0; i < poll_cnt; i++) {
>> dp_netdev_process_rxq_port(pmd, poll_list[i].port,
>> poll_list[i].rx);
>> @@ -3223,6 +3233,7 @@ dp_netdev_pmd_reload_done(struct dp_netdev_pmd_thread
>> *pmd)
>> {
>> ovs_mutex_lock(&pmd->cond_mutex);
>> atomic_store_relaxed(&pmd->reload, false);
>> + pmd->last_reload_seq = seq_read(pmd->reload_seq);
>> xpthread_cond_signal(&pmd->cond);
>> ovs_mutex_unlock(&pmd->cond_mutex);
>> }
>> @@ -3317,6 +3328,8 @@ dp_netdev_configure_pmd(struct dp_netdev_pmd_thread
>> *pmd, struct dp_netdev *dp,
>>
>> ovs_refcount_init(&pmd->ref_cnt);
>> latch_init(&pmd->exit_latch);
>> + pmd->reload_seq = seq_create();
>> + pmd->last_reload_seq = seq_read(pmd->reload_seq);
>> atomic_init(&pmd->reload, false);
>> xpthread_cond_init(&pmd->cond, NULL);
>> ovs_mutex_init(&pmd->cond_mutex);
>> @@ -3356,6 +3369,7 @@ dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread *pmd)
>> cmap_destroy(&pmd->flow_table);
>> ovs_mutex_destroy(&pmd->flow_mutex);
>> latch_destroy(&pmd->exit_latch);
>> + seq_destroy(pmd->reload_seq);
>> xpthread_cond_destroy(&pmd->cond);
>> ovs_mutex_destroy(&pmd->cond_mutex);
>> ovs_mutex_destroy(&pmd->port_mutex);
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
