On Thu, Apr 7, 2022 at 4:13 AM Peng He <[email protected]> wrote:
>
> I understand there is no need to introduce a rcu api,
> but at least put these codes into a separate function ? :-)
No strong opinion.
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index 6601f2346..402cdcdf5 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -1805,6 +1805,7 @@ destruct(struct ofproto *ofproto_, bool del)
>> struct rule_dpif *rule;
>> struct oftable *table;
>> struct ovs_list ams;
>> + struct seq *seq;
>>
>> ofproto->backer->need_revalidate = REV_RECONFIGURE;
>> xlate_txn_start();
>> @@ -1848,6 +1849,14 @@ destruct(struct ofproto *ofproto_, bool del)
>>
>> seq_destroy(ofproto->ams_seq);
>>
>> + /* Wait for all the meter rules to be destroyed. */
>> + seq = seq_create();
>> + ovsrcu_synchronize();
>> + seq_wait(seq, seq_read(seq));
>> + ovsrcu_postpone__((void (*)(void *))seq_change, seq);
>>
>> + poll_block();
>
>
> after poll block, we should check if seq is changed or not, other fds will
> wake up block too. so we need a while loop to do the seq_wait + postpone +
> check
> stuff.
>
>> + seq_destroy(seq);
>> +
If there can be other event nodes registered at this point, once you
leave poll_block() no event node is kept.
Adding to this, poll_block -> timepoll -> poll can fail (though I am
not clear it is something that is supposed to happen).
Once a first poll_block() returns, the only event that this code can
wait for is the seq changing.
There should be no need to wake up with a timeout, since there is no
useful work to do.
Am I missing something else?
In the end, that would translate to:
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6601f23464..dd1b9e5138 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1805,6 +1805,8 @@ destruct(struct ofproto *ofproto_, bool del)
struct rule_dpif *rule;
struct oftable *table;
struct ovs_list ams;
+ struct seq *seq;
+ uint64_t seqno;
ofproto->backer->need_revalidate = REV_RECONFIGURE;
xlate_txn_start();
@@ -1848,6 +1850,17 @@ destruct(struct ofproto *ofproto_, bool del)
seq_destroy(ofproto->ams_seq);
+ /* Wait for all the meter rules to be destroyed. */
+ seq = seq_create();
+ seqno = seq_read();
+ ovsrcu_synchronize();
+ ovsrcu_postpone__((void (*)(void *))seq_change, seq);
+ do {
+ seq_wait(seq, seqno);
+ poll_block();
+ } while (seqno == seq_read(seq));
+ seq_destroy(seq);
+
close_dpif_backer(ofproto->backer, del);
}
--
David Marchand
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
