public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Create a function that indents without the tid()
@ 2009-09-30 18:11 Breno Leitao
  2009-09-30 19:15 ` Frank Ch. Eigler
  0 siblings, 1 reply; 7+ messages in thread
From: Breno Leitao @ 2009-09-30 18:11 UTC (permalink / raw)
  To: systemtap

Actually indent_thread() is a very useful function, but
sometimes you're probing something that is not related to
thread, as an interrupt function, and if the application 
changes during the interrupt, the indentation gets confused.
For example: 

    0 swapper(0): -> neo_copy_data_from_queue_to_uart
    69 a.out(7659):   -> neo_parse_modem
    74 a.out(7659):    -> neo_param
    14 swapper(0): <- neo_copy_data_from_queue_to_uart
    83 a.out(7659):  -> jsm_carrier
    86 a.out(7659):  <- jsm_carrier
     0 swapper(0): -> neo_parse_modem
    94 a.out(7659): <- jsm_tty_set_termios
     8 swapper(0): <- neo_parse_modem

So, I decided to create a simpler function that doesn't
consider the thread.
 
---
diff --git a/tapset/indent.stp b/tapset/indent.stp
index dface78..ee4e7b5 100644
--- a/tapset/indent.stp
+++ b/tapset/indent.stp
@@ -20,3 +20,11 @@ function thread_indent (delta)
 {
   return _generic_indent (tid(), delta)  
 }
+
+function indent(delta){
+  _indent += delta
+
+  for (i=1; i<_indent; i++) r .= " "
+
+  return r
+}

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

* Re: [PATCH] Create a function that indents without the tid()
  2009-09-30 18:11 [PATCH] Create a function that indents without the tid() Breno Leitao
@ 2009-09-30 19:15 ` Frank Ch. Eigler
  2009-09-30 19:57   ` Breno Leitao
  0 siblings, 1 reply; 7+ messages in thread
From: Frank Ch. Eigler @ 2009-09-30 19:15 UTC (permalink / raw)
  To: Breno Leitao; +Cc: systemtap

Breno Leitao <leitao@linux.vnet.ibm.com> writes:

> Actually indent_thread() is a very useful function, but
> sometimes you're probing something that is not related to
> thread, as an interrupt function, and if the application 
> changes during the interrupt, the indentation gets confused.
> [...]

It gets a bit confusing to read, but it is correct in several senses.
(I wonder what ftrace pretty-printed function traces with SMP look
like.)

> [...]
> So, I decided to create a simpler function that doesn't
> consider the thread.

> +function indent(delta){
> +  _indent += delta
> +  for (i=1; i<_indent; i++) r .= " "
> +  return r
> +}

Perhaps, but why not just

function indent(delta) { return _generic_indent(0, delta) }

?

- FChE

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

* Re: [PATCH] Create a function that indents without the tid()
  2009-09-30 19:15 ` Frank Ch. Eigler
@ 2009-09-30 19:57   ` Breno Leitao
  2009-09-30 20:25     ` Frank Ch. Eigler
  0 siblings, 1 reply; 7+ messages in thread
From: Breno Leitao @ 2009-09-30 19:57 UTC (permalink / raw)
  To: Frank Ch. Eigler; +Cc: systemtap

Hi Frank.

Frank Ch. Eigler wrote:
>> So, I decided to create a simpler function that doesn't
>> consider the thread.
> 
>> +function indent(delta){
>> +  _indent += delta
>> +  for (i=1; i<_indent; i++) r .= " "
>> +  return r
>> +}
> 
> Perhaps, but why not just
> 
> function indent(delta) { return _generic_indent(0, delta) }
Well, it makes sense, and it was my first implementation, but later I found that
tid() can be 0, and then the space counter (_indent_counters[idx]) will be 
corrupted when running indent() and thread_indent() together.

So, that is why in my example I put the example that contains the first line as:

    0 swapper(0): -> neo_copy_data_from_queue_to_uart

So, we can assume the risk and implement as you proposed, assuming that no one will
use both function together.

Thanks for you review.

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

* Re: [PATCH] Create a function that indents without the tid()
  2009-09-30 19:57   ` Breno Leitao
@ 2009-09-30 20:25     ` Frank Ch. Eigler
  2009-09-30 20:55       ` Breno Leitao
  0 siblings, 1 reply; 7+ messages in thread
From: Frank Ch. Eigler @ 2009-09-30 20:25 UTC (permalink / raw)
  To: Breno Leitao; +Cc: systemtap

Breno Leitao <leitao@linux.vnet.ibm.com> writes:

> [...]
>> function indent(delta) { return _generic_indent(0, delta) }

> Well, it makes sense, and it was my first implementation, but later I found that
> tid() can be 0, and then the space counter (_indent_counters[idx]) will be 
> corrupted when running indent() and thread_indent() together. [...]

Well, ok, how about { return _generic_indent(-1, delta) } ?

- FChE

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

* Re: [PATCH] Create a function that indents without the tid()
  2009-09-30 20:25     ` Frank Ch. Eigler
@ 2009-09-30 20:55       ` Breno Leitao
  2009-09-30 21:00         ` Frank Ch. Eigler
  0 siblings, 1 reply; 7+ messages in thread
From: Breno Leitao @ 2009-09-30 20:55 UTC (permalink / raw)
  To: Frank Ch. Eigler; +Cc: systemtap

Frank Ch. Eigler wrote:
> Breno Leitao <leitao@linux.vnet.ibm.com> writes:
> 
>> [...]
>>> function indent(delta) { return _generic_indent(0, delta) }
> 
>> Well, it makes sense, and it was my first implementation, but later I found that
>> tid() can be 0, and then the space counter (_indent_counters[idx]) will be 
>> corrupted when running indent() and thread_indent() together. [...]
> 
> Well, ok, how about { return _generic_indent(-1, delta) } ?
Nice. It works. Silly me, I didn't think about negative indexes.

The only problem I see now, is that it will display the scheduled process, and
since the process names have different lengths, the alignment will be lost and 
 sometimes it's difficult to follow/read, as an example I just got.

534776 a.out(6928):    -> neo_set_no_input_flow_control
535094 swapper(0):     <- neo_parse_modem
535357 a.out(6928):    <- neo_set_no_input_flow_control

So, if this is not important, I can submit a new patch with the implementation as 
you just described.

Thanks

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

* Re: [PATCH] Create a function that indents without the tid()
  2009-09-30 20:55       ` Breno Leitao
@ 2009-09-30 21:00         ` Frank Ch. Eigler
  2009-10-01  1:26           ` Breno Leitao
  0 siblings, 1 reply; 7+ messages in thread
From: Frank Ch. Eigler @ 2009-09-30 21:00 UTC (permalink / raw)
  To: Breno Leitao; +Cc: systemtap

Hi -

On Wed, Sep 30, 2009 at 05:55:01PM -0300, Breno Leitao wrote:

> [...]  The only problem I see now, is that it will display the
> scheduled process [...]

How about generalizing _generic_indent to also take the 
execname()/tid() as incoming parameters:

function _generic_indent (idx, desc, delta) { 
 ...
  r = sprintf("%6d %s:", (ts - _indent_timestamps[idx]), desc)
 ...
}

function thread_indent (delta) 
{
  return _generic_indent (tid(), sprintf("%s(%d)", execname(), tid()), delta)  
}

function indent (delta) 
{
  return _generic_indent (-1, "", delta)  
}

- FChE

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

* Re: [PATCH] Create a function that indents without the tid()
  2009-09-30 21:00         ` Frank Ch. Eigler
@ 2009-10-01  1:26           ` Breno Leitao
  0 siblings, 0 replies; 7+ messages in thread
From: Breno Leitao @ 2009-10-01  1:26 UTC (permalink / raw)
  To: Frank Ch. Eigler; +Cc: systemtap

Frank,

Frank Ch. Eigler wrote:
> How about generalizing _generic_indent to also take the 
> execname()/tid() as incoming parameters:
Yes. It works perfectly as you told.

Here is the patch, with some basic documentation:

---
diff --git a/tapset/indent.stp b/tapset/indent.stp
index dface78..1dbbebd 100644
--- a/tapset/indent.stp
+++ b/tapset/indent.stp
@@ -1,6 +1,6 @@
 global _indent_counters, _indent_timestamps
 
-function _generic_indent (idx, delta) 
+function _generic_indent (idx, desc, delta) 
 {
   ts = __indent_timestamp ()
   if (! _indent_counters[idx]) _indent_timestamps[idx] = ts
@@ -9,14 +9,26 @@ function _generic_indent (idx, delta)
   x = _indent_counters[idx] + (delta > 0 ? delta : 0)
   _indent_counters[idx] += delta
 
-  r = sprintf("%6d %s(%d):", (ts - _indent_timestamps[idx]),
-                             execname(), tid())
+  r = sprintf("%6d %s:", (ts - _indent_timestamps[idx]), desc)
+
   for (i=1; i<x; i++) r .= " "
 
   return r
 }
 
+/**
+ * sfunction thread_indent - returns an amount of space with the current task information
+ * @delta: the amount of space added/removed for each call
+ */
 function thread_indent (delta) 
 {
-  return _generic_indent (tid(), delta)  
+  return _generic_indent (tid(), sprintf("%s(%d)", execname(), tid()), delta)
+}
+
+/**
+ * sfunction indent - returns an amount of space to indent
+ * @delta: the amount of space added/removed for each call
+ */
+function indent(delta){
+  return _generic_indent(-1, "",  delta)
 }

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

end of thread, other threads:[~2009-10-01  1:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-30 18:11 [PATCH] Create a function that indents without the tid() Breno Leitao
2009-09-30 19:15 ` Frank Ch. Eigler
2009-09-30 19:57   ` Breno Leitao
2009-09-30 20:25     ` Frank Ch. Eigler
2009-09-30 20:55       ` Breno Leitao
2009-09-30 21:00         ` Frank Ch. Eigler
2009-10-01  1:26           ` Breno Leitao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).