Reduction of guru use in systemtap war stories

Reduction of guru use in systemtap war stories

[William Cohen]

I am looking through the war stories and figuring out how to clean
things up.  Out of the 28 war stories 12 require guru mode.  Seems
like there should be fewer examples requiring the use of guru
mode. Three of the examples (WSimmortal, WSprintkBootParams, and
WSautoNice) are writing writing to kernel variables, which will always
require guru mode. WSPanicOnOom call kernel panic function. For the
remaining war stories using guru mode there a some idoms that are not
easily available from the tapsets.

WSthreadCPUshare uses guru mode to determine the instruction pointer
value for the sample and then determine whether this value is in
kernel or user-space. For some 32-bit kernels this approach is not
correct. A better approach would be make the kernel function
user_mode() available in the tapsets. This eliminate the need for guru
mode WSthreadCPUshare.

Some of the other war stories (WSPfiles, WSPsig, and WSPlimit) the
idiom to map a pid to a task struct point comes up and would be a
candidate to add to the tapsets. It looks like some of the other guru
functions in those scripts are already available and can be recast as
task.stp tapset functions. I will see if can reduce the scope of guru
functions used in these war stories examples.

Several of the war stories (such as WSPfiles) include header files and
make use of defines to extract bits/flags out of values. Seems like
there should be a cleaner way to make those bit values available to
scripts.

-Will

Re: Reduction of guru use in systemtap war stories

[Frank Ch. Eigler]

wcohen wrote:

> I am looking through the war stories and figuring out how to clean
> things up.  [...]

Thanks.

> WSthreadCPUshare uses guru mode to determine the instruction pointer
> value for the sample and then determine whether this value is in
> kernel or user-space. For some 32-bit kernels this approach is not
> correct. A better approach would be make the kernel function
> user_mode() available in the tapsets. [...]

Indeed, please make it so!

> Some of the other war stories (WSPfiles, WSPsig, and WSPlimit) the
> idiom to map a pid to a task struct point comes up and would be a
> candidate to add to the tapsets. It looks like some of the other guru
> functions in those scripts are already available and can be recast as
> task.stp tapset functions. [...]

See bug #6525 for a probably more robust way.  The one used in some of
that embedded-c code, find_task_by_pid(), is not safe to call from all
contexts.

> Several of the war stories (such as WSPfiles) include header files
> and make use of defines to extract bits/flags out of values. Seems
> like there should be a cleaner way to make those bit values
> available to scripts.

One possibility is to lobby the kernel to run gcc with -g3 or somesuch
to get it to dump macro definitions into the dwarf data; then we could
pull it out of there with $MACRO.  Sounds unlikely.

Other ideas?


- FChE

Re: Reduction of guru use in systemtap war stories

[William Cohen]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

Frank Ch. Eigler wrote:
> wcohen wrote:
> 
>> I am looking through the war stories and figuring out how to clean
>> things up.  [...]
> 
> Thanks.
> 
>> WSthreadCPUshare uses guru mode to determine the instruction pointer
>> value for the sample and then determine whether this value is in
>> kernel or user-space. For some 32-bit kernels this approach is not
>> correct. A better approach would be make the kernel function
>> user_mode() available in the tapsets. [...]
> 
> Indeed, please make it so!

Attached is a simple little patch for the context.stp tapset and documentation 
to "make it so." Assuming that it looks okay I will commit it.

> 
>> Some of the other war stories (WSPfiles, WSPsig, and WSPlimit) the
>> idiom to map a pid to a task struct point comes up and would be a
>> candidate to add to the tapsets. It looks like some of the other guru
>> functions in those scripts are already available and can be recast as
>> task.stp tapset functions. [...]
> 
> See bug #6525 for a probably more robust way.  The one used in some of
> that embedded-c code, find_task_by_pid(), is not safe to call from all
> contexts.

Thanks for the pointer to 6525.

>> Several of the war stories (such as WSPfiles) include header files
>> and make use of defines to extract bits/flags out of values. Seems
>> like there should be a cleaner way to make those bit values
>> available to scripts.
> 
> One possibility is to lobby the kernel to run gcc with -g3 or somesuch
> to get it to dump macro definitions into the dwarf data; then we could
> pull it out of there with $MACRO.  Sounds unlikely.

At the moment asking for -g3 sounds unlikely. The debuginfo is already large. 
Adding in all the macros is going to make the debug information even larger.

> Other ideas?

-Will


[-- Attachment #2: stap_user_mode.diff --]
[-- Type: text/plain, Size: 911 bytes --]

diff --git a/stapfuncs.5.in b/stapfuncs.5.in
index 6d4546a..d8a1a55 100644
--- a/stapfuncs.5.in
+++ b/stapfuncs.5.in
@@ -252,6 +252,9 @@ Return the probe point's module name, if known.
 target:long ()
 Return the pid of the target process.
 .TP
+user_mode:long ()
+Return 1 if the probe point occurred in user-mode.
+.TP
 is_return:long ()
 Return 1 if the probe point is a return probe.  Deprecated.
 
diff --git a/tapset/context.stp b/tapset/context.stp
index 45e3be3..d2195f8 100644
--- a/tapset/context.stp
+++ b/tapset/context.stp
@@ -140,6 +140,11 @@ function registers_valid:long () %{ /* pure */
 	THIS->__retvalue = (CONTEXT->regs != NULL);
 %}
 
+function user_mode:long () %{ /* pure */ /* currently a user-mode address? */
+  if (CONTEXT->regs)
+    THIS->__retvalue = (uint64_t) user_mode (CONTEXT->regs);
+%}
+
 function is_return:long () %{ /* pure */
 	if (CONTEXT->pi)
 		THIS->__retvalue = 1;

Re: Reduction of guru use in systemtap war stories

[William Cohen]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

William Cohen wrote:
> Frank Ch. Eigler wrote:
>> wcohen wrote:
>>
>>> I am looking through the war stories and figuring out how to clean
>>> things up.  [...]
>>
>> Thanks.
>>
>>> WSthreadCPUshare uses guru mode to determine the instruction pointer
>>> value for the sample and then determine whether this value is in
>>> kernel or user-space. For some 32-bit kernels this approach is not
>>> correct. A better approach would be make the kernel function
>>> user_mode() available in the tapsets. [...]
>>
>> Indeed, please make it so!
> 
> Attached is a simple little patch for the context.stp tapset and 
> documentation to "make it so." Assuming that it looks okay I will commit 
> it.

Frank pointed out that there were some probes contexts that CONTEXT->regs was 
not available, e.g. probe begin,end. The begin and end could be considered to be 
in kernel mode. The probe timer sets CONTEXT->regs (if it didn't, the original 
WSthreadCPUshare example wouldn't work). The revised version of the patch 
assumes that kernel mode if CONTEXT->regs isn't available.

-Will

[-- Attachment #2: stap_user_mode2.diff --]
[-- Type: text/plain, Size: 946 bytes --]

diff --git a/stapfuncs.5.in b/stapfuncs.5.in
index 6d4546a..d8a1a55 100644
--- a/stapfuncs.5.in
+++ b/stapfuncs.5.in
@@ -252,6 +252,9 @@ Return the probe point's module name, if known.
 target:long ()
 Return the pid of the target process.
 .TP
+user_mode:long ()
+Return 1 if the probe point occurred in user-mode.
+.TP
 is_return:long ()
 Return 1 if the probe point is a return probe.  Deprecated.
 
diff --git a/tapset/context.stp b/tapset/context.stp
index 45e3be3..f9ebf9c 100644
--- a/tapset/context.stp
+++ b/tapset/context.stp
@@ -140,6 +140,13 @@ function registers_valid:long () %{ /* pure */
 	THIS->__retvalue = (CONTEXT->regs != NULL);
 %}
 
+function user_mode:long () %{ /* pure */ /* currently a user-mode address? */
+  if (CONTEXT->regs)
+    THIS->__retvalue = (uint64_t) user_mode (CONTEXT->regs);
+  else
+    THIS->__retvalue = 0;
+%}
+
 function is_return:long () %{ /* pure */
 	if (CONTEXT->pi)
 		THIS->__retvalue = 1;

Re: Reduction of guru use in systemtap war stories

[Roland McGrath]

> > Several of the war stories (such as WSPfiles) include header files
> > and make use of defines to extract bits/flags out of values. Seems
> > like there should be a cleaner way to make those bit values
> > available to scripts.
> 
> One possibility is to lobby the kernel to run gcc with -g3 or somesuch
> to get it to dump macro definitions into the dwarf data; then we could
> pull it out of there with $MACRO.  Sounds unlikely.
> 
> Other ideas?

Could look into swig or perl's header-grovelling stuff or one of those
sorts of things.  Before pondering this too specifically, I want to ask
Will from his survey whether we are only concerned with constants, or also
function-like macros that contain something other that simple arithmetic.


Thanks,
Roland

Re: Reduction of guru use in systemtap war stories

[William Cohen]

Roland McGrath wrote:
>>> Several of the war stories (such as WSPfiles) include header files
>>> and make use of defines to extract bits/flags out of values. Seems
>>> like there should be a cleaner way to make those bit values
>>> available to scripts.
>> One possibility is to lobby the kernel to run gcc with -g3 or somesuch
>> to get it to dump macro definitions into the dwarf data; then we could
>> pull it out of there with $MACRO.  Sounds unlikely.
>>
>> Other ideas?
> 
> Could look into swig or perl's header-grovelling stuff or one of those
> sorts of things.  Before pondering this too specifically, I want to ask
> Will from his survey whether we are only concerned with constants, or also
> function-like macros that contain something other that simple arithmetic.
> 
> 
> Thanks,
> Roland

Hi Roland,

I don't have good statistics on the break down of simple constants expressions 
vs. actual code in the macros. Most of the uses appear to be just simple constants.

Running the kernel with -g3 to include macro information is going to make the 
debuginfo even larger. The debuginfo is already taking up too much space. On 
2.6.25.6-55.fc9.x86_64 the installed debuginfo rpms:

$ rpm -qa --queryformat '%{name} %{size}\n' |grep debuginfo|grep kernel
kernel-debuginfo 684495701
kernel-debuginfo-common 135176987


Thanks for the mention of swig and perl's header-grovelling stuff. I found some 
pointers to that material:

http://www.swig.org/

For perl's header-grovelling stuff talking about something like:

http://search.cpan.org/~rgarcia/perl-5.10.0/utils/h2xs.PL
http://search.cpan.org/~rgarcia/perl-5.10.0/utils/h2ph.PL
http://search.cpan.org/~icerider/C-Scan-Constants-1.017/lib/C/Scan/Constants.pm
http://search.cpan.org/~gbarr/Include-1.02a/Include.pm

-Will

Re: Reduction of guru use in systemtap war stories

[Roland McGrath]

> I don't have good statistics on the break down of simple constants
> expressions vs. actual code in the macros. Most of the uses appear to be
> just simple constants.

Ok.  That's what I guessed.

> Running the kernel with -g3 to include macro information is going to make the 
> debuginfo even larger. The debuginfo is already taking up too much space.

We'll have to see how it looks one day when we have DWARF compaction.
But yeah, it's big and seems impractical for the foreseeable future.
Especially when in most cases the macro source is already in kernel-devel
headers that we already have on hand without any debuginfo at all.


Thanks,
Roland