[ovs-dev] [PATCH 3/3] ofproto: Rename struct rule's evict lock and use it more widely.

Ethan Jackson ethan at nicira.com
Wed Sep 4 22:47:26 UTC 2013


> 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.

Ethan



More information about the dev mailing list