Thanks for the reviews, I've merged and backported this. Ethan
On Wed, Sep 4, 2013 at 3:54 PM, Ben Pfaff <b...@nicira.com> wrote: > 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
