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

A suspicious multi-thread race condition of cfs-throttling-trace.bpf.c #442

Open
labilezhu opened this issue Jun 25, 2024 · 3 comments
Open

Comments

@labilezhu
Copy link

In examples/cfs-throttling-trace.bpf.c we use cgroup id as the key of traced_cgroups map :

SEC("usdt/./tracing/demos/cfs-throttling/demo:ebpf_exporter:cfs_set_parent_span")
int BPF_USDT(cfs_set_parent_span, u64 trace_id_hi, u64 trace_id_lo, u64 span_id)
{
    u32 cgroup = bpf_get_current_cgroup_id();
    struct span_parent_t parent = { .trace_id_hi = trace_id_hi, .trace_id_lo = trace_id_lo, .span_id = span_id };

    bpf_map_update_elem(&traced_cgroups, &cgroup, &parent, BPF_ANY);

    return 0;
}

When unthrottle, we use cgroup id as key to get from the map :

SEC("fentry/unthrottle_cfs_rq")
int BPF_PROG(unthrottle_cfs_rq, struct cfs_rq *cfs_rq)
{
    u32 cgroup = cfs_rq->tg->css.cgroup->kn->id;
    u64 throttled_ns = cfs_rq->rq->clock - cfs_rq->throttled_clock;
    struct span_parent_t *parent = bpf_map_lookup_elem(&traced_cgroups, &cgroup);

But in a multi-threaded environment. It seems not aways true. May be thread id and cgroup id be composited as key?

@bobrik
Copy link
Contributor

bobrik commented Jun 29, 2024

I'm not following. Could you show how you would change the code or describe the scenario where it's broken?

@labilezhu
Copy link
Author

I am not a kernel expert. As far as I know:

The ideal world is:
cfs-throttling-trace drawio

The real world is:
cfs-throttling-trace-realworld drawio

@bobrik
Copy link
Contributor

bobrik commented Jul 23, 2024

I see, you're saying that a throttle is associated with a whole cgroup and it's impossible to have two traces with the same throttle, even if it affects both. I don't think that keying by thread id would help here, as there might be multiple concurrent traces happening in the same thread as well.

The solution is probably to have a set of traces that are interested in throttle events. I'm not really sure what's the best way to implement this.

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

No branches or pull requests

2 participants