Skip to content

Commit d99317f

Browse files
committed
perf lock contention: Clear lock addr after use
It checks the current lock to calculated the delta of contention time. The address is saved in the tstamp map which is allocated at begining of contention and released at end of contention. But it's possible for bpf_map_delete_elem() to fail. In that case, the element in the tstamp map kept for the current lock and it makes the next contention for the same lock tracked incorrectly. Specificially the next contention begin will see the existing element for the task and it'd just return. Then the next contention end will see the element and calculate the time using the timestamp for the previous begin. This can result in a large value for two small contentions happened from time to time. Let's clear the lock address so that it can be updated next time even if the bpf_map_delete_elem() failed. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Acked-by: Ian Rogers <irogers@google.com> Cc: Hao Luo <haoluo@google.com> Cc: Song Liu <song@kernel.org> Cc: bpf@vger.kernel.org Link: https://lore.kernel.org/r/20231020204741.1869520-1-namhyung@kernel.org
1 parent e093a22 commit d99317f

1 file changed

Lines changed: 4 additions & 0 deletions

File tree

tools/perf/util/bpf_skel/lock_contention.bpf.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ int contention_end(u64 *ctx)
389389

390390
duration = bpf_ktime_get_ns() - pelem->timestamp;
391391
if ((__s64)duration < 0) {
392+
pelem->lock = 0;
392393
bpf_map_delete_elem(&tstamp, &pid);
393394
__sync_fetch_and_add(&time_fail, 1);
394395
return 0;
@@ -422,6 +423,7 @@ int contention_end(u64 *ctx)
422423
data = bpf_map_lookup_elem(&lock_stat, &key);
423424
if (!data) {
424425
if (data_map_full) {
426+
pelem->lock = 0;
425427
bpf_map_delete_elem(&tstamp, &pid);
426428
__sync_fetch_and_add(&data_fail, 1);
427429
return 0;
@@ -445,6 +447,7 @@ int contention_end(u64 *ctx)
445447
data_map_full = 1;
446448
__sync_fetch_and_add(&data_fail, 1);
447449
}
450+
pelem->lock = 0;
448451
bpf_map_delete_elem(&tstamp, &pid);
449452
return 0;
450453
}
@@ -458,6 +461,7 @@ int contention_end(u64 *ctx)
458461
if (data->min_time > duration)
459462
data->min_time = duration;
460463

464+
pelem->lock = 0;
461465
bpf_map_delete_elem(&tstamp, &pid);
462466
return 0;
463467
}

0 commit comments

Comments
 (0)