[ovs-dev] [PATCH v4 2/7] datapath: Clarify locking.

Jarno Rajahalme jrajahalme at nicira.com
Mon Mar 24 22:35:22 UTC 2014


On Mar 24, 2014, at 1:30 PM, Pravin Shelar <pshelar at nicira.com> wrote:

> Since you are fixing locking, can you also fix flow-stats access in
> ovs_flow_stats_get(). The access is always under ovs-mutex.
> 

You mean adding a comment to that effect on top of ovs_flow_stats_get() in datapath/flow.c?

How about the patch otherwise, you want to see a new version, or should I push this together with the new comment?

Thanks,

  Jarno


> On Mon, Mar 24, 2014 at 11:56 AM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>> Remove unnecessary locking from functions that are always called with
>> appropriate locking.
>> 
>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
>> ---
>> datapath/datapath.c |   13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>> 
>> diff --git a/datapath/datapath.c b/datapath/datapath.c
>> index 918a990..7ce3ea6 100644
>> --- a/datapath/datapath.c
>> +++ b/datapath/datapath.c
>> @@ -175,6 +175,7 @@ static struct hlist_head *vport_hash_bucket(const struct datapath *dp,
>>        return &dp->ports[port_no & (DP_VPORT_HASH_BUCKETS - 1)];
>> }
>> 
>> +/* Called with ovs_mutex or RCU read lock. */
>> struct vport *ovs_lookup_vport(const struct datapath *dp, u16 port_no)
>> {
>>        struct vport *vport;
>> @@ -653,7 +654,7 @@ static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts)
>>                + nla_total_size(acts->actions_len); /* OVS_FLOW_ATTR_ACTIONS */
>> }
>> 
>> -/* Called with ovs_mutex. */
>> +/* Called with ovs_mutex or RCU read lock. */
>> static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
>>                                  struct sk_buff *skb, u32 portid,
>>                                  u32 seq, u32 flags, u8 cmd)
>> @@ -744,6 +745,7 @@ error:
>>        return err;
>> }
>> 
>> +/* Must be called with ovs_mutex. */
>> static struct sk_buff *ovs_flow_cmd_alloc_info(struct sw_flow *flow,
>>                                               struct genl_info *info)
>> {
>> @@ -754,6 +756,7 @@ static struct sk_buff *ovs_flow_cmd_alloc_info(struct sw_flow *flow,
>>        return genlmsg_new_unicast(len, info, GFP_KERNEL);
>> }
>> 
>> +/* Must be called with ovs_mutex. */
>> static struct sk_buff *ovs_flow_cmd_build_info(struct sw_flow *flow,
>>                                               struct datapath *dp,
>>                                               struct genl_info *info,
>> @@ -1094,6 +1097,7 @@ static size_t ovs_dp_cmd_msg_size(void)
>>        return msgsize;
>> }
>> 
>> +/* Called with ovs_mutex or RCU read lock. */
>> static int ovs_dp_cmd_fill_info(struct datapath *dp, struct sk_buff *skb,
>>                                u32 portid, u32 seq, u32 flags, u8 cmd)
>> {
>> @@ -1109,9 +1113,7 @@ static int ovs_dp_cmd_fill_info(struct datapath *dp, struct sk_buff *skb,
>> 
>>        ovs_header->dp_ifindex = get_dpifindex(dp);
>> 
>> -       rcu_read_lock();
>>        err = nla_put_string(skb, OVS_DP_ATTR_NAME, ovs_dp_name(dp));
>> -       rcu_read_unlock();
>>        if (err)
>>                goto nla_put_failure;
>> 
>> @@ -1136,6 +1138,7 @@ error:
>>        return -EMSGSIZE;
>> }
>> 
>> +/* Must be called with ovs_mutex. */
>> static struct sk_buff *ovs_dp_cmd_build_info(struct datapath *dp,
>>                                             struct genl_info *info, u8 cmd)
>> {
>> @@ -1154,7 +1157,7 @@ static struct sk_buff *ovs_dp_cmd_build_info(struct datapath *dp,
>>        return skb;
>> }
>> 
>> -/* Called with ovs_mutex. */
>> +/* Called with rcu_read_lock or ovs_mutex. */
>> static struct datapath *lookup_datapath(struct net *net,
>>                                        struct ovs_header *ovs_header,
>>                                        struct nlattr *a[OVS_DP_ATTR_MAX + 1])
>> @@ -1166,10 +1169,8 @@ static struct datapath *lookup_datapath(struct net *net,
>>        else {
>>                struct vport *vport;
>> 
>> -               rcu_read_lock();
>>                vport = ovs_vport_locate(net, nla_data(a[OVS_DP_ATTR_NAME]));
>>                dp = vport && vport->port_no == OVSP_LOCAL ? vport->dp : NULL;
>> -               rcu_read_unlock();
>>        }
>>        return dp ? dp : ERR_PTR(-ENODEV);
>> }
>> --
>> 1.7.10.4
>> 
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list