GNOME Bugzilla – Bug 759813
Add more SystemTap/DTrace probes for main context and GTask
Last modified: 2016-06-16 16:25:27 UTC
Patches coming. One open question is GLib’s stability policy for its tapsets: these patches add some extra parameters to some of the existing GMainContext probes (but don’t modify any of the existing variable names). SystemTap scripts written against the old probes should continue to work, although they would need to be recompiled by the stap-server (though this happens for every run anyway). I don’t know how it would affect DTrace. If such an API break is not desired, I can split out the (quite useful) extra parameters into new ‘*_full’ probes. It’ll be ugly, but won’t break tapset API.
Created attachment 317819 [details] [review] glib: Add more GLib main context SystemTap and DTrace probes Expand the set of available probes, and add a few more output parameters to some of the existing ones to make them more useful. I do not know if this breaks any existing stability guarantees for GLib’s SystemTap tapset, as it is effectively just adding some more local variables in the user’s probe.
Created attachment 317820 [details] [review] gio: Add SystemTap and DTrace probes for GTask This adds a basic tapset for GIO, covering various interesting parts of GTask.
Although thinking about it, perhaps it would be best to split up the GSourceFuncs parameters for g_source_new() and pass them separately, so that user probes can examine them (since I don’t think user probes have access to the definition of the GSourceFuncs struct).
Created attachment 317870 [details] [review] glib: Add more GLib main context SystemTap and DTrace probes Expand the set of available probes, and add a few more output parameters to some of the existing ones to make them more useful. I do not know if this breaks any existing stability guarantees for GLib’s SystemTap tapset, as it is effectively just adding some more local variables in the user’s probe.
Created attachment 317871 [details] [review] gio: Add SystemTap and DTrace probes for GTask This adds a basic tapset for GIO, covering various interesting parts of GTask.
Review of attachment 317870 [details] [review]: Ah, wow. Did you generate this somehow? Did you write any systemtap scripts which consume this? I thought SystemTap nowadays had decent support for userspace tracing, although I admit I need to try it... but if that's the case, do we really need all of these static probes?
Created attachment 319605 [details] [review] gio: Add SystemTap and DTrace probes for GTask This adds a basic tapset for GIO, covering various interesting parts of GTask.
(In reply to Colin Walters from comment #6) > Review of attachment 317870 [details] [review] [review]: > > Ah, wow. Did you generate this somehow? Did you write any systemtap > scripts which consume this? It was written by hand over a period of weeks. Consumed by the (fledgling) https://gitlab.com/pwithnall/dunfell. > I thought SystemTap nowadays had decent support for userspace tracing, > although I admit I need to try it... but if that's the case, do we really > need all of these static probes? I tried quite hard to get Dunfell to work with userspace tracing, but it either isn’t ready in F23, or I was doing it wrong. As it stands, you still need to be running the stap-server in normal mode to use these probes. Patch updated after pushing bug #744772.
I only briefly looked at dunfell (it requires a newer version of GLib than I have in RHEL Workstation 7.2), but userspace probing seems to work for me: #!/usr/bin/env stap probe begin { log("Probe running.") } probe process("/usr/lib64/libgio-2.0.so.0").function("g_task_thread_complete") { m = sprintf ("[%d] g_task_thread_complete(%u, object:%u)", pid(), $task, $task->source_object); log(m) }
Also, it's worth mentioning what you're working on is cool, and thanks for proposing these patches!
(In reply to Colin Walters from comment #9) > I only briefly looked at dunfell (it requires a newer version of GLib than I > have in RHEL Workstation 7.2), but userspace probing seems to work for me: Yeah, userspace probing works fine. Sorry, I misread what you wrote and thought you were referring to unprivileged dyninst probing, which currently doesn’t (for me): $ stap --unprivileged --dyninst ./test.stap -c "/usr/bin/true" --FATAL-- #68: Dyninst was unable to create the specified process --FATAL-- #68: create process failed bootstrap stapdyn: ERROR: Couldn't create the target process WARNING: /usr/bin/stapdyn exited with status: 1 Pass 5: run failed. [man error::pass5] So as things stand, we can probe userspace processes, but require stap-server to be running to do so. With dyninst, we shouldn’t require stap-server.
Can you attach test.stap so we're on the same page? Would you agree most of these static probes are just as easy to do via debuginfo? I don't understand actually why static probes would work without stap-server, but dyninst wouldn't. Well OK I guess I know why, the're probably totally different code paths as dyninst is attempting to retain some security. Have you tried filing a systemtap bug?
(In reply to Colin Walters from comment #12) > Can you attach test.stap so we're on the same page? probe begin { log("Probe running.") } > Would you agree most of these static probes are just as easy to do via > debuginfo? I’m not sure what you mean. > I don't understand actually why static probes would work without > stap-server, but dyninst wouldn't. Well OK I guess I know why, the're > probably totally different code paths as dyninst is attempting to retain > some security. > > Have you tried filing a systemtap bug? I haven’t had time to investigate properly and write up the bug, bug I do plan to. For the moment, stap-server works fine for my purposes, so I want to push on with Dunfell — but it would be a whole lot nicer if dyninst worked, so I do plan to investigate and file a bug later.
Created attachment 319976 [details] [review] glib: Add more GLib main context SystemTap and DTrace probes Expand the set of available probes, and add a few more output parameters to some of the existing ones to make them more useful. I do not know if this breaks any existing stability guarantees for GLib’s SystemTap tapset, as it is effectively just adding some more local variables in the user’s probe.
Created attachment 319977 [details] [review] gio: Add SystemTap and DTrace probes for GTask This adds a basic tapset for GIO, covering various interesting parts of GTask.
(Updated to add a missing user_string() call and a new glib_thread_spawned probe.)
(In reply to Philip Withnall from comment #13) > > Would you agree most of these static probes are just as easy to do via > > debuginfo? > > I’m not sure what you mean. Ah, systemtap has debuginfo probing support: https://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps Most of them could be done via function probe points using debuginfo, but some of the more important ones (such as GLIB_MAIN_[BEFORE|AFTER]_DISPATCH) would need to use line numbers, and hence would be very specific on the GLib version; and would have to be in-tree. In that case, it would be more maintainable to simple use probe points as in the patch.
Acked in person yesterday by desrt (while walking to dinner and desperately trying to avoid me), who volunteers to take all the blame if people don't like this.
Attachment 319976 [details] pushed as cfb6928 - glib: Add more GLib main context SystemTap and DTrace probes Attachment 319977 [details] pushed as 195a0cb - gio: Add SystemTap and DTrace probes for GTask
(In reply to Philip Withnall from comment #18) > Acked in person yesterday by desrt (while walking to dinner and desperately > trying to avoid me) I like your new strategy for patch review.
(In reply to Philip Withnall from comment #17) > and hence would be very specific on the GLib > version; and would have to be in-tree. In that case, it would be more > maintainable to simple use probe points as in the patch. That sounds reasonable to me. I'll hopefully get an excuse to revisit this again personally.