libbpf-tools/offcputime: Add multi process/thread support

[ekyooo]

This is a test example.

  # ./offcputime -p 16,48
    Tracing off-CPU time (us) of PID [16, 48]... Hit Ctrl-C to end.
        bpf_prog_a42aae11c0bc18f2_sched_switch
        bpf_prog_a42aae11c0bc18f2_sched_switch
        bpf_trace_run4
        __traceiter_sched_switch
        __schedule
        schedule
        worker_thread
        kthread
        ret_from_fork
        ret_from_fork_asm
        -                kworker/2:1 (48)
            3353019

        bpf_prog_a42aae11c0bc18f2_sched_switch
        bpf_prog_a42aae11c0bc18f2_sched_switch
        bpf_trace_run4
        __traceiter_sched_switch
        __schedule
        schedule
        rcu_gp_kthread
        kthread
        ret_from_fork
        ret_from_fork_asm
        -                rcu_preempt (16)
            1720974

[Bojun-Seo]

I think help message should be changed together.
For example, Trace this PID only doesn't explain the -p option any more after this patch merged.

[ekyooo]

I think help message should be changed together. For example, Trace this PID only doesn't explain the -p option any more after this patch merged.

Thank you for the review. It's fixed.

[yonghong-song]

Should we print both PID and TID in the above?

[ekyooo]

it's modified. thank you.

[yonghong-song]

Should we do both obj->rodata->filter_by_tgid and obj->rodata->filter_by_pid?

[ekyooo]

it's modified. thank you.

[yonghong-song]

Remove above 'else'?

[ekyooo]

Do you mean to support the use of the -p option and the -t option simultaneously?

[yonghong-song]

The original code does seem to support both pid and tgid though.

static bool allow_record(struct task_struct *t)
{       
        if (targ_tgid != -1 && targ_tgid != t->tgid)
                return false;
        if (targ_pid != -1 && targ_pid != t->pid)
                return false;
        if (user_threads_only && t->flags & PF_KTHREAD)
                return false;
        else if (kernel_threads_only && !(t->flags & PF_KTHREAD))
                return false;
        if (state != -1 && get_task_state(t) != state)
                return false;
        return true;
}

It is totally possible that user provides both tgid and pid where 'pid' is one of tasks under tgid. But in that case, the tracing is actually for 'pid'. So we can consider the tracing for 'pid' then.

So in the above, I guess we might want to do both. Please double check though.

[ekyooo]

Thank you for your detailed feedback.
I understood that you meant to output the OR result of the -p and -t options.

1. To clarify, the original code does not support the simultaneous use of both options. The allow_record function is a filtering function, so only threads that satisfy both tgid and pid conditions are recorded. When using -p and -t simultaneously, threads that satisfy only one of the two conditions are filtered out.
   For your information, this is the result of testing the simultaneous use of -p and -t without these patch sets.

// use -p option only
# ./offcputime -p 26209
Tracing off-CPU time (us)... Hit Ctrl-C to end.
^C    bpf_prog_efc0a21eee79af12_sched_switch
    bpf_prog_efc0a21eee79af12_sched_switch
    bpf_trace_run4
    __traceiter_sched_switch
    __schedule
    schedule
    irqentry_exit_to_user_mode
    asm_sysvec_apic_timer_interrupt
    strlen
    b
    a
    main
    __libc_start_main
    -                test-strlen-abc (26209)
        78

// use -t option only
# ./offcputime -t 16
Tracing off-CPU time (us)... Hit Ctrl-C to end.
^C    bpf_prog_efc0a21eee79af12_sched_switch
    bpf_prog_efc0a21eee79af12_sched_switch
    bpf_trace_run4
    __traceiter_sched_switch
    __schedule
    schedule
    schedule_timeout
    rcu_gp_fqs_loop
    rcu_gp_kthread
    kthread
    ret_from_fork
    ret_from_fork_asm
    -                rcu_preempt (16)
        4005

// use -p and -t option together
# ./offcputime -p 26209 -t 16
Tracing off-CPU time (us)... Hit Ctrl-C to end.
#

This behavior is the same in offcputime.py, profile.py, and profile.c.

2. However, I agree with your opinion that we should support the simultaneous use of both options because it is more reasonable and accurate. The reason I checked your intention was to ensure the change, as it could also affect offcputime.py, profile.py, and profile.c.

Now, I have updated the patches to support the simultaneous use of both options.
Thank you for your good opinion, and please provide feedback if there is any miscommunication.

[ekyooo]

Ah, you meant to output the AND result of the -p and -t options?

[ekyooo]

The original code supports both PID and TGID using an AND condition, as you mentioned. Therefore, I have modified this patch to focus solely on multi-process/thread support without altering the AND rule.

However, as far as I know, many other tools like perf and SystemTap use an OR condition in these cases. It would be beneficial to discuss the rule for usability and consistency across tools.

Thank you.

[yonghong-song]

Since str_to_int() and str_to_long() are in trace_helpers.c, we should add 'error = 0;' before strtol(). It is possible that errno is not 0 before strtol() and if strtol() works perfectly, errno won't change and the return value will be non-zero.

[ekyooo]

Yes, it's right. It's fixed. Thank you very much.