date:20121205

Re: [lttng-dev] [URCU pull request review] urcu improvements

2012-12-05 Thread Mathieu Desnoyers

* Mathieu Desnoyers (mathieu.desnoy...@efficios.com) wrote:
 Here is a pull request to myself for my recent work on wfcqueue and urcu
 flavors into the userspace RCU master branch.
 
 Please let me know if you see any immediate show stoppers.

FYI, after small modifications to answer the comments, I pulled these
changes into master.

Thanks,

Mathieu

 
 List of changes:
 
 [PATCH 01/16] Fix: wfcqueue nonblocking dequeue
 [PATCH 02/16] wfcqueue: enqueue and splice return queue state
 [PATCH 03/16] test wfcqueue: add tests for queue state return value
 [PATCH 04/16] urcu-call-rcu: use wait-free splice return value
 [PATCH 05/16] wfcqueue: document empty criterion
 [PATCH 06/16] wfcqueue: implement mutex-free splice
 [PATCH 07/16] urcu-qsbr: improve 2-phase wait scheme
 [PATCH 08/16] urcu-mb/signal/membarrier: improve 2-phase wait scheme
 [PATCH 09/16] urcu-bp: improve 2-phase wait scheme
 [PATCH 10/16] urcu-qsbr: move offline threads to separate list
 [PATCH 11/16] urcu-mb/signal/membarrier: move quiescent threads to
 [PATCH 12/16] urcu-bp: move quiescent threads to separate list
 [PATCH 13/16] tests: use standard malloc/free for synchronize_rcu()
 [PATCH 14/16] urcu-qsbr: batch concurrent synchronize_rcu()
 [PATCH 15/16] urcu-wait: move wait code into separate file
 [PATCH 16/16] urcu-wait: move queue management code into urcu-wait.h
 
 diffstat:
 
  Makefile.am |2 
  tests/test_urcu.c   |   60 ++
  tests/test_urcu_bp.c|   59 ++
  tests/test_urcu_qsbr.c  |   61 ++-
  tests/test_urcu_wfcq.c  |  131 +---
  urcu-bp.c   |   88 ++---
  urcu-call-rcu-impl.h|9 +-
  urcu-qsbr.c |  193 
 
  urcu-wait.h |  184 +
  urcu.c  |   95 +++
  urcu/static/urcu-bp.h   |   29 ---
  urcu/static/urcu-qsbr.h |   14 ++-
  urcu/static/urcu.h  |   15 ++-
  urcu/static/wfcqueue.h  |  133 +
  urcu/wfcqueue.h |   62 +++
  wfcqueue.c  |   16 +--
  16 files changed, 786 insertions(+), 365 deletions(-)
 
 Thanks,
 
 Mathieu
 

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

___
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

2012-12-05 Thread Hui Zhu

Hi,

I am working on add CTF support to GDB.  You can see my patch review threads in:
http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html

To make GDB support CTF read, I use libbabeltrace with GDB.  You can
see the patch in
http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
I have a issue is  libbabeltrace have a function called lookup_enum
that is same with a GDB function.
I change the function name of GDB to handle this issue in my patch.

But Tom said let libbabeltrace to change function name is better.
So I send this mail to ask do you mind change the function name of
lookup_enum?   If you can change the function name that will be really
helpful for us.  Thanks a lot.
And I post a patch about change the function name in libbabeltrace.

Thanks,
Hui

--- a/formats/ctf/ctf.c
+++ b/formats/ctf/ctf.c
@@ -423,7 +423,7 @@ int ctf_read_event(struct stream_pos *pp
} else {
struct definition_enum *enum_definition;

-   enum_definition = 
lookup_enum(stream-stream_event_header-p, id, FALSE);
+   enum_definition = 
ctf_lookup_enum(stream-stream_event_header-p,
id, FALSE);
if (enum_definition) {
id = enum_definition-integer-value._unsigned;
}
--- a/include/babeltrace/types.h
+++ b/include/babeltrace/types.h
@@ -513,7 +513,7 @@ struct definition *lookup_definition(con
 struct definition_integer *lookup_integer(const struct definition *definition,
  const char *field_name,
  int signedness);
-struct definition_enum *lookup_enum(const struct definition *definition,
+struct definition_enum *ctf_lookup_enum(const struct definition *definition,
const char *field_name,
int signedness);
 struct definition *lookup_variant(const struct definition *definition,
--- a/types/types.c
+++ b/types/types.c
@@ -634,7 +634,7 @@ struct definition_integer *lookup_intege
return lookup_integer;
 }

-struct definition_enum *lookup_enum(const struct definition *definition,
+struct definition_enum *ctf_lookup_enum(const struct definition *definition,
const char *field_name,
int signedness)
 {

___
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [lttng-dev] [PATCH lttng-modules] Update kernel probes to more detailed match to kernel versions

2012-12-05 Thread Gabbasov, Andrew

Hi Mathieu,

 * Gabbasov, Andrew (andrew_gabba...@mentor.com) wrote:
  Hi Mathieu,
 
   * Andrew Gabbasov (andrew_gabba...@mentor.com) wrote:
We have added some more ifdef's to kernel probes to more closely match
the kernel tracepoints for different kernel versions. The changes cover
kernel versions from 2.6.38 up to 3.7. We did not track the earlier
versions, and they are filtered out in probes Makefile.
   
ext3 probe was modified to use #include ../fs/ext3/ext3.h instead of
copying this include from kernel source to lttng-modules source.
Using such includes (there will be more similar in other probes)
imposes a restriction to use the full kernel source for building
lttng-modules rather than having just kernel include directory,
but it looks like full source is required anyway, at least with respect
to Makefiles structure. May be it is worth mentioning somewhere
in the documentation.
   
The code was verified to compile with latest versions in all stable
branches from 2.6.38.x to 3.6.x and 3.7-rc7.
  
   I get this build failure on 3.5 with this patch:
  
 CC [M]  /home/compudj/work/lttng-modules/probes/lttng-probe-ext3.o
   /home/compudj/work/lttng-modules/probes/lttng-probe-ext3.c:30:29: fatal
   error: ../fs/ext3/ext3.h: No such file or directory
   compilation terminated.
  
   can you look into it and submit a revised patch ?
 
  You must be using kernel headers for building lttng-modules
  (linux-headers package or something like that), right? This is what I
  was writing about: since ext3.h is not available in kernel's include
  directory, and in order to avoid duplicating this file in
  lttng-modules sources, we have to use that ../ based reference, and
  this requires having full kernel source for building. The same issue
  will later be with btrfs and ext4 probes: their necessary include
  files are also not available in kernel headers.
 
  Trying to duplicate these header files to lttng-modules source seems
  to be not a good idea, especially if we need to add many ifdef's to
  them to track different kernel versions. We can try to insert some
  checks to the Makefile and skip building those probes if we do not
  have full kernel sources (actually if we do not have necessary include
  files). Or to declare having full kernel source as a requirement for
  building lttng-modules. What do you think?
 
 The user experience should be that if lttng-modules is built against
 linux-headers packages, the build should work fine, even if it means
 some extra features are disabled.
 
 And we don't want local copies of kernel headers, because then we could
 trigger runtime bugs if the headers don't match between the kernel
 version and local version.

OK, then I'm inserting the checks to Makefile, that build such probes if the 
full
kernel source is available and to skip them if not.

 
 
   Also, although the README file states that we support only 3.6.38+, we
   happen to also have support for kernels down to 2.6.32 with the patches
   in the linux-patches/ directory applied to the appropriate kernel
   versions. So it would be good that the instrumentation compiles for
   those older kernels too. I promise we won't go further back than 2.6.32
   though.
 
  We'll try to add the ifdef's for earlier version down to 2.6.32 to the
  probes, although the additional amount of conditions makes the code
  overcomplicated ;-)
 
 Indeed, but I never said supporting various kernels was easy ;)
 
 I don't care immensely if older kernels have less features: in some
 cases, it can be just OK to disable an entire tracepoint probe for older
 kernels. I just want the build to work fine for those.

Understood, updated probes (as well as some additional ones) will be
sent soon.

 By the way, we're planning to go for a 2.1 release in a matter of days.
 At that point, I'll have to decide if we keep the recent instrumentation
 commits introduced after the start of the -rc cycle in this release (if
 mature enough), or if we should branch before those for the 2.1 release
 if they need more testing (and keep these instrumentation extensions for
 2.2). Any thoughts on this ?

I believe, it would be desirable to make more thorough testing of new
instrumentations. On our side the testing was not too extensive, and was
done for ARM platform only. Although the probes are mostly replicating
the kernel's tracepoints, it would be reasonable to keep the commits
in a branch for some time so that more people in the community could
make some testing.

Also, a side note: the lock probe seems to be not working. It causes
kernel hang when trying to perform 'lttng start' at least on arm platform).
We suspect that it may be caused by some kind of locks recursion or
deadlocks (since tracing engine use locks too and they may be tried
to trace again). There is no solution or even more detailed analysis
so far, so I'll disable building of lock probe in the next update.

Thanks.

Best

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

2012-12-05 Thread Hui Zhu

On Wed, Dec 5, 2012 at 8:08 PM, Mathieu Desnoyers
mathieu.desnoy...@efficios.com wrote:
 * Hui Zhu (teawa...@gmail.com) wrote:
 Hi,

 I am working on add CTF support to GDB.  You can see my patch review threads 
 in:
 http://sourceware.org/ml/gdb-patches/2012-11/msg00552.html
 http://sourceware.org/ml/gdb-patches/2012-11/msg00554.html
 http://sourceware.org/ml/gdb-patches/2012-11/msg00553.html
 http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html
 http://sourceware.org/ml/gdb-patches/2012-11/msg00556.html

 To make GDB support CTF read, I use libbabeltrace with GDB.  You can
 see the patch in
 http://sourceware.org/ml/gdb-patches/2012-11/msg00555.html.
 I have a issue is  libbabeltrace have a function called lookup_enum
 that is same with a GDB function.
 I change the function name of GDB to handle this issue in my patch.

 But Tom said let libbabeltrace to change function name is better.
 So I send this mail to ask do you mind change the function name of
 lookup_enum?   If you can change the function name that will be really
 helpful for us.  Thanks a lot.
 And I post a patch about change the function name in libbabeltrace.

 I'm CCing Julien Desfossez on this one. From what I see,
 include/babeltrace/types.h is not included into the system, so it should
 not be considered to be a public header of libbabeltrace.

 Julien, is there an publically exposed babeltrace API that performs
 something similar to the internal lookup_enum() ?

 Hui, are you using other functions from include/babeltrace/types.h ?

No, my patch didn't use any function in this file.

Thanks,
Hui


 Thanks,

 Mathieu


 Thanks,
 Hui

 --- a/formats/ctf/ctf.c
 +++ b/formats/ctf/ctf.c
 @@ -423,7 +423,7 @@ int ctf_read_event(struct stream_pos *pp
   } else {
   struct definition_enum *enum_definition;

 - enum_definition = 
 lookup_enum(stream-stream_event_header-p, id, FALSE);
 + enum_definition = 
 ctf_lookup_enum(stream-stream_event_header-p,
 id, FALSE);
   if (enum_definition) {
   id = enum_definition-integer-value._unsigned;
   }
 --- a/include/babeltrace/types.h
 +++ b/include/babeltrace/types.h
 @@ -513,7 +513,7 @@ struct definition *lookup_definition(con
  struct definition_integer *lookup_integer(const struct definition 
 *definition,
 const char *field_name,
 int signedness);
 -struct definition_enum *lookup_enum(const struct definition *definition,
 +struct definition_enum *ctf_lookup_enum(const struct definition *definition,
   const char *field_name,
   int signedness);
  struct definition *lookup_variant(const struct definition *definition,
 --- a/types/types.c
 +++ b/types/types.c
 @@ -634,7 +634,7 @@ struct definition_integer *lookup_intege
   return lookup_integer;
  }

 -struct definition_enum *lookup_enum(const struct definition *definition,
 +struct definition_enum *ctf_lookup_enum(const struct definition *definition,
   const char *field_name,
   int signedness)
  {

 ___
 lttng-dev mailing list
 lttng-dev@lists.lttng.org
 http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

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

___
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[lttng-dev] [PATCH urcu] wfstack: implement nonblocking pop and next

2012-12-05 Thread Mathieu Desnoyers

Signed-off-by: Mathieu Desnoyers mathieu.desnoy...@efficios.com
---
diff --git a/urcu/static/wfstack.h b/urcu/static/wfstack.h
index 018a121..9bc9519 100644
--- a/urcu/static/wfstack.h
+++ b/urcu/static/wfstack.h
@@ -137,7 +137,7 @@ int _cds_wfs_push(struct cds_wfs_stack *s, struct 
cds_wfs_node *node)
  * Waiting for push to complete enqueue and return the next node.
  */
 static inline struct cds_wfs_node *
-___cds_wfs_node_sync_next(struct cds_wfs_node *node)
+___cds_wfs_node_sync_next(struct cds_wfs_node *node, int blocking)
 {
struct cds_wfs_node *next;
int attempt = 0;
@@ -146,6 +146,8 @@ ___cds_wfs_node_sync_next(struct cds_wfs_node *node)
 * Adaptative busy-looping waiting for push to complete.
 */
while ((next = CMM_LOAD_SHARED(node-next)) == NULL) {
+   if (!blocking)
+   return CDS_WFS_WOULDBLOCK;
if (++attempt = CDS_WFS_ADAPT_ATTEMPTS) {
poll(NULL, 0, CDS_WFS_WAIT);/* Wait for 10ms */
attempt = 0;
@@ -157,6 +159,29 @@ ___cds_wfs_node_sync_next(struct cds_wfs_node *node)
return next;
 }
 
+static inline
+struct cds_wfs_node *
+___cds_wfs_pop(struct cds_wfs_stack *s, int blocking)
+{
+   struct cds_wfs_head *head, *new_head;
+   struct cds_wfs_node *next;
+
+   for (;;) {
+   head = CMM_LOAD_SHARED(s-head);
+   if (___cds_wfs_end(head))
+   return NULL;
+   next = ___cds_wfs_node_sync_next(head-node, blocking);
+   if (!blocking  next == CDS_WFS_WOULDBLOCK)
+   return CDS_WFS_WOULDBLOCK;
+   new_head = caa_container_of(next, struct cds_wfs_head, node);
+   if (uatomic_cmpxchg(s-head, head, new_head) == head)
+   return head-node;
+   if (!blocking)
+   return CDS_WFS_WOULDBLOCK;
+   /* busy-loop if head changed under us */
+   }
+}
+
 /*
  * __cds_wfs_pop_blocking: pop a node from the stack.
  *
@@ -177,19 +202,20 @@ static inline
 struct cds_wfs_node *
 ___cds_wfs_pop_blocking(struct cds_wfs_stack *s)
 {
-   struct cds_wfs_head *head, *new_head;
-   struct cds_wfs_node *next;
+   return ___cds_wfs_pop(s, 1);
+}
 
-   for (;;) {
-   head = CMM_LOAD_SHARED(s-head);
-   if (___cds_wfs_end(head))
-   return NULL;
-   next = ___cds_wfs_node_sync_next(head-node);
-   new_head = caa_container_of(next, struct cds_wfs_head, node);
-   if (uatomic_cmpxchg(s-head, head, new_head) == head)
-   return head-node;
-   /* busy-loop if head changed under us */
-   }
+/*
+ * __cds_wfs_pop_nonblocking: pop a node from the stack.
+ *
+ * Same as __cds_wfs_pop_blocking, but returns CDS_WFS_WOULDBLOCK if
+ * it needs to block.
+ */
+static inline
+struct cds_wfs_node *
+___cds_wfs_pop_nonblocking(struct cds_wfs_stack *s)
+{
+   return ___cds_wfs_pop(s, 0);
 }
 
 /*
@@ -303,6 +329,22 @@ _cds_wfs_first(struct cds_wfs_head *head)
return head-node;
 }
 
+static inline struct cds_wfs_node *
+___cds_wfs_next(struct cds_wfs_node *node, int blocking)
+{
+   struct cds_wfs_node *next;
+
+   next = ___cds_wfs_node_sync_next(node, blocking);
+   /*
+* CDS_WFS_WOULDBLOCK != CSD_WFS_END, so we can check for end
+* even if ___cds_wfs_node_sync_next returns CDS_WFS_WOULDBLOCK,
+* and still return CDS_WFS_WOULDBLOCK.
+*/
+   if (___cds_wfs_end(next))
+   return NULL;
+   return next;
+}
+
 /*
  * cds_wfs_next_blocking: get next node of a popped stack.
  *
@@ -319,12 +361,20 @@ _cds_wfs_first(struct cds_wfs_head *head)
 static inline struct cds_wfs_node *
 _cds_wfs_next_blocking(struct cds_wfs_node *node)
 {
-   struct cds_wfs_node *next;
+   return ___cds_wfs_next(node, 1);
+}
 
-   next = ___cds_wfs_node_sync_next(node);
-   if (___cds_wfs_end(next))
-   return NULL;
-   return next;
+
+/*
+ * cds_wfs_next_nonblocking: get next node of a popped stack.
+ *
+ * Same as cds_wfs_next_blocking, but returns CDS_WFS_WOULDBLOCK if it
+ * needs to block.
+ */
+static inline struct cds_wfs_node *
+_cds_wfs_next_nonblocking(struct cds_wfs_node *node)
+{
+   return ___cds_wfs_next(node, 0);
 }
 
 #ifdef __cplusplus
diff --git a/urcu/wfstack.h b/urcu/wfstack.h
index 0e435ba..03fee8f 100644
--- a/urcu/wfstack.h
+++ b/urcu/wfstack.h
@@ -58,6 +58,8 @@ extern C {
  * synchronization.
  */
 
+#define CDS_WFS_WOULDBLOCK ((void *) -1UL)
+
 /*
  * struct cds_wfs_node is returned by __cds_wfs_pop, and also used as
  * iterator on stack. It is not safe to dereference the node next
@@ -184,6 +186,14 @@ extern struct cds_wfs_node *cds_wfs_first(struct 
cds_wfs_head *head);
 extern struct cds_wfs_node *cds_wfs_next_blocking(struct cds_wfs_node *node);
 
 /*
+ *

Re: [lttng-dev] Test if tracepoint is enabled

2012-12-05 Thread David Bryant


On 05/12/12 20:49, Mathieu Desnoyers wrote:

* David Bryant (david.bry...@quantum.com) wrote:

So, building on your example:

#define tracepoint_cond(provider, name, pre, post, ...) \
 do { \
 if (caa_unlikely(__tracepoint_##provider##___##name.state)) { \
 pre \
 __tracepoint_cb_##provider##___##name(__VA_ARGS__); \
 post \
 } \
 } \
 while (0)

And a contrived example:

#include stdio.h
#include stdlib.h

int main() {
 char * a;
 tracepoint_cond(provider, name,
 { printf(pre\n);  a = malloc(16); },
 { printf(post\n); free(a);}
);
 return 0;
}

This is ok. But more work with the macro handling would be required to
get the pre/post code fragments through as parameters. The example only
compiles because its pre/post arguments don't contain any commas.

What do you think?

I'm not sure what parameters would cause issues with commas. If they are
between parenthesis or brackets, it should be fine. Do you have an
example perhaps ?
Ok, I've just learnt what 'statement-expressions' are 
(http://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html). Would this be 
the first use of these in lttng-ust? If so, I'm wary of making lttng-ust 
dependent on this gcc extension. Would it be dependent on this? It is up 
to the user to decide what to pass to the macro, but the macro may 
impose constraints by way of how the arguments are employed within the 
macro...


As it happens, we compile with -pedantic which won't allow 
statement-expressions. (As an aside, -pedantic doesn't like tracepoint 
that don't take any arguments because it expects the ... to have at 
least one argument).


My example above (that doesn't use statement-expressions - I removed the 
round brackets) compiles correctly. But it you introduce a comma then 
the compiler seems to interpret this as an argument separator. So this 
works:


  #include stdio.h
  #include stdlib.h

  #define tracepoint_cond(provider, name, cond, pre, post, ...) \
  do { \
  if (caa_unlikely(__tracepoint_##provider##___##name.state)) { \
  if (cond) { \
  pre \
__tracepoint_cb_##provider##___##name(__VA_ARGS__); \
  post \
  } \
  } \
  } \
  while (0)

  int main() {
  int i = 1, j = 1;

  char * a;
  tracepoint_cond(provider, name,
  i == j,  /* condition */
  {   /* pre */
  a = malloc(16);
  memset(a, 0, 16);
  },
  {   /* post */
  free(a);
  int dummy1;
  },
  a
 );
  return 0;
  }

But changing tracepoint_cond to this breaks it:

  tracepoint_cond(provider, name,
  i == j,  /* condition */
  {   /* pre */
  a = malloc(16);
  memset(a, 0, 16);
  },
  {   /* post */
  free(a);
  int dummy1, dummy2;  /* XXX */
  },
  a
 );

Examining the pre-processed code reveals that the comma causes dummy2 to 
become a subsequent argument to the macro and land in the ... region, 
and hell breaks loose.


Do you think we could come up with a scheme that doesn't impose the use 
of statement-expressions, but supports them if they are provided? In my 
example, I simply provide 'i == j' as the condition, but a 
statement-expression would also work. Are we conflating separate ideas - 
the idea of user conditions and the idea of setup/cleanup?



While we are there, it would be good to make the pre handler test a
condition. For your use-case, you would just have to pass a pre :

  { printf(pre\n);  a = malloc(16); 1; }

So we could do:

if (pre)
 __tracepoint_cb_##provider##___##name(__VA_ARGS__);
(post)

and therefore support not only to setup something, but also to add an
extra check on whether tracing should be skipped or not. If someone
passes 1 as end of the compound statement, it will always evaluate to
1, and therefore the compiler will remove any extra branch in -O2.

One question stands though: how would we integrate this with sdt.h
STAP_PROVEV() ?

I'm yet to look at this.

Thanks,
Dave

--
The information contained in this transmission may be confidential. Any 
disclosure, copying, or further distribution of confidential information is not 
permitted unless such privilege is explicitly granted in writing by Quantum. 
Quantum reserves the right to have electronic communications, including email 
and attachments, sent across its networks filtered through anti virus and spam 
software programs and retain such messages in order to

Re: [lttng-dev] [URCU pull request review] urcu improvements

[lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

Re: [lttng-dev] [PATCH lttng-modules] Update kernel probes to more detailed match to kernel versions

Re: [lttng-dev] Request change name of function lookup_enum in libbabeltrace to make GDB use this lib

[lttng-dev] [PATCH urcu] wfstack: implement nonblocking pop and next

Re: [lttng-dev] Test if tracepoint is enabled

6 matches

Site Navigation

Mail list logo

Footer information