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

Ben Pfaff blp at nicira.com
Wed Dec 29 18:20:36 UTC 2010


On Wed, Dec 29, 2010 at 01:19:40PM -0500, Jesse Gross wrote:
> 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.

OK, I agree now.




More information about the dev mailing list