Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
* [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1
@ 2011-09-15  5:38 Lai Jiangshan
  2011-09-15  5:38 ` [ltt-dev] [PATCH 01/10] add missing static Lai Jiangshan
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:38 UTC (permalink / raw)


2cleanups and 8bugfixes

Lai Jiangshan (10):
  add missing static
  init maxcpus before use it
  use get_cpu_call_rcu_data() for get_call_rcu_data()
  use cds_list_for_each_entry_safe()
  avoid memory leak in call_rcu_data_free()
  wake up default call_rcu thread after we move the leftover callbacks
  protect writing to per_cpu_call_rcu_data[*]
  Return -EEXIST when the old cpu call_rcu_data has not been removed
  avoid leaking crdp for failed path
  protects call_rcu_data_list when remove node

 urcu-call-rcu-impl.h |   61 +++++++++++++++++++++++++++++--------------------
 1 files changed, 36 insertions(+), 25 deletions(-)

-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 01/10] add missing static
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
@ 2011-09-15  5:38 ` Lai Jiangshan
  2011-09-15 14:54   ` Mathieu Desnoyers
  2011-09-15  5:38 ` [ltt-dev] [PATCH 02/10] init maxcpus before use it Lai Jiangshan
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:38 UTC (permalink / raw)


Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index c14cc18..700d128 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -383,7 +383,7 @@ struct call_rcu_data *create_call_rcu_data(unsigned long flags,
 
 int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
 {
-	int warned = 0;
+	static int warned = 0;
 
 	call_rcu_lock(&call_rcu_mutex);
 	if (cpu < 0 || maxcpus <= cpu) {
-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 02/10] init maxcpus before use it
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
  2011-09-15  5:38 ` [ltt-dev] [PATCH 01/10] add missing static Lai Jiangshan
@ 2011-09-15  5:38 ` Lai Jiangshan
  2011-09-15 14:59   ` Mathieu Desnoyers
  2011-09-15  5:38 ` [ltt-dev] [PATCH 03/10] use get_cpu_call_rcu_data() for get_call_rcu_data() Lai Jiangshan
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:38 UTC (permalink / raw)


Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index 700d128..e0a8fd8 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -386,6 +386,7 @@ int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
 	static int warned = 0;
 
 	call_rcu_lock(&call_rcu_mutex);
+	alloc_cpu_call_rcu_data();
 	if (cpu < 0 || maxcpus <= cpu) {
 		if (!warned) {
 			fprintf(stderr, "[error] liburcu: set CPU # out of range\n");
@@ -395,7 +396,6 @@ int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
 		errno = EINVAL;
 		return -EINVAL;
 	}
-	alloc_cpu_call_rcu_data();
 	call_rcu_unlock(&call_rcu_mutex);
 	if (per_cpu_call_rcu_data == NULL) {
 		errno = ENOMEM;
-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 03/10] use get_cpu_call_rcu_data() for get_call_rcu_data()
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
  2011-09-15  5:38 ` [ltt-dev] [PATCH 01/10] add missing static Lai Jiangshan
  2011-09-15  5:38 ` [ltt-dev] [PATCH 02/10] init maxcpus before use it Lai Jiangshan
@ 2011-09-15  5:38 ` Lai Jiangshan
  2011-09-15 15:08   ` Mathieu Desnoyers
  2011-09-15  5:38 ` [ltt-dev] [PATCH 04/10] use cds_list_for_each_entry_safe() Lai Jiangshan
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:38 UTC (permalink / raw)


Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index e0a8fd8..1fbce98 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -435,22 +435,17 @@ struct call_rcu_data *get_default_call_rcu_data(void)
  */
 struct call_rcu_data *get_call_rcu_data(void)
 {
-	int curcpu;
-	static int warned = 0;
+	struct call_rcu_data *crd;
 
 	if (thread_call_rcu_data != NULL)
 		return thread_call_rcu_data;
-	if (maxcpus <= 0)
-		return get_default_call_rcu_data();
-	curcpu = sched_getcpu();
-	if (!warned && (curcpu < 0 || maxcpus <= curcpu)) {
-		fprintf(stderr, "[error] liburcu: gcrd CPU # out of range\n");
-		warned = 1;
+
+	if (maxcpus > 0) {
+		crd = get_cpu_call_rcu_data(sched_getcpu());
+		if (crd)
+			return crd;
 	}
-	if (curcpu >= 0 && maxcpus > curcpu &&
-	    per_cpu_call_rcu_data != NULL &&
-	    per_cpu_call_rcu_data[curcpu] != NULL)
-	    	return per_cpu_call_rcu_data[curcpu];
+
 	return get_default_call_rcu_data();
 }
 
-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 04/10] use cds_list_for_each_entry_safe()
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
                   ` (2 preceding siblings ...)
  2011-09-15  5:38 ` [ltt-dev] [PATCH 03/10] use get_cpu_call_rcu_data() for get_call_rcu_data() Lai Jiangshan
@ 2011-09-15  5:38 ` Lai Jiangshan
  2011-09-15 15:11   ` Mathieu Desnoyers
  2011-09-15  5:39 ` [ltt-dev] [PATCH 05/10] avoid memory leak in call_rcu_data_free() Lai Jiangshan
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:38 UTC (permalink / raw)


Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index 1fbce98..6cb08be 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -651,7 +651,7 @@ void call_rcu_after_fork_parent(void)
  */
 void call_rcu_after_fork_child(void)
 {
-	struct call_rcu_data *crdp;
+	struct call_rcu_data *crdp, *next;
 
 	/* Release the mutex. */
 	call_rcu_unlock(&call_rcu_mutex);
@@ -664,12 +664,9 @@ void call_rcu_after_fork_child(void)
 	(void)get_default_call_rcu_data();
 
 	/* Dispose of all of the rest of the call_rcu_data structures. */
-	while (call_rcu_data_list.next != call_rcu_data_list.prev) {
-		crdp = cds_list_entry(call_rcu_data_list.prev,
-				      struct call_rcu_data, list);
+	cds_list_for_each_entry_safe(crdp, next, &call_rcu_data_list, list) {
 		if (crdp == default_call_rcu_data)
-			crdp = cds_list_entry(crdp->list.prev,
-					      struct call_rcu_data, list);
+			continue;
 		uatomic_set(&crdp->flags, URCU_CALL_RCU_STOPPED);
 		call_rcu_data_free(crdp);
 	}
-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 05/10] avoid memory leak in call_rcu_data_free()
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
                   ` (3 preceding siblings ...)
  2011-09-15  5:38 ` [ltt-dev] [PATCH 04/10] use cds_list_for_each_entry_safe() Lai Jiangshan
@ 2011-09-15  5:39 ` Lai Jiangshan
  2011-09-15 15:14   ` Mathieu Desnoyers
  2011-09-15  5:39 ` [ltt-dev] [PATCH 06/10] wake up default call_rcu thread after we move the leftover callbacks Lai Jiangshan
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:39 UTC (permalink / raw)


Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index 6cb08be..1ab49bc 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -600,9 +600,10 @@ void call_rcu_data_free(struct call_rcu_data *crdp)
 		*cbs_endprev = cbs;
 		uatomic_add(&default_call_rcu_data->qlen,
 			    uatomic_read(&crdp->qlen));
-		cds_list_del(&crdp->list);
-		free(crdp);
 	}
+
+	cds_list_del(&crdp->list);
+	free(crdp);
 }
 
 /*
-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 06/10] wake up default call_rcu thread after we move the leftover callbacks
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
                   ` (4 preceding siblings ...)
  2011-09-15  5:39 ` [ltt-dev] [PATCH 05/10] avoid memory leak in call_rcu_data_free() Lai Jiangshan
@ 2011-09-15  5:39 ` Lai Jiangshan
  2011-09-15 15:16   ` Mathieu Desnoyers
  2011-09-15  5:39 ` [ltt-dev] [PATCH 07/10] protect writing to per_cpu_call_rcu_data[*] Lai Jiangshan
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:39 UTC (permalink / raw)


Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index 1ab49bc..6e80fa9 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -600,6 +600,7 @@ void call_rcu_data_free(struct call_rcu_data *crdp)
 		*cbs_endprev = cbs;
 		uatomic_add(&default_call_rcu_data->qlen,
 			    uatomic_read(&crdp->qlen));
+		wake_call_rcu_thread(default_call_rcu_data);
 	}
 
 	cds_list_del(&crdp->list);
-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 07/10] protect writing to per_cpu_call_rcu_data[*]
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
                   ` (5 preceding siblings ...)
  2011-09-15  5:39 ` [ltt-dev] [PATCH 06/10] wake up default call_rcu thread after we move the leftover callbacks Lai Jiangshan
@ 2011-09-15  5:39 ` Lai Jiangshan
  2011-09-15 15:17   ` Mathieu Desnoyers
  2011-09-15  5:39 ` [ltt-dev] [PATCH 08/10] Return -EEXIST when the old cpu call_rcu_data has not been removed Lai Jiangshan
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:39 UTC (permalink / raw)


Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index 6e80fa9..c822646 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -396,12 +396,13 @@ int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
 		errno = EINVAL;
 		return -EINVAL;
 	}
-	call_rcu_unlock(&call_rcu_mutex);
 	if (per_cpu_call_rcu_data == NULL) {
+		call_rcu_unlock(&call_rcu_mutex);
 		errno = ENOMEM;
 		return -ENOMEM;
 	}
 	per_cpu_call_rcu_data[cpu] = crdp;
+	call_rcu_unlock(&call_rcu_mutex);
 	return 0;
 }
 
-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 08/10] Return -EEXIST when the old cpu call_rcu_data has not been removed
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
                   ` (6 preceding siblings ...)
  2011-09-15  5:39 ` [ltt-dev] [PATCH 07/10] protect writing to per_cpu_call_rcu_data[*] Lai Jiangshan
@ 2011-09-15  5:39 ` Lai Jiangshan
  2011-09-15 15:19   ` Mathieu Desnoyers
  2011-09-15  5:39 ` [ltt-dev] [PATCH 09/10] avoid leaking crdp for failed path Lai Jiangshan
  2011-09-15 15:23 ` [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Mathieu Desnoyers
  9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:39 UTC (permalink / raw)


To make it matches the comments.

It is the caller's responsibility to use
set_cpu_call_rcu_data(cpu, NULL) to remove the CPU's
call_rcu_data structure and dispose it.

Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index c822646..ae93468 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -396,11 +396,19 @@ int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
 		errno = EINVAL;
 		return -EINVAL;
 	}
+
 	if (per_cpu_call_rcu_data == NULL) {
 		call_rcu_unlock(&call_rcu_mutex);
 		errno = ENOMEM;
 		return -ENOMEM;
 	}
+
+	if (per_cpu_call_rcu_data[cpu] != NULL && crdp != NULL) {
+		call_rcu_unlock(&call_rcu_mutex);
+		errno = EEXIST;
+		return -EEXIST;
+	}
+
 	per_cpu_call_rcu_data[cpu] = crdp;
 	call_rcu_unlock(&call_rcu_mutex);
 	return 0;
-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 09/10] avoid leaking crdp for failed path
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
                   ` (7 preceding siblings ...)
  2011-09-15  5:39 ` [ltt-dev] [PATCH 08/10] Return -EEXIST when the old cpu call_rcu_data has not been removed Lai Jiangshan
@ 2011-09-15  5:39 ` Lai Jiangshan
  2011-09-15 15:21   ` Mathieu Desnoyers
  2011-09-15 15:23 ` [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Mathieu Desnoyers
  9 siblings, 1 reply; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-15  5:39 UTC (permalink / raw)


Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index ae93468..f9250e8 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -522,8 +522,13 @@ int create_all_cpu_call_rcu_data(unsigned long flags)
 		}
 		call_rcu_unlock(&call_rcu_mutex);
 		if ((ret = set_cpu_call_rcu_data(i, crdp)) != 0) {
-			/* FIXME: Leaks crdp for now. */
-			return ret; /* Can happen on race. */
+			call_rcu_data_free(crdp);
+
+			/* it has been created by other thread */
+			if (ret == -EEXIST)
+				continue;
+
+			return ret;
 		}
 	}
 	return 0;
-- 
1.7.4.4





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 01/10] add missing static
  2011-09-15  5:38 ` [ltt-dev] [PATCH 01/10] add missing static Lai Jiangshan
@ 2011-09-15 14:54   ` Mathieu Desnoyers
  0 siblings, 0 replies; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 14:54 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>

Merged, thanks Lai!

Mathieu

> ---
>  urcu-call-rcu-impl.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> index c14cc18..700d128 100644
> --- a/urcu-call-rcu-impl.h
> +++ b/urcu-call-rcu-impl.h
> @@ -383,7 +383,7 @@ struct call_rcu_data *create_call_rcu_data(unsigned long flags,
>  
>  int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
>  {
> -	int warned = 0;
> +	static int warned = 0;
>  
>  	call_rcu_lock(&call_rcu_mutex);
>  	if (cpu < 0 || maxcpus <= cpu) {
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 02/10] init maxcpus before use it
  2011-09-15  5:38 ` [ltt-dev] [PATCH 02/10] init maxcpus before use it Lai Jiangshan
@ 2011-09-15 14:59   ` Mathieu Desnoyers
  0 siblings, 0 replies; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 14:59 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>

    [ Edit:
      Covers the per-cpu call_rcu data setup (not all_cpus helper, which is
      why we did not trigger it in our tests. ]

Merged, thanks!

Mathieu

> ---
>  urcu-call-rcu-impl.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> index 700d128..e0a8fd8 100644
> --- a/urcu-call-rcu-impl.h
> +++ b/urcu-call-rcu-impl.h
> @@ -386,6 +386,7 @@ int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
>  	static int warned = 0;
>  
>  	call_rcu_lock(&call_rcu_mutex);
> +	alloc_cpu_call_rcu_data();
>  	if (cpu < 0 || maxcpus <= cpu) {
>  		if (!warned) {
>  			fprintf(stderr, "[error] liburcu: set CPU # out of range\n");
> @@ -395,7 +396,6 @@ int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
>  		errno = EINVAL;
>  		return -EINVAL;
>  	}
> -	alloc_cpu_call_rcu_data();
>  	call_rcu_unlock(&call_rcu_mutex);
>  	if (per_cpu_call_rcu_data == NULL) {
>  		errno = ENOMEM;
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 03/10] use get_cpu_call_rcu_data() for get_call_rcu_data()
  2011-09-15  5:38 ` [ltt-dev] [PATCH 03/10] use get_cpu_call_rcu_data() for get_call_rcu_data() Lai Jiangshan
@ 2011-09-15 15:08   ` Mathieu Desnoyers
  0 siblings, 0 replies; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 15:08 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>

[ Impact: refactor duplicated code ]

Merged, thanks!

Mathieu

> ---
>  urcu-call-rcu-impl.h |   19 +++++++------------
>  1 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> index e0a8fd8..1fbce98 100644
> --- a/urcu-call-rcu-impl.h
> +++ b/urcu-call-rcu-impl.h
> @@ -435,22 +435,17 @@ struct call_rcu_data *get_default_call_rcu_data(void)
>   */
>  struct call_rcu_data *get_call_rcu_data(void)
>  {
> -	int curcpu;
> -	static int warned = 0;
> +	struct call_rcu_data *crd;
>  
>  	if (thread_call_rcu_data != NULL)
>  		return thread_call_rcu_data;
> -	if (maxcpus <= 0)
> -		return get_default_call_rcu_data();
> -	curcpu = sched_getcpu();
> -	if (!warned && (curcpu < 0 || maxcpus <= curcpu)) {
> -		fprintf(stderr, "[error] liburcu: gcrd CPU # out of range\n");
> -		warned = 1;
> +
> +	if (maxcpus > 0) {
> +		crd = get_cpu_call_rcu_data(sched_getcpu());
> +		if (crd)
> +			return crd;
>  	}
> -	if (curcpu >= 0 && maxcpus > curcpu &&
> -	    per_cpu_call_rcu_data != NULL &&
> -	    per_cpu_call_rcu_data[curcpu] != NULL)
> -	    	return per_cpu_call_rcu_data[curcpu];
> +
>  	return get_default_call_rcu_data();
>  }
>  
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 04/10] use cds_list_for_each_entry_safe()
  2011-09-15  5:38 ` [ltt-dev] [PATCH 04/10] use cds_list_for_each_entry_safe() Lai Jiangshan
@ 2011-09-15 15:11   ` Mathieu Desnoyers
  0 siblings, 0 replies; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 15:11 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>

Renamed patch to:

urcu call_rcu: fix use after free()

call_rcu_after_fork_child() needs to use cds_list_for_each_entry_safe to
safely iterate on the list as its item is being freed.


Merged, thanks!

Mathieu

> ---
>  urcu-call-rcu-impl.h |    9 +++------
>  1 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> index 1fbce98..6cb08be 100644
> --- a/urcu-call-rcu-impl.h
> +++ b/urcu-call-rcu-impl.h
> @@ -651,7 +651,7 @@ void call_rcu_after_fork_parent(void)
>   */
>  void call_rcu_after_fork_child(void)
>  {
> -	struct call_rcu_data *crdp;
> +	struct call_rcu_data *crdp, *next;
>  
>  	/* Release the mutex. */
>  	call_rcu_unlock(&call_rcu_mutex);
> @@ -664,12 +664,9 @@ void call_rcu_after_fork_child(void)
>  	(void)get_default_call_rcu_data();
>  
>  	/* Dispose of all of the rest of the call_rcu_data structures. */
> -	while (call_rcu_data_list.next != call_rcu_data_list.prev) {
> -		crdp = cds_list_entry(call_rcu_data_list.prev,
> -				      struct call_rcu_data, list);
> +	cds_list_for_each_entry_safe(crdp, next, &call_rcu_data_list, list) {
>  		if (crdp == default_call_rcu_data)
> -			crdp = cds_list_entry(crdp->list.prev,
> -					      struct call_rcu_data, list);
> +			continue;
>  		uatomic_set(&crdp->flags, URCU_CALL_RCU_STOPPED);
>  		call_rcu_data_free(crdp);
>  	}
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 05/10] avoid memory leak in call_rcu_data_free()
  2011-09-15  5:39 ` [ltt-dev] [PATCH 05/10] avoid memory leak in call_rcu_data_free() Lai Jiangshan
@ 2011-09-15 15:14   ` Mathieu Desnoyers
  0 siblings, 0 replies; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 15:14 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>

Merged, thanks!

Mathieu

> ---
>  urcu-call-rcu-impl.h |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> index 6cb08be..1ab49bc 100644
> --- a/urcu-call-rcu-impl.h
> +++ b/urcu-call-rcu-impl.h
> @@ -600,9 +600,10 @@ void call_rcu_data_free(struct call_rcu_data *crdp)
>  		*cbs_endprev = cbs;
>  		uatomic_add(&default_call_rcu_data->qlen,
>  			    uatomic_read(&crdp->qlen));
> -		cds_list_del(&crdp->list);
> -		free(crdp);
>  	}
> +
> +	cds_list_del(&crdp->list);
> +	free(crdp);
>  }
>  
>  /*
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 06/10] wake up default call_rcu thread after we move the leftover callbacks
  2011-09-15  5:39 ` [ltt-dev] [PATCH 06/10] wake up default call_rcu thread after we move the leftover callbacks Lai Jiangshan
@ 2011-09-15 15:16   ` Mathieu Desnoyers
  0 siblings, 0 replies; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 15:16 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>

Merged, thanks!

Mathieu

> ---
>  urcu-call-rcu-impl.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> index 1ab49bc..6e80fa9 100644
> --- a/urcu-call-rcu-impl.h
> +++ b/urcu-call-rcu-impl.h
> @@ -600,6 +600,7 @@ void call_rcu_data_free(struct call_rcu_data *crdp)
>  		*cbs_endprev = cbs;
>  		uatomic_add(&default_call_rcu_data->qlen,
>  			    uatomic_read(&crdp->qlen));
> +		wake_call_rcu_thread(default_call_rcu_data);
>  	}
>  
>  	cds_list_del(&crdp->list);
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 07/10] protect writing to per_cpu_call_rcu_data[*]
  2011-09-15  5:39 ` [ltt-dev] [PATCH 07/10] protect writing to per_cpu_call_rcu_data[*] Lai Jiangshan
@ 2011-09-15 15:17   ` Mathieu Desnoyers
  0 siblings, 0 replies; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 15:17 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>

Merged, thanks!

Mathieu

> ---
>  urcu-call-rcu-impl.h |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> index 6e80fa9..c822646 100644
> --- a/urcu-call-rcu-impl.h
> +++ b/urcu-call-rcu-impl.h
> @@ -396,12 +396,13 @@ int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
>  		errno = EINVAL;
>  		return -EINVAL;
>  	}
> -	call_rcu_unlock(&call_rcu_mutex);
>  	if (per_cpu_call_rcu_data == NULL) {
> +		call_rcu_unlock(&call_rcu_mutex);
>  		errno = ENOMEM;
>  		return -ENOMEM;
>  	}
>  	per_cpu_call_rcu_data[cpu] = crdp;
> +	call_rcu_unlock(&call_rcu_mutex);
>  	return 0;
>  }
>  
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 08/10] Return -EEXIST when the old cpu call_rcu_data has not been removed
  2011-09-15  5:39 ` [ltt-dev] [PATCH 08/10] Return -EEXIST when the old cpu call_rcu_data has not been removed Lai Jiangshan
@ 2011-09-15 15:19   ` Mathieu Desnoyers
  0 siblings, 0 replies; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 15:19 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> To make it matches the comments.
> 
> It is the caller's responsibility to use
> set_cpu_call_rcu_data(cpu, NULL) to remove the CPU's
> call_rcu_data structure and dispose it.

Merged, thanks!

Mathieu

> 
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> ---
>  urcu-call-rcu-impl.h |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> index c822646..ae93468 100644
> --- a/urcu-call-rcu-impl.h
> +++ b/urcu-call-rcu-impl.h
> @@ -396,11 +396,19 @@ int set_cpu_call_rcu_data(int cpu, struct call_rcu_data *crdp)
>  		errno = EINVAL;
>  		return -EINVAL;
>  	}
> +
>  	if (per_cpu_call_rcu_data == NULL) {
>  		call_rcu_unlock(&call_rcu_mutex);
>  		errno = ENOMEM;
>  		return -ENOMEM;
>  	}
> +
> +	if (per_cpu_call_rcu_data[cpu] != NULL && crdp != NULL) {
> +		call_rcu_unlock(&call_rcu_mutex);
> +		errno = EEXIST;
> +		return -EEXIST;
> +	}
> +
>  	per_cpu_call_rcu_data[cpu] = crdp;
>  	call_rcu_unlock(&call_rcu_mutex);
>  	return 0;
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 09/10] avoid leaking crdp for failed path
  2011-09-15  5:39 ` [ltt-dev] [PATCH 09/10] avoid leaking crdp for failed path Lai Jiangshan
@ 2011-09-15 15:21   ` Mathieu Desnoyers
  0 siblings, 0 replies; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 15:21 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>

    [ Comment: now that set_cpu_call_rcu_data() is not racy and detects
      overwrites, we can effectively trust its return value and free the
      crdp if already set. ]

Merged, thanks!

Mathieu

> ---
>  urcu-call-rcu-impl.h |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> index ae93468..f9250e8 100644
> --- a/urcu-call-rcu-impl.h
> +++ b/urcu-call-rcu-impl.h
> @@ -522,8 +522,13 @@ int create_all_cpu_call_rcu_data(unsigned long flags)
>  		}
>  		call_rcu_unlock(&call_rcu_mutex);
>  		if ((ret = set_cpu_call_rcu_data(i, crdp)) != 0) {
> -			/* FIXME: Leaks crdp for now. */
> -			return ret; /* Can happen on race. */
> +			call_rcu_data_free(crdp);
> +
> +			/* it has been created by other thread */
> +			if (ret == -EEXIST)
> +				continue;
> +
> +			return ret;
>  		}
>  	}
>  	return 0;
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1
  2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
                   ` (8 preceding siblings ...)
  2011-09-15  5:39 ` [ltt-dev] [PATCH 09/10] avoid leaking crdp for failed path Lai Jiangshan
@ 2011-09-15 15:23 ` Mathieu Desnoyers
  2011-09-15 15:57   ` Paul E. McKenney
  9 siblings, 1 reply; 22+ messages in thread
From: Mathieu Desnoyers @ 2011-09-15 15:23 UTC (permalink / raw)


* Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> 2cleanups and 8bugfixes
> 
> Lai Jiangshan (10):
>   add missing static
>   init maxcpus before use it
>   use get_cpu_call_rcu_data() for get_call_rcu_data()
>   use cds_list_for_each_entry_safe()
>   avoid memory leak in call_rcu_data_free()
>   wake up default call_rcu thread after we move the leftover callbacks
>   protect writing to per_cpu_call_rcu_data[*]
>   Return -EEXIST when the old cpu call_rcu_data has not been removed
>   avoid leaking crdp for failed path
>   protects call_rcu_data_list when remove node

Hi Lai,

I think patch 10 (protects call_rcu_data_list when remove node) did not
make it into my inbox. Can you resend it please ?

Thank you,

Mathieu

> 
>  urcu-call-rcu-impl.h |   61 +++++++++++++++++++++++++++++--------------------
>  1 files changed, 36 insertions(+), 25 deletions(-)
> 
> -- 
> 1.7.4.4
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com




^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1
  2011-09-15 15:23 ` [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Mathieu Desnoyers
@ 2011-09-15 15:57   ` Paul E. McKenney
  2011-09-16  3:33     ` Lai Jiangshan
  0 siblings, 1 reply; 22+ messages in thread
From: Paul E. McKenney @ 2011-09-15 15:57 UTC (permalink / raw)


On Thu, Sep 15, 2011 at 11:23:04AM -0400, Mathieu Desnoyers wrote:
> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
> > 2cleanups and 8bugfixes
> > 
> > Lai Jiangshan (10):
> >   add missing static
> >   init maxcpus before use it
> >   use get_cpu_call_rcu_data() for get_call_rcu_data()
> >   use cds_list_for_each_entry_safe()
> >   avoid memory leak in call_rcu_data_free()
> >   wake up default call_rcu thread after we move the leftover callbacks
> >   protect writing to per_cpu_call_rcu_data[*]
> >   Return -EEXIST when the old cpu call_rcu_data has not been removed
> >   avoid leaking crdp for failed path
> >   protects call_rcu_data_list when remove node

These look good to me, thank you, Lai!

> Hi Lai,
> 
> I think patch 10 (protects call_rcu_data_list when remove node) did not
> make it into my inbox. Can you resend it please ?

Same here.

							Thanx, Paul

> Thank you,
> 
> Mathieu
> 
> > 
> >  urcu-call-rcu-impl.h |   61 +++++++++++++++++++++++++++++--------------------
> >  1 files changed, 36 insertions(+), 25 deletions(-)
> > 
> > -- 
> > 1.7.4.4
> > 
> 
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1
  2011-09-15 15:57   ` Paul E. McKenney
@ 2011-09-16  3:33     ` Lai Jiangshan
  0 siblings, 0 replies; 22+ messages in thread
From: Lai Jiangshan @ 2011-09-16  3:33 UTC (permalink / raw)




On 09/15/2011 11:57 PM, Paul E. McKenney wrote:
> On Thu, Sep 15, 2011 at 11:23:04AM -0400, Mathieu Desnoyers wrote:
>> * Lai Jiangshan (laijs at cn.fujitsu.com) wrote:
>>> 2cleanups and 8bugfixes
>>>
>>> Lai Jiangshan (10):
>>>   add missing static
>>>   init maxcpus before use it
>>>   use get_cpu_call_rcu_data() for get_call_rcu_data()
>>>   use cds_list_for_each_entry_safe()
>>>   avoid memory leak in call_rcu_data_free()
>>>   wake up default call_rcu thread after we move the leftover callbacks
>>>   protect writing to per_cpu_call_rcu_data[*]
>>>   Return -EEXIST when the old cpu call_rcu_data has not been removed
>>>   avoid leaking crdp for failed path
>>>   protects call_rcu_data_list when remove node
> 
> These look good to me, thank you, Lai!
> 
>> Hi Lai,
>>
>> I think patch 10 (protects call_rcu_data_list when remove node) did not
>> make it into my inbox. Can you resend it please ?
> 
> Same here.
> 
> 							Thanx, Paul
> 

sorry, patch is appended.

Thanks,
Lai

Subject: [PATCH 10/10] protects call_rcu_data_list when remove node

Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
---
 urcu-call-rcu-impl.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
index f9250e8..87d9157 100644
--- a/urcu-call-rcu-impl.h
+++ b/urcu-call-rcu-impl.h
@@ -617,7 +617,10 @@ void call_rcu_data_free(struct call_rcu_data *crdp)
 		wake_call_rcu_thread(default_call_rcu_data);
 	}
 
+	call_rcu_lock(&call_rcu_mutex);
 	cds_list_del(&crdp->list);
+	call_rcu_unlock(&call_rcu_mutex);
+
 	free(crdp);
 }
 
-- 
1.7.4.4




^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2011-09-16  3:33 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-15  5:38 [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Lai Jiangshan
2011-09-15  5:38 ` [ltt-dev] [PATCH 01/10] add missing static Lai Jiangshan
2011-09-15 14:54   ` Mathieu Desnoyers
2011-09-15  5:38 ` [ltt-dev] [PATCH 02/10] init maxcpus before use it Lai Jiangshan
2011-09-15 14:59   ` Mathieu Desnoyers
2011-09-15  5:38 ` [ltt-dev] [PATCH 03/10] use get_cpu_call_rcu_data() for get_call_rcu_data() Lai Jiangshan
2011-09-15 15:08   ` Mathieu Desnoyers
2011-09-15  5:38 ` [ltt-dev] [PATCH 04/10] use cds_list_for_each_entry_safe() Lai Jiangshan
2011-09-15 15:11   ` Mathieu Desnoyers
2011-09-15  5:39 ` [ltt-dev] [PATCH 05/10] avoid memory leak in call_rcu_data_free() Lai Jiangshan
2011-09-15 15:14   ` Mathieu Desnoyers
2011-09-15  5:39 ` [ltt-dev] [PATCH 06/10] wake up default call_rcu thread after we move the leftover callbacks Lai Jiangshan
2011-09-15 15:16   ` Mathieu Desnoyers
2011-09-15  5:39 ` [ltt-dev] [PATCH 07/10] protect writing to per_cpu_call_rcu_data[*] Lai Jiangshan
2011-09-15 15:17   ` Mathieu Desnoyers
2011-09-15  5:39 ` [ltt-dev] [PATCH 08/10] Return -EEXIST when the old cpu call_rcu_data has not been removed Lai Jiangshan
2011-09-15 15:19   ` Mathieu Desnoyers
2011-09-15  5:39 ` [ltt-dev] [PATCH 09/10] avoid leaking crdp for failed path Lai Jiangshan
2011-09-15 15:21   ` Mathieu Desnoyers
2011-09-15 15:23 ` [ltt-dev] [PATCH 00/10] urcu,call_rcu: minior fixes, round1 Mathieu Desnoyers
2011-09-15 15:57   ` Paul E. McKenney
2011-09-16  3:33     ` Lai Jiangshan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox