[ovs-dev] [netlink 03/16] datapath: Merge "struct dp_port" into "struct vport".

Jesse Gross jesse at nicira.com
Fri Nov 5 19:01:07 UTC 2010


On Fri, Nov 5, 2010 at 11:28 AM, Ben Pfaff <blp at nicira.com> wrote:
> On Wed, Nov 03, 2010 at 04:43:08PM -0700, Ben Pfaff wrote:
>> On Thu, Sep 23, 2010 at 07:11:43PM -0700, Jesse Gross wrote:
>> > On Fri, Sep 10, 2010 at 3:55 PM, Ben Pfaff <blp at nicira.com> wrote:
>> > > diff --git a/datapath/vport.c b/datapath/vport.c
>> > > index 2ae0053..2363802 100644
>> > > @@ -47,13 +47,12 @@ static struct hlist_head *dev_table;
>> > >  * one of these locks if you don't want the vport to be deleted out from under
>> > >  * you.
>> > >  *
>> > > - * If you get a reference to a vport through a dp_port, it is protected
>> > > + * If you get a reference to a vport through a datapath, it is protected
>> > >  * by RCU and you need to hold rcu_read_lock instead when reading.
>> > >  *
>> > >  * If multiple locks are taken, the hierarchy is:
>> > >  * 1. RTNL
>> > > - * 2. DP
>> > > - * 3. vport
>> > > + * 2. vport
>> > >  */
>> > >  static DEFINE_MUTEX(vport_mutex);
>> >
>> > It would be nice if we can simply this a little bit.  vport_mutex was
>> > introduced because vports existed outside the context of a particular
>> > datapath and therefore it wasn't possible to rely on dp_mutex to
>> > protect them from disappearing.
>
> Hmm, vport_mutex is almost completely redundant with rtnl_lock now.  If
> I insert rtnl_lock/rtnl_unlock pairs into query_port(),
> vport_user_stats_get(), vport_user_ether_get(), and
> vport_user_mtu_get(), then I can drop vport_mutex entirely.
>
> This seems reasonable to me so I've added patches to that effect to the
> series.

One of the purposes of vport_mutex was actually to avoid doing exactly
this.  All of these are read only functions, which is why they don't
need RTNL.  I don't see a good justification for extending the use of
RTNL to cover our private data structures, it already covers a huge
number of things throughout the kernel.  This should really use
dp_mutex instead.




More information about the dev mailing list