headers
Turgis, Frederic | 1 May 2012 22:09
Picon
Favicon

Tune reader_thread poll timeout value

Hi,

I am following-up remark made in the context of "Making the transport layer more robust" topic. Goal is to
reduce number of systemtap wake-ups in the context of embedded low-power use cases like MP3 playback.
The last non tunable or small duration source of wake-up is "reader_thread" ppoll timeout value, every
200ms. We compile systemtap with a value of 2s or more for our daily use.

So I am proposing to introduce tunables below as an example. Don't know if I should have correlated s and ns,
also don't know if this should also be applied to relay_old.c. Please tell me if this is an acceptable change:

From 3fac053713be44c00e05423b4f31e2ed8edaa993 Mon Sep 17 00:00:00 2001
From: Frederic Turgis <f-turgis <at> ti.com>
Date: Wed, 2 May 2012 01:16:28 +0200
Subject: [PATCH] Make reader_thread poll timeout tunable at compilation

---
 runtime/staprun/relay.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/runtime/staprun/relay.c b/runtime/staprun/relay.c
index e08ff60..d81a45f 100644
--- a/runtime/staprun/relay.c
+++ b/runtime/staprun/relay.c
 <at>  <at>  -12,6 +12,13  <at>  <at> 

 #include "staprun.h"

+#ifndef STP_RELAY_TIMEOUT_S
+#define STP_RELAY_TIMEOUT_S 0
+#endif
(Continue reading)

Permalink | Reply | 13 comments |
headers
Frank Ch. Eigler | 1 May 2012 23:30
Picon
Favicon
Gravatar

Re: Tune reader_thread poll timeout value


f-turgis wrote:

> [...]  The last non tunable or small duration source of wake-up is
> "reader_thread" ppoll timeout value, every 200ms. We compile
> systemtap with a value of 2s or more for our daily use.

Do you notice any adverse effects from this?  For an output-heavy
script for example, does this cause more out-of-buffer conditions?

> So I am proposing to introduce tunables below as an example. Don't
> know if I should have correlated s and ns, also don't know if this
> should also be applied to relay_old.c. Please tell me if this is an
> acceptable change:

Looks OK, though I would rather have just one configurable like
   STP_RELAY_TIMEOUT_MS
from which the .tv_sec and .tv_nsec values are calculated.

(Ideally, a baby PLL could regulate this timeout dynamically, adjusted
with the actual ebb and flow of tracing traffic.)

- FChE
Permalink | Reply | 0 comments |
headers
Mark Wielaard | 2 May 2012 10:46
Picon
Favicon

Re: Tune reader_thread poll timeout value

On Tue, 2012-05-01 at 20:09 +0000, Turgis, Frederic wrote:
> The last non tunable or small duration source of wake-up is
> "reader_thread" ppoll timeout value, every 200ms. We compile systemtap
> with a value of 2s or more for our daily use.
> 
> So I am proposing to introduce tunables below as an example. Don't
> know if I should have correlated s and ns, also don't know if this
> should also be applied to relay_old.c. Please tell me if this is an
> acceptable change:

I agree with Frank that it would be nicer to have one "tunable" instead
of two. Also don't forget to add some small documentation to stap.1
describing the default value and implications of tuning it higher/lower.

Thanks,

Mark
Permalink | Reply | 11 comments |
headers
adrian.m.negreanu at intel dot com | 2 May 2012 15:21
Favicon

[Bug runtime/14026] print_ubacktrace doesn't resolve the symbol name

http://sourceware.org/bugzilla/show_bug.cgi?id=14026

--- Comment #6 from Negreanu Adrian <adrian.m.negreanu at intel dot com> 2012-05-02 13:21:55 UTC ---
The patch below made it work for me:

diff --git a/runtime/unwind.c b/runtime/unwind.c
index e440177..3ac7f8d 100644
--- a/runtime/unwind.c
+++ b/runtime/unwind.c
 <at>  <at>  -610,7 +610,7  <at>  <at>  static int processCFI(const u8 *start, const u8 *end,
unsigned long targetLoc,
                        break;
                }
                dbug_unwind(1, "targetLoc=%lx state->loc=%lx\n", targetLoc,
state->loc);
-               if (ptr.p8 > end)
+               if (ptr.p8 >= end)
                        result = 0;
                if (result && targetLoc != 0 && targetLoc < state->loc)
                        return 1;

--

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
Permalink | Reply | 0 comments |
headers
mjw at redhat dot com | 2 May 2012 16:05
Favicon

[Bug runtime/14026] print_ubacktrace doesn't resolve the symbol name

http://sourceware.org/bugzilla/show_bug.cgi?id=14026

--- Comment #7 from Mark Wielaard <mjw at redhat dot com> 2012-05-02 14:05:25 UTC ---
(In reply to comment #6)
> The patch below made it work for me:
> 
> 
> diff --git a/runtime/unwind.c b/runtime/unwind.c
> index e440177..3ac7f8d 100644
> --- a/runtime/unwind.c
> +++ b/runtime/unwind.c
>  <at>  <at>  -610,7 +610,7  <at>  <at>  static int processCFI(const u8 *start, const u8 *end,
> unsigned long targetLoc,
>                         break;
>                 }
>                 dbug_unwind(1, "targetLoc=%lx state->loc=%lx\n", targetLoc,
> state->loc);
> -               if (ptr.p8 > end)
> +               if (ptr.p8 >= end)
>                         result = 0;
>                 if (result && targetLoc != 0 && targetLoc < state->loc)
>                         return 1;

I am not sure that is correct. In principle ptr being equal to end should be
fine (see also the last return condition in ProcessCFI). It means we read
everything, the last instruction and arguments (and nothing more).

If you hit this case then processCFI () will fail, which will make unwind_frame
() also fail. So I am somewhat surprised this seems to work for you. Do you
have debug output for before/after this patch?
(Continue reading)

Permalink | Reply | 0 comments |
headers
adrian.m.negreanu at intel dot com | 2 May 2012 16:29
Favicon

[Bug runtime/14026] print_ubacktrace doesn't resolve the symbol name

http://sourceware.org/bugzilla/show_bug.cgi?id=14026

--- Comment #8 from Negreanu Adrian <adrian.m.negreanu at intel dot com> 2012-05-02 14:29:53 UTC ---
00000040 00000014 00000000 CIE
  Version:               1
  Augmentation:          "zR"
  Code alignment factor: 1
  Data alignment factor: -4
  Return address column: 8
  Augmentation data:     1b

  DW_CFA_def_cfa: r4 (esp) ofs 4
  DW_CFA_offset: r8 (eip) at cfa-4
  DW_CFA_nop
  DW_CFA_nop

--

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
Permalink | Reply | 0 comments |
headers
adrian.m.negreanu at intel dot com | 2 May 2012 16:32
Favicon

[Bug runtime/14026] print_ubacktrace doesn't resolve the symbol name

http://sourceware.org/bugzilla/show_bug.cgi?id=14026

--- Comment #9 from Negreanu Adrian <adrian.m.negreanu at intel dot com> 2012-05-02 14:32:43 UTC ---
(gdb) disassemble 0x8074810
Dump of assembler code for function chvt_reinit:
   0x08074810 <+0>:     push   %ebp
   0x08074811 <+1>:     mov    %esp,%ebp
   0x08074813 <+3>:     lea    -0x8(%esp),%esp
   0x08074817 <+7>:     mov    %ebx,(%esp)
   0x0807481a <+10>:    mov    %esi,0x4(%esp)
   0x0807481e <+14>:    call   0x8049fc5 <__x86.get_pc_thunk.bx>
   0x08074823 <+19>:    add    $0x1a559,%ebx
   0x08074829 <+25>:    call   0x8049ac0 <getpid <at> plt>
   0x0807482e <+30>:    mov    0x1e0(%ebx),%edx
   0x08074834 <+36>:    mov    0x14c(%ebx),%esi
   0x0807483a <+42>:    mov    %eax,0x14(%edx)
   0x0807483d <+45>:    mov    %eax,0x4(%esi)
   0x08074840 <+48>:    call   0x8049a70 <geteuid <at> plt>
   0x08074845 <+53>:    mov    %eax,0xc(%esi)
   0x08074848 <+56>:    call   0x8049bf0 <getpgrp <at> plt>
   0x0807484d <+61>:    mov    %eax,0x8(%esi)
   0x08074850 <+64>:    call   0x8049d80 <getppid <at> plt>
   0x08074855 <+69>:    mov    %eax,0x10(%esi)
   0x08074858 <+72>:    mov    (%esp),%ebx
   0x0807485b <+75>:    mov    0x4(%esp),%esi
   0x0807485f <+79>:    mov    %ebp,%esp
   0x08074861 <+81>:    pop    %ebp
   0x08074862 <+82>:    ret    
End of assembler dump.
(Continue reading)

Permalink | Reply | 0 comments |
headers
adrian.m.negreanu at intel dot com | 2 May 2012 16:44
Favicon

[Bug runtime/14026] print_ubacktrace doesn't resolve the symbol name

http://sourceware.org/bugzilla/show_bug.cgi?id=14026

--- Comment #10 from Negreanu Adrian <adrian.m.negreanu at intel dot com> 2012-05-02 14:44:02 UTC ---
(In reply to comment #7)
> (In reply to comment #6)
> > The patch below made it work for me:
> > 
> > 
> > diff --git a/runtime/unwind.c b/runtime/unwind.c
> > index e440177..3ac7f8d 100644
> > --- a/runtime/unwind.c
> > +++ b/runtime/unwind.c
> >  <at>  <at>  -610,7 +610,7  <at>  <at>  static int processCFI(const u8 *start, const u8 *end,
> > unsigned long targetLoc,
> >                         break;
> >                 }
> >                 dbug_unwind(1, "targetLoc=%lx state->loc=%lx\n", targetLoc,
> > state->loc);
> > -               if (ptr.p8 > end)
> > +               if (ptr.p8 >= end)
> >                         result = 0;
> >                 if (result && targetLoc != 0 && targetLoc < state->loc)
> >                         return 1;
> 
> I am not sure that is correct. In principle ptr being equal to end should be
> fine (see also the last return condition in ProcessCFI). It means we read
> everything, the last instruction and arguments (and nothing more).
> 
> If you hit this case then processCFI () will fail, which will make unwind_frame
> () also fail. So I am somewhat surprised this seems to work for you. Do you
(Continue reading)

Permalink | Reply | 0 comments |
headers
mjw at redhat dot com | 2 May 2012 16:49
Favicon

[Bug runtime/14026] print_ubacktrace doesn't resolve the symbol name

http://sourceware.org/bugzilla/show_bug.cgi?id=14026

--- Comment #11 from Mark Wielaard <mjw at redhat dot com> 2012-05-02 14:49:11 UTC ---
And the FDE as discussed on irc:

<groleo>  8302 00003dd4 00000024 00003d98 FDE cie=00000040
pc=08074810..08074863
<groleo>  8303   DW_CFA_advance_loc: 1 to 08074811
<groleo>  8304   DW_CFA_def_cfa_offset: 8
<groleo>  8305   DW_CFA_advance_loc: 2 to 08074813
<groleo>  8306   DW_CFA_offset: r5 (ebp) at cfa-8
<groleo>  8307   DW_CFA_def_cfa_register: r5 (ebp)
<groleo>  8308   DW_CFA_advance_loc: 22 to 08074829
<groleo>  8309   DW_CFA_offset: r6 (esi) at cfa-12

--

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
Permalink | Reply | 0 comments |
headers
mjw at redhat dot com | 2 May 2012 17:34
Favicon

[Bug runtime/14026] print_ubacktrace doesn't resolve the symbol name

http://sourceware.org/bugzilla/show_bug.cgi?id=14026

--- Comment #12 from Mark Wielaard <mjw at redhat dot com> 2012-05-02 15:34:20 UTC ---
I am wondering whether we are using the data_alignment correctly.

Currently we are adjusting the offset in unwind_frame. Assuming
state->dataAlign always has to be used. But the DWARF spec says that some
DW_CFA operations (DW_CFA_def_cfa and DW_CFA_def_cfa_offset) take a
non-factored offset. So we should factor the offset in processCFI() instead
only when needed and not adjust in unwind_frame.

--

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
Permalink | Reply | 0 comments |

Gmane