Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

utils: enable @k as an abbreviated form of @kernel #631

Merged
merged 1 commit into from
Mar 20, 2019

Conversation

DanielTimLee
Copy link
Contributor

this commit allows passing '@k' for kernel option as an abbreviated form of '@kernel'

created a util function for strstr(buf, '@k')

// before
$uftrace -K 2 -F sys_openat@kernel --force ./echo-sync
// after
$uftrace -K 2 -F sys_openat@k --force ./echo-sync

Signed-off-by: Daniel T. Lee [email protected]

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And you need to change setup_trigger() function to handle kernel filters/symbols separately.

@DanielTimLee
Copy link
Contributor Author

DanielTimLee commented Mar 6, 2019

I'm really sorry for the late response.

I might not following you, but the thing

change setup_trigger() function to handle kernel filters/symbols separately.

at here, the kernel filter/symbols you're mentioning about is it related to module in setup_trigger():utils/fitler.c?

uftrace/utils/filter.c

Lines 981 to 1017 in 2aac509

if (module) {
/* is it the main executable? */
if (!strncmp(module, basename(symtabs->filename),
strlen(module))) {
ret += add_trigger_entry(root, &symtabs->symtab,
&patt, &tr,
&symtabs->dinfo,
setting);
ret += add_trigger_entry(root, &symtabs->dsymtab,
&patt, &tr,
&symtabs->dinfo,
setting);
}
else if (!strcasecmp(module, "PLT")) {
ret = add_trigger_entry(root, &symtabs->dsymtab,
&patt, &tr,
&symtabs->dinfo,
setting);
}
else if (!strcasecmp(module, "kernel")) {
ret = add_trigger_entry(root, get_kernel_symtab(),
&patt, &tr,
&symtabs->dinfo,
setting);
}
else {
map = find_map_by_name(symtabs, module);
if (map) {
ret = add_trigger_entry(root, &map->symtab,
&patt, &tr,
&map->dinfo,
setting);
}
}
free(module);
}

(ps. I've updated code with 'comparing whole word' )

@namhyung
Copy link
Owner

Yes, module is a name of executable or library. The module name should come first after the @ sign but it can also have other parts like trigger actions.

@honggyukim
Copy link
Collaborator

It's not because of this change, but there is a different behavior between -F sys_write@kernel and -T sys_write@kernel,filter as follows:

$ uftrace -K 2 -F sys_write@kernel --force /usr/bin/uptime
 23:40:15 up 30 days, 12:51, 41 users,  load average: 0.34, 0.32, 0.19
# DURATION     TID     FUNCTION
 124.261 us [ 12374] | setlocale();
   2.400 us [ 12374] | bindtextdomain();
   1.667 us [ 12374] | textdomain();
   1.150 us [ 12374] | __cxa_atexit();
  15.070 us [ 12374] | getopt_long();
   2.416 ms [ 12374] | print_uptime();
   1.590 us [ 12374] | __fpending();
   1.086 us [ 12374] | ferror();
            [ 12374] | fclose() {
            [ 12374] |   sys_write() {
   0.870 us [ 12374] |     __fdget_pos();
  15.964 us [ 12374] |     vfs_write();
  33.307 us [ 12374] |   } /* sys_write */
  39.217 us [ 12374] | } /* fclose */
   0.207 us [ 12374] | __fpending();
   0.230 us [ 12374] | ferror();
   1.690 us [ 12374] | fclose();

Unlike -F option, -T sys_write@kernel,filter hides user functions as well.

$ uftrace -K 2 -T sys_write@kernel,filter --force /usr/bin/uptime
 23:40:28 up 30 days, 12:51, 41 users,  load average: 0.37, 0.32, 0.19
# DURATION     TID     FUNCTION
            [ 12409] | sys_write() {
   0.877 us [ 12409] |   __fdget_pos();
  12.154 us [ 12409] |   vfs_write();
  18.764 us [ 12409] | } /* sys_write */

@namhyung
Copy link
Owner

Yes, because the trigger also works for replay. Maybe it's better to remove reset_live_opts()?

this commit allows to pass '@k' for kernel option
as an abbreviated form of '@kernel'

created a util function for strstr(buf, '@k')

// before
$uftrace -K 2 -F sys_openat@kernel --force ./echo-sync
// after
$uftrace -K 2 -F sys_openat@k --force ./echo-sync

Signed-off-by: Daniel T. Lee <[email protected]>
@DanielTimLee
Copy link
Contributor Author

updated.

@honggyukim
Copy link
Collaborator

honggyukim commented Mar 14, 2019

Yes, because the trigger also works for replay. Maybe it's better to remove reset_live_opts()?

The replay output looks same if remove_live_opts() is not called as follows:

$ uftrace -K 2 -F sys_write@kernel --force /usr/bin/uptime
 14:54:18 up 34 days,  4:05, 46 users,  load average: 0.41, 0.18, 0.12
# DURATION     TID     FUNCTION
            [175126] | sys_write() {
   0.907 us [175126] |   __fdget_pos();
  20.483 us [175126] |   vfs_write();
  36.657 us [175126] | } /* sys_write */

$ uftrace -K 2 -T sys_write@kernel,filter --force /usr/bin/uptime
 14:54:26 up 34 days,  4:05, 46 users,  load average: 0.42, 0.19, 0.12
# DURATION     TID     FUNCTION
            [175156] | sys_write() {
   0.704 us [175156] |   __fdget_pos();
  22.287 us [175156] |   vfs_write();
  28.407 us [175156] | } /* sys_write */

@honggyukim
Copy link
Collaborator

I applied @DanielTimLee's update and the result looks good.

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants