On Wed, Sep 04, 2013 at 03:47:26PM -0700, Ethan Jackson wrote: > > I wonder whether it makes sense to speak of 'cr' as being "guarded" by > > anything. Most of the data that its users really look at, notably > > 'priority' and 'match', is immutable. It is inserted into a > > classifier at construction time, and removed at destruction time, and > > other changes to the classifier can change 'hmap_node' and 'list', but > > few of the users really care: > > Good point I changed it. > > > Marking 'flow_cookie' as OVS_GUARDED, as I imagine it should be, makes > > tons of new Clang warnings pop up: > > As discussed off-list, you're right that ideally it should be, but > it's going to be a bit of a pain to get the thread safety to play > nicely with us here. In my judgement it was cleaner just to skip it. > Since it's pretty much impossible for a thread to get access to a rule > without taking it's rdlock, I think we're good. > > I fixed up that comment so it reads as below: > > /* The rwlock is used to protect those elements in struct rule which are > * accessed by multiple threads. While maintaining a pointer to struct > * rule, threads are required to hold a readlock. The main ofproto code > is > * guaranteed not to evict the rule, or change any of the elements > "Guarded > * by rwlock" without holding the writelock. > * > * A rule will not be evicted unless both its own and its classifier's > * write locks are held. Therefore, while holding a classifier readlock, > * one can be assured that write locked rules are safe to reference. */ > > I can resend the patch if you'd like.
No need, I wanted to make sure that you had considered these issues but you reassured me. Acked-by: Ben Pfaff <b...@nicira.com> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
