Re: [lttng-dev] [rp] [RFC] Readiness for URCU release with RCU lock-free hash table
* Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Wed, May 02, 2012 at 12:16:39AM -0400, Mathieu Desnoyers wrote: * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Tue, May 01, 2012 at 05:02:06PM -0400, Mathieu Desnoyers wrote: * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Tue, May 01, 2012 at 01:41:36PM -0400, Mathieu Desnoyers wrote: * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Tue, May 01, 2012 at 11:21:44AM -0400, Mathieu Desnoyers wrote: * Mathieu Desnoyers (mathieu.desnoy...@efficios.com) wrote: * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Tue, May 01, 2012 at 10:16:09AM -0400, Mathieu Desnoyers wrote: Hi! After 1 year of development, with the last 6-7 months spent polishing the API and testing the implementation, I think it is getting about time to release the RCU lock-free hash table in a new Userspace RCU version (0.7). I recently described the guarantees provided by the hash table in more detail, and created tests for the uniqueness guarantee for traversals performed concurrently with add_unique and add_replace operations. I also added test modes that create long hash chains, to test corner-cases of the hash table. One small thing I wonder is whether we should document that the hash table update operations imply full memory barriers ? The Linux kernel's rule seems good here -- if a hash-table operation is atomic and returns a value, it should imply a full barrier. So: cds_lfht_new(): No point in claiming barriers -- publishing the pointer to the hash table is where the barriers are important. cds_lfht_destroy(): Ditto. cds_lfht_lookup(): Not an update (let alone an atomic update), no barriers. cds_lfht_next_duplicate(): Ditto. cds_lfht_first(): Ditto. cds_lfht_next(): Ditto. cds_lfht_add(): Atomic update, but no return value, so no barrier implied. Yep, makes sense. We use cmpxchg internally to perform the update, but it could make sense to eventually use a cmpxchg that has no memory barriers to perform this update. So I agree on not providing a memory barrier guarantee on the add operation, since it does not return any value. cds_lfht_add_unique(): Atomic update that returns a value, so should imply a full memory barrier. add_unique is a bit special: - if it returns the node received as parameter, it means the add succeeded, which imply an update, and thus a memory barrier. Hrm, thinking further: if we make the add operation not act as a full memory barrier, then the add_unique success should not act as a full mb neither. Think of it as being similar to the Linux kernel's atomic_inc() and atomic_add_return() primitives. The latter guarantees memory barriers and the former does not. I think add/add_unique vs atomic_inc/atomic_add_return are fundamentally different. atomic_inc: - input: increment - output: none - effect: increment target address value atomic_add_return: - input: increment - output: incremented value - effect: increment target address value, atomically reading the resulting value. hash table add: - input: new node to add - output: none - effect: atomically add the new node into the table hash table add_unique (success): - input: new node to add - output: (we just return whether the operation has succeeded) - effect: atomically add the new node into the table hash table add_unique (failure): - input: new node to try adding - output: (we just return whether the operation has succeeded) - effect: simple lookup (read) So as we see, the add_unique failure only performs a read. Adding a memory barrier before this read would require us to add a memory barrier also on the success path, which would degrade performance. The success path does: lookup failure, cmpxchg to add the node, retry if changed. Adding a memory barrier before the lookup would add an extra memory barrier in
Re: [lttng-dev] [rp] [RFC] Readiness for URCU release with RCU lock-free hash table
On Thu, May 03, 2012 at 01:13:30PM -0400, Mathieu Desnoyers wrote: * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Wed, May 02, 2012 at 12:16:39AM -0400, Mathieu Desnoyers wrote: * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Tue, May 01, 2012 at 05:02:06PM -0400, Mathieu Desnoyers wrote: * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Tue, May 01, 2012 at 01:41:36PM -0400, Mathieu Desnoyers wrote: * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Tue, May 01, 2012 at 11:21:44AM -0400, Mathieu Desnoyers wrote: * Mathieu Desnoyers (mathieu.desnoy...@efficios.com) wrote: * Paul E. McKenney (paul...@linux.vnet.ibm.com) wrote: On Tue, May 01, 2012 at 10:16:09AM -0400, Mathieu Desnoyers wrote: Hi! After 1 year of development, with the last 6-7 months spent polishing the API and testing the implementation, I think it is getting about time to release the RCU lock-free hash table in a new Userspace RCU version (0.7). I recently described the guarantees provided by the hash table in more detail, and created tests for the uniqueness guarantee for traversals performed concurrently with add_unique and add_replace operations. I also added test modes that create long hash chains, to test corner-cases of the hash table. One small thing I wonder is whether we should document that the hash table update operations imply full memory barriers ? The Linux kernel's rule seems good here -- if a hash-table operation is atomic and returns a value, it should imply a full barrier. So: cds_lfht_new(): No point in claiming barriers -- publishing the pointer to the hash table is where the barriers are important. cds_lfht_destroy(): Ditto. cds_lfht_lookup(): Not an update (let alone an atomic update), no barriers. cds_lfht_next_duplicate(): Ditto. cds_lfht_first(): Ditto. cds_lfht_next(): Ditto. cds_lfht_add(): Atomic update, but no return value, so no barrier implied. Yep, makes sense. We use cmpxchg internally to perform the update, but it could make sense to eventually use a cmpxchg that has no memory barriers to perform this update. So I agree on not providing a memory barrier guarantee on the add operation, since it does not return any value. cds_lfht_add_unique(): Atomic update that returns a value, so should imply a full memory barrier. add_unique is a bit special: - if it returns the node received as parameter, it means the add succeeded, which imply an update, and thus a memory barrier. Hrm, thinking further: if we make the add operation not act as a full memory barrier, then the add_unique success should not act as a full mb neither. Think of it as being similar to the Linux kernel's atomic_inc() and atomic_add_return() primitives. The latter guarantees memory barriers and the former does not. I think add/add_unique vs atomic_inc/atomic_add_return are fundamentally different. atomic_inc: - input: increment - output: none - effect: increment target address value atomic_add_return: - input: increment - output: incremented value - effect: increment target address value, atomically reading the resulting value. hash table add: - input: new node to add - output: none - effect: atomically add the new node into the table hash table add_unique (success): - input: new node to add - output: (we just return whether the operation has succeeded) - effect: atomically add the new node into the table hash table add_unique (failure): - input: new node to try adding - output: (we just return whether the operation has succeeded) - effect: simple lookup (read) So as we see, the add_unique failure only performs a read. Adding a memory barrier before this read would require us to add a memory barrier also on the success path, which would
[lttng-dev] [lttng-ust PATCH 2/2] Add 2x int and 2x long types to the Java interface
Since we have to statically define all the available event types,
offer types for two integers and two longs, which are relatively
common use cases.
Signed-off-by: Alexandre Montplaisir alexandre.montplai...@polymtl.ca
---
liblttng-ust-java/LTTngUst.c | 28
liblttng-ust-java/LTTngUst.java| 28
liblttng-ust-java/lttng_ust_java.h | 18 ++
3 files changed, 74 insertions(+)
diff --git a/liblttng-ust-java/LTTngUst.c b/liblttng-ust-java/LTTngUst.c
index 0bef89d..9ae3983 100644
--- a/liblttng-ust-java/LTTngUst.c
+++ b/liblttng-ust-java/LTTngUst.c
@@ -35,6 +35,20 @@ JNIEXPORT void JNICALL
Java_org_lttng_ust_LTTngUst_tracepointInt(JNIEnv *env,
(*env)-ReleaseStringUTFChars(env, ev_name, ev_name_cstr);
}
+JNIEXPORT void JNICALL Java_org_lttng_ust_LTTngUst_tracepointIntInt(JNIEnv
*env,
+ jobject jobj,
+ jstring ev_name,
+ jint payload1,
+ jint payload2)
+{
+ jboolean iscopy;
+ const char *ev_name_cstr = (*env)-GetStringUTFChars(env, ev_name,
iscopy);
+
+ tracepoint(lttng_ust_java, int_int_event, ev_name_cstr, payload1,
payload2);
+
+ (*env)-ReleaseStringUTFChars(env, ev_name, ev_name_cstr);
+}
+
JNIEXPORT void JNICALL Java_org_lttng_ust_LTTngUst_tracepointLong(JNIEnv *env,
jobject jobj,
jstring ev_name,
@@ -48,6 +62,20 @@ JNIEXPORT void JNICALL
Java_org_lttng_ust_LTTngUst_tracepointLong(JNIEnv *env,
(*env)-ReleaseStringUTFChars(env, ev_name, ev_name_cstr);
}
+JNIEXPORT void JNICALL Java_org_lttng_ust_LTTngUst_tracepointLongLong(JNIEnv
*env,
+ jobject jobj,
+ jstring ev_name,
+ jlong payload1,
+ jlong payload2)
+{
+ jboolean iscopy;
+ const char *ev_name_cstr = (*env)-GetStringUTFChars(env, ev_name,
iscopy);
+
+ tracepoint(lttng_ust_java, long_long_event, ev_name_cstr, payload1,
payload2);
+
+ (*env)-ReleaseStringUTFChars(env, ev_name, ev_name_cstr);
+}
+
JNIEXPORT void JNICALL Java_org_lttng_ust_LTTngUst_tracepointString(JNIEnv
*env,
jobject jobj,
jstring ev_name,
diff --git a/liblttng-ust-java/LTTngUst.java b/liblttng-ust-java/LTTngUst.java
index f4bea32..79ad97a 100644
--- a/liblttng-ust-java/LTTngUst.java
+++ b/liblttng-ust-java/LTTngUst.java
@@ -57,6 +57,20 @@ public abstract class LTTngUst {
public static native void tracepointInt(String name, int payload);
/**
+ * Insert a tracepoint with a payload consisting of two integers.
+ *
+ * @param name
+ *The name assigned to this event. For best performance, this
+ *should be a statically-defined String, or a literal.
+ * @param payload1
+ *The first int payload
+ * @param payload2
+ *The second int payload
+ */
+public static native void
+tracepointIntInt(String name, int payload1, int payload2);
+
+/**
* Insert a tracepoint with a payload of type Long
*
* @param name
@@ -68,6 +82,20 @@ public abstract class LTTngUst {
public static native void tracepointLong(String name, long payload);
/**
+ * Insert a tracepoint with a payload consisting of two longs.
+ *
+ * @param name
+ *The name assigned to this event. For best performance, this
+ *should be a statically-defined String, or a literal.
+ * @param payload1
+ *The first long payload
+ * @param payload2
+ *The second long payload
+ */
+public static native void
+tracepointLongLong(String name, long payload1, long payload2);
+
+/**
* Insert a tracepoint with a String payload.
*
* @param name
diff --git a/liblttng-ust-java/lttng_ust_java.h
b/liblttng-ust-java/lttng_ust_java.h
index 7cdbea5..7a8b391 100644
--- a/liblttng-ust-java/lttng_ust_java.h
+++ b/liblttng-ust-java/lttng_ust_java.h
@@ -32,6 +32,15 @@ TRACEPOINT_EVENT(lttng_ust_java, int_event,
)
)
+TRACEPOINT_EVENT(lttng_ust_java, int_int_event,
+ TP_ARGS(const char *, name, int, payload1, int, payload2),
+ TP_FIELDS(
+ ctf_string(name, name)
+ ctf_integer(int, int_payload1, payload1)
+ ctf_integer(int, int_payload2, payload2)
+ )
+)
+
TRACEPOINT_EVENT(lttng_ust_java, long_event,
TP_ARGS(const char *, name, long, payload),
TP_FIELDS(
@@ -40,6 +49,15 @@ TRACEPOINT_EVENT(lttng_ust_java, long_event,
