On Thu, Apr 7, 2022 at 4:13 AM Peng He <xnhp0...@gmail.com> 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev