Inserting delays with systemtap

Inserting delays with systemtap

[Bryn M. Reeves]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Folks,

Recently I found it useful to use systemtap to add a configurable
delay inside a critical section of module code. This was done to widen
a (very difficult to hit) race window for testing/verification purposes.

I got this working with a bit of embedded C:

# cat cdc-acm-test.stp
#!/usr/bin/stap -DSTP_NO_OVERLOAD -g

%{
#include <linux/delay.h>
%}

function mdelay(ms:long) %{
 /* guru */
 mdelay(THIS->ms);
%}

probe begin {
 printf("%s\n", "cdc-acm-test: inserting delay probe in tasklet fn")
}

probe end {
 printf("%s\n", "cdc-acm-test: removing delay probe from tasklet fn")
 printf("%s\n", "cdc-acm-test: done")
}

probe module("cdc_acm").statement("*@drivers/usb/class/cdc-acm.c:341") {
 mdelay($1)
}

The DWARF probe is necessary as we need to land in the middle of a
section protected by a spinlock (that's taken from a tasklet without
disabling interrupts..).

Would it be worth having something like this in a tapset? The
requirement to use STP_NO_OVERLOAD for long-running delays (I needed
200ms to reliably hit the lockup in cdc-acm) might make this less
useful but I found the technique very useful for this particular case.

Cheers,
Bryn.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk9XP+MACgkQ6YSQoMYUY96kdgCfURomvfhypSukwT+DZ6eyvWip
+cYAnRTENEwT7IhWEVUdhsKq7AEJwNOY
=RWuQ
-----END PGP SIGNATURE-----

Re: Inserting delays with systemtap

[Frank Ch. Eigler]

bmr wrote:

> [...]
> %{
> #include <linux/delay.h>
> %}
>
> function mdelay(ms:long) %{
>  /* guru */
>  mdelay(THIS->ms);
> %}
> [...]
> Would it be worth having something like this in a tapset? 

Absolutely.  Would you mind writing it up as a patch against some new
file like .../tapset/guru-delay.stp?

> The requirement to use STP_NO_OVERLOAD for long-running delays [...]

... is a problem.  It may be possible to automagically disable
overload processing if any of these functions are used, by including:

%{
/* guru */
#undef STP_OVERLOAD
%}

in the tapset file.


- FChE

Re: Inserting delays with systemtap

[Bryn M. Reeves]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/07/2012 03:23 PM, Frank Ch. Eigler wrote:
> bmr wrote:
>> Would it be worth having something like this in a tapset?
> 
> Absolutely.  Would you mind writing it up as a patch against some
> new file like .../tapset/guru-delay.stp?

Happy to - I'll try to get something done today or tomorrow.

>> The requirement to use STP_NO_OVERLOAD for long-running delays
>> [...]
> 
> ... is a problem.  It may be possible to automagically disable 
> overload processing if any of these functions are used, by
> including:
> 
> %{ /* guru */ #undef STP_OVERLOAD %}

Ah, thanks - I tried adding -DSTP_NO_OVERLOAD to the shabang line
along with -g (which worked) but I appear to hit one of the safety
regexes and it's rejected:

# head -1 ./cdc-acm-test.stp
#!/usr/bin/stap -DSTP_NO_OVERLOAD -g

# ././cdc-acm-test.stp
ERROR: Safety pattern mismatch for -D parameter ('STP_NO_OVERLOAD -g'
vs. '^[a-z_][a-z_0-9]*(=-?[a-z_0-9]+)?$') rc=1

In some ways I didn't think this was so bad - I guess it's definitely
a feature you want to know what you're doing with!

Cheers,
Bryn.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk9Xfi4ACgkQ6YSQoMYUY95p/QCeOLMfiu2t1nruTJNKTo9GRM/h
GZMAoJPHyjCVOqdT8s6k6pccLXP/4iLg
=o/Pc
-----END PGP SIGNATURE-----

Re: Inserting delays with systemtap

[Frank Ch. Eigler]

Hi -

bmr wrote:
> [...]
> # head -1 ./cdc-acm-test.stp
> #!/usr/bin/stap -DSTP_NO_OVERLOAD -g
> 
> # ././cdc-acm-test.stp
> ERROR: Safety pattern mismatch for -D parameter ('STP_NO_OVERLOAD -g'
> vs. '^[a-z_][a-z_0-9]*(=-?[a-z_0-9]+)?$') rc=1

Nah, that's only because #! shebang lines are defined to pass a
solitary argument to the interpreter.  Note the "-g" in the quoted
string.

- FChE

Re: Inserting delays with systemtap

[Bryn M. Reeves]

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

On 03/07/2012 03:41 PM, Frank Ch. Eigler wrote:
> Nah, that's only because #! shebang lines are defined to pass a
> solitary argument to the interpreter.  Note the "-g" in the quoted
> string.

Gotcha - thanks. It does indeed work fine if I remove the -g and specify
it on the command line.

I've attached a patch that adds the guru_delay.stp patchset - I also
included a wrapper around udelay() in case anyone has a use for that
(it's too fine a granularity for my application but I can see other
instances where it could be useful).

Tested locally with a simplified version of the original script and
appears to work. Using -g is required as expected:

#!/usr/bin/stap
probe module("cdc_acm").statement("*@drivers/usb/class/cdc-acm.c:341") {
  mdelay($1)
}

Cheers,
Bryn.


[-- Attachment #2: 0001-Add-guru_delay.stp-to-tapset.patch --]
[-- Type: text/x-patch, Size: 1240 bytes --]

From a3df25a00d83314912bdd743080287618be7896d Mon Sep 17 00:00:00 2001
From: "Bryn M. Reeves" <bmr@redhat.com>
Date: Thu, 8 Mar 2012 17:10:37 +0000
Subject: [PATCH] Add guru_delay.stp to tapset

Adds systemtap mdelay(ms) and udelay(us) functions for inserting
controlled programmable delays at probe sites.
---
 tapset/guru-delay.stp |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)
 create mode 100644 tapset/guru-delay.stp

diff --git a/tapset/guru-delay.stp b/tapset/guru-delay.stp
new file mode 100644
index 0000000..ad3a6be
--- /dev/null
+++ b/tapset/guru-delay.stp
@@ -0,0 +1,28 @@
+// Copyright (C) 2102 Red Hat Inc., Bryn M. Reeves <bmr@redhat.com>
+//
+// This file is part of systemtap, and is free software.  You can
+// redistribute it and/or modify it under the terms of the GNU General
+// Public License (GPL); either version 2, or (at your option) any
+// later version.
+//
+
+// <tapsetdescription>
+// Programable delays at probe sites
+// </tapsetdescription>
+
+%{
+/* guru */
+#undef STP_OVERLOAD
+#include <linux/delay.h>
+%}
+
+function mdelay(ms:long) %{
+ /* guru */
+  mdelay(THIS->ms);
+%}
+
+function udelay(us:long) %{
+ /* guru */
+  udelay(THIS->us);
+%}
+
-- 
1.7.6.5

Re: Inserting delays with systemtap

[Josh Stone]

On 03/08/2012 09:17 AM, Bryn M. Reeves wrote:
> On 03/07/2012 03:41 PM, Frank Ch. Eigler wrote:
>> Nah, that's only because #! shebang lines are defined to pass a
>> solitary argument to the interpreter.  Note the "-g" in the quoted
>> string.
> 
> Gotcha - thanks. It does indeed work fine if I remove the -g and specify
> it on the command line.

You can also mash short options together, as long as only one has an
argument and is placed last, e.g.

#!/usr/bin/stap -gDSTP_NO_OVERLOAD

[PATCH] Re: Inserting delays with systemtap

[Bryn M. Reeves]

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

On 03/08/2012 05:17 PM, Bryn M. Reeves wrote:
> I've attached a patch that adds the guru_delay.stp patchset - I also
> included a wrapper around udelay() in case anyone has a use for that
> (it's too fine a granularity for my application but I can see other
> instances where it could be useful).

Here's one with less silly typos in it. Shouldn't try to finish patches
while meetings are still running..

Bryn.


[-- Attachment #2: 0001-Add-guru_delay.stp-to-tapset.patch --]
[-- Type: text/x-patch, Size: 1232 bytes --]

From c64762a11e41848e894b965cb99094886198b73d Mon Sep 17 00:00:00 2001
From: "Bryn M. Reeves" <bmr@redhat.com>
Date: Thu, 8 Mar 2012 17:41:17 +0000
Subject: [PATCH] Add guru_delay.stp to tapset

Adds systemtap mdelay(ms) and udelay(us) functions for inserting
programmable delays at probe sites.
---
 tapset/guru-delay.stp |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)
 create mode 100644 tapset/guru-delay.stp

diff --git a/tapset/guru-delay.stp b/tapset/guru-delay.stp
new file mode 100644
index 0000000..f66a024
--- /dev/null
+++ b/tapset/guru-delay.stp
@@ -0,0 +1,28 @@
+// Copyright (C) 2102 Red Hat Inc., Bryn M. Reeves <bmr@redhat.com>
+//
+// This file is part of systemtap, and is free software.  You can
+// redistribute it and/or modify it under the terms of the GNU General
+// Public License (GPL); either version 2, or (at your option) any
+// later version.
+//
+
+// <tapsetdescription>
+// Programmable delays at probe sites
+// </tapsetdescription>
+
+%{
+/* guru */
+#undef STP_OVERLOAD
+#include <linux/delay.h>
+%}
+
+function mdelay(ms:long) %{
+  /* guru */
+  mdelay(THIS->ms);
+%}
+
+function udelay(us:long) %{
+  /* guru */
+  udelay(THIS->us);
+%}
+
-- 
1.7.6.5