Re: [ovs-dev] [PATCH 4/6] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

> On Apr 21, 2017, at 8:59 AM, Jarno Rajahalme  > wrote:
> 
>> 
>> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao > > 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 > >
>> ---
>> 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 4/6] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

> On Apr 21, 2017, at 8:59 AM, Jarno Rajahalme  > wrote:
> 
>> 
>> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao > > 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 > >
>> ---
>> 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 4/6] rstp: Init a recursive mutex for rstp.

[Jarno Rajahalme]

> On Mar 31, 2017, at 8:11 PM, nickcooper-zhangtonghao  
> 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?

> 
> Signed-off-by: nickcooper-zhangtonghao 
> ---
> 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.

> 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

> #define RSTP_MAX_PORTS 4095
> 
> struct dp_packet;
> -- 
> 1.8.3.1
> 
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 4/6] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

This patch will be used for next patch.

Signed-off-by: nickcooper-zhangtonghao 
---
 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 = 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;
-
 #define RSTP_MAX_PORTS 4095
 
 struct dp_packet;
-- 
1.8.3.1



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev