> On Apr 21, 2017, at 8:59 AM, Jarno Rajahalme <[email protected] 
> <mailto:[email protected]>> wrote:
> 
>> 
>> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>> This patch will be used for next patch.
> 
> I don’t see exactly what in the following patch(es) need this. Could you 
> elaborate?

The next patch 6/6 will call some functions which use the mutex and the 
‘rstp/show’ use it agin. 
we should init it as a recursive mutex. 

> 
>> 
>> Signed-off-by: nickcooper-zhangtonghao <[email protected] 
>> <mailto:[email protected]>>
>> ---
>> lib/rstp.c | 15 ++++++++++++---
>> lib/rstp.h |  6 ------
>> 2 files changed, 12 insertions(+), 9 deletions(-)
>> 
>> diff --git a/lib/rstp.c b/lib/rstp.c
>> index 907a907..6f1c1e3 100644
>> --- a/lib/rstp.c
>> +++ b/lib/rstp.c
>> @@ -50,7 +50,7 @@
>> 
>> VLOG_DEFINE_THIS_MODULE(rstp);
>> 
>> -struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
>> +static struct ovs_mutex rstp_mutex;
>> 
>> static struct ovs_list all_rstps__ = OVS_LIST_INITIALIZER(&all_rstps__);
>> static struct ovs_list *const all_rstps OVS_GUARDED_BY(rstp_mutex) = 
>> &all_rstps__;
>> @@ -239,8 +239,15 @@ void
>> rstp_init(void)
>>    OVS_EXCLUDED(rstp_mutex)
>> {
>> -    unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
>> -                             NULL);
>> +    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>> +
>> +    if (ovsthread_once_start(&once)) {
>> +        ovs_mutex_init_recursive(&rstp_mutex);
>> +
>> +        unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
>> rstp_unixctl_tcn,
>> +                                 NULL);
>> +        ovsthread_once_done(&once);
>> +    }
>> }
>> 
>> /* Creates and returns a new RSTP instance that initially has no ports. */
>> @@ -255,6 +262,8 @@ rstp_create(const char *name, rstp_identifier 
>> bridge_address,
>> 
>>    VLOG_DBG("Creating RSTP instance");
>> 
>> +    rstp_init();
>> +
> 
> rstp_init() is already called earlier from the bridge_init(), so I see little 
> point calling it from here. Not having multiple call sites would also remove 
> the need for most of the changes above.

Yes, but some rstp testes, which run with ovstest, will not run rstp_init in 
the bridge_init.

> 
>>    rstp = xzalloc(sizeof *rstp);
>>    rstp->name = xstrdup(name);
>> 
>> diff --git a/lib/rstp.h b/lib/rstp.h
>> index 4942d59..78e07fb 100644
>> --- a/lib/rstp.h
>> +++ b/lib/rstp.h
>> @@ -36,12 +36,6 @@
>> #include "compiler.h"
>> #include "util.h"
>> 
>> -/* Thread Safety: Callers passing in RSTP and RSTP port object
>> - * pointers must hold a reference to the passed object to ensure that
>> - * the object does not become stale while it is being accessed. */
>> -
>> -extern struct ovs_mutex rstp_mutex;
>> -
> 
> This change, if needed, should be in a separate patch with it’s own commit 
> message.
> 
>  Jarno

Yes, If other patches will be ok, I will put it to a separate patch.




_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to