[ovs-dev] [PATCH 02/10] datapath: Use get_table_protected() in additional places.

Jesse Gross jesse at nicira.com
Wed Dec 29 18:19:40 UTC 2010


On Wed, Dec 29, 2010 at 1:13 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Wed, Dec 29, 2010 at 01:10:43PM -0500, Jesse Gross wrote:
>> On Wed, Dec 29, 2010 at 12:52 PM, Ben Pfaff <blp at nicira.com> wrote:
>> > On Tue, Dec 28, 2010 at 08:50:40PM -0800, Jesse Gross wrote:
>> >> There are several places where the flow table is accessed
>> >> without any kind of RCU protection.  This is fine because dp
>> >> mutex is held so this adds checks for that condition.
>> >>
>> >> Found with sparse.
>> >>
>> >> Signed-off-by: Jesse Gross <jesse at nicira.com>
>> >
>> > The change to create_dp() looks fine to me.
>> >
>> > But as far as I can tell dp->mutex is not held in do_destroy_dp(),
>> > although dp_mutex is, and so I don't see why get_table_protected() would
>> > be OK with that.
>>
>> I believe that the annotation is correct but the existing locking is wrong.
>>
>> If we had one writer modifying the flow table it would hold dp_mutex
>> only long enough to acquire dp->mutex, it would not continue to hold
>> dp_mutex or RTNL.
>>
>> Then a second writer comes in and tries to delete the DP.  It grabs
>> RTNL and dp_mutex with no problem and deletes the DP out from under
>> the first guy.
>
> I think that the current locking is OK because the second writer has to
> grab dp_mutex before it can get dp->mutex -- see get_dp_locked().

It works if the datapath is being deleted and a second writer comes
in, for this reason.

It does not work if the second writer is one doing the deletion.  The
first writer will have already dropped dp_mutex but is still accessing
the data structures.  That's the case in the example I gave.




More information about the dev mailing list