Skip to content

Commit cbf4100

Browse files
vedangpatel1rostedt
authored andcommitted
tracing: Add support to detect and avoid duplicates
A duplicate in the tracing_map hash table is when 2 different entries have the same key and, as a result, the key_hash. This is possible due to a race condition in the algorithm. This race condition is inherent to the algorithm and not a bug. This was fine because, until now, we were only interested in the sum of all the values related to a particular key (the duplicates are dealt with in tracing_map_sort_entries()). But, with the inclusion of variables[1], we are interested in individual values. So, it will not be clear what value to choose when there are duplicates. So, the duplicates need to be removed. The duplicates can occur in the code in the following scenarios: - A thread is in the process of adding a new element. It has successfully executed cmpxchg() and inserted the key. But, it is still not done acquiring the trace_map_elt struct, populating it and storing the pointer to the struct in the value field of tracing_map hash table. If another thread comes in at this time and wants to add an element with the same key, it will not see the current element and add a new one. - There are multiple threads trying to execute cmpxchg at the same time, one of the threads will succeed and the others will fail. The ones which fail will go ahead increment 'idx' and add a new element there creating a duplicate. This patch detects and avoids the first condition by asking the thread which detects the duplicate to loop one more time. There is also a possibility of infinite loop if the thread which is trying to insert goes to sleep indefinitely and the one which is trying to insert a new element detects a duplicate. Which is why, the thread loops for map_size iterations before returning NULL. The second scenario is avoided by preventing the threads which failed cmpxchg() from incrementing idx. This way, they will loop around and check if the thread which succeeded in executing cmpxchg() had the same key. [1] http://lkml.kernel.org/r/cover.1498510759.git.tom.zanussi@linux.intel.com Link: http://lkml.kernel.org/r/e178e89ec399240331d383bd5913d649713110f4.1516069914.git.tom.zanussi@linux.intel.com Signed-off-by: Vedang Patel <vedang.patel@intel.com> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
1 parent 442c948 commit cbf4100

1 file changed

Lines changed: 36 additions & 5 deletions

File tree

kernel/trace/tracing_map.c

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,9 @@ static inline struct tracing_map_elt *
414414
__tracing_map_insert(struct tracing_map *map, void *key, bool lookup_only)
415415
{
416416
u32 idx, key_hash, test_key;
417+
int dup_try = 0;
417418
struct tracing_map_entry *entry;
419+
struct tracing_map_elt *val;
418420

419421
key_hash = jhash(key, map->key_size, 0);
420422
if (key_hash == 0)
@@ -426,11 +428,33 @@ __tracing_map_insert(struct tracing_map *map, void *key, bool lookup_only)
426428
entry = TRACING_MAP_ENTRY(map->map, idx);
427429
test_key = entry->key;
428430

429-
if (test_key && test_key == key_hash && entry->val &&
430-
keys_match(key, entry->val->key, map->key_size)) {
431-
if (!lookup_only)
432-
atomic64_inc(&map->hits);
433-
return entry->val;
431+
if (test_key && test_key == key_hash) {
432+
val = READ_ONCE(entry->val);
433+
if (val &&
434+
keys_match(key, val->key, map->key_size)) {
435+
if (!lookup_only)
436+
atomic64_inc(&map->hits);
437+
return val;
438+
} else if (unlikely(!val)) {
439+
/*
440+
* The key is present. But, val (pointer to elt
441+
* struct) is still NULL. which means some other
442+
* thread is in the process of inserting an
443+
* element.
444+
*
445+
* On top of that, it's key_hash is same as the
446+
* one being inserted right now. So, it's
447+
* possible that the element has the same
448+
* key as well.
449+
*/
450+
451+
dup_try++;
452+
if (dup_try > map->map_size) {
453+
atomic64_inc(&map->drops);
454+
break;
455+
}
456+
continue;
457+
}
434458
}
435459

436460
if (!test_key) {
@@ -452,6 +476,13 @@ __tracing_map_insert(struct tracing_map *map, void *key, bool lookup_only)
452476
atomic64_inc(&map->hits);
453477

454478
return entry->val;
479+
} else {
480+
/*
481+
* cmpxchg() failed. Loop around once
482+
* more to check what key was inserted.
483+
*/
484+
dup_try++;
485+
continue;
455486
}
456487
}
457488

0 commit comments

Comments
 (0)