Skip to content

Commit 1145abc

Browse files
committed
opal: make interval tree resilient to similar intervals
There are cases where the same interval may be in the tree multiple times. This generally isn't a problem when searching the tree but may cause issues when attempting to delete a particular registration from the tree. The issue is fixed by breaking a low value tie by checking the high value then the interval data. If the high, low, and data of a new insertion exactly matches an existing interval then an assertion is raised. Signed-off-by: Nathan Hjelm <[email protected]>
1 parent 9caf43a commit 1145abc

File tree

1 file changed

+42
-13
lines changed

1 file changed

+42
-13
lines changed

opal/class/opal_interval_tree.c

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* All rights reserved.
1313
* Copyright (c) 2015-2018 Los Alamos National Security, LLC. All rights
1414
* reserved.
15+
* Copyright (c) 2020 Google, LLC. All rights reserved.
1516
* $COPYRIGHT$
1617
*
1718
* Additional copyrights may follow
@@ -39,7 +40,7 @@ static opal_interval_tree_node_t *opal_interval_tree_next (opal_interval_tree_t
3940
opal_interval_tree_node_t *node);
4041
static opal_interval_tree_node_t * opal_interval_tree_find_node(opal_interval_tree_t *tree,
4142
uint64_t low, uint64_t high,
42-
bool exact, void *data);
43+
void *data);
4344

4445
static opal_interval_tree_node_t *left_rotate (opal_interval_tree_t *tree, opal_interval_tree_node_t *x);
4546
static opal_interval_tree_node_t *right_rotate (opal_interval_tree_t *tree, opal_interval_tree_node_t *x);
@@ -355,31 +356,54 @@ int opal_interval_tree_insert (opal_interval_tree_t *tree, void *value, uint64_t
355356
return OPAL_SUCCESS;
356357
}
357358

359+
static int opal_interval_tree_compare_node(opal_interval_tree_node_t *node, uint64_t low, uint64_t high, void *data) {
360+
if ((data && node->low == low && node->high == high && node->data == data) ||
361+
(!data && node->low <= low && node->high >= high)) {
362+
return 0;
363+
}
364+
if (node->low > low) {
365+
return -1;
366+
}
367+
if (node->low < low) {
368+
return 1;
369+
}
370+
if (node->high < high) {
371+
return -1;
372+
}
373+
if (node->high > high) {
374+
return 1;
375+
}
376+
if (node->data > data) {
377+
return -1;
378+
}
379+
return 1;
380+
}
381+
358382
static opal_interval_tree_node_t *opal_interval_tree_find_interval(opal_interval_tree_t *tree, opal_interval_tree_node_t *node, uint64_t low,
359-
uint64_t high, bool exact, void *data)
383+
uint64_t high, void *data)
360384
{
361385
if (node == &tree->nill) {
362386
return NULL;
363387
}
364388

365-
if (((exact && node->low == low && node->high == high) || (!exact && node->low <= low && node->high >= high)) &&
366-
(!data || node->data == data)) {
389+
int check = opal_interval_tree_compare_node(node, low, high, data);
390+
if (0 == check) {
367391
return node;
368392
}
369393

370-
if (low <= node->low) {
371-
return opal_interval_tree_find_interval (tree, node->left, low, high, exact, data);
394+
if (-1 == check) {
395+
return opal_interval_tree_find_interval (tree, node->left, low, high, data);
372396
}
373397

374-
return opal_interval_tree_find_interval (tree, node->right, low, high, exact, data);
398+
return opal_interval_tree_find_interval (tree, node->right, low, high, data);
375399
}
376400

377401
/* Finds the node in the tree based on the key and returns a pointer
378402
* to the node. This is a bit a code duplication, but this has to be fast
379403
* so we go ahead with the duplication */
380-
static opal_interval_tree_node_t *opal_interval_tree_find_node(opal_interval_tree_t *tree, uint64_t low, uint64_t high, bool exact, void *data)
404+
static opal_interval_tree_node_t *opal_interval_tree_find_node(opal_interval_tree_t *tree, uint64_t low, uint64_t high, void *data)
381405
{
382-
return opal_interval_tree_find_interval (tree, tree->root.left, low, high, exact, data);
406+
return opal_interval_tree_find_interval (tree, tree->root.left, low, high, data);
383407
}
384408

385409
void *opal_interval_tree_find_overlapping (opal_interval_tree_t *tree, uint64_t low, uint64_t high)
@@ -388,7 +412,7 @@ void *opal_interval_tree_find_overlapping (opal_interval_tree_t *tree, uint64_t
388412
opal_interval_tree_node_t *node;
389413

390414
token = opal_interval_tree_reader_get_token (tree);
391-
node = opal_interval_tree_find_node (tree, low, high, true, NULL);
415+
node = opal_interval_tree_find_node (tree, low, high, NULL);
392416
opal_interval_tree_reader_return_token (tree, token);
393417

394418
return node ? node->data : NULL;
@@ -536,7 +560,7 @@ int opal_interval_tree_delete (opal_interval_tree_t *tree, uint64_t low, uint64_
536560
opal_interval_tree_node_t *node;
537561

538562
opal_interval_tree_write_lock (tree);
539-
node = opal_interval_tree_find_node (tree, low, high, true, data);
563+
node = opal_interval_tree_find_node (tree, low, high, data);
540564
if (NULL == node) {
541565
opal_interval_tree_write_unlock (tree);
542566
return OPAL_ERR_NOT_FOUND;
@@ -618,18 +642,23 @@ static void opal_interval_tree_insert_node (opal_interval_tree_t *tree, opal_int
618642
node->right = nill;
619643

620644
/* find the leaf where we will insert the node */
645+
int check = -1;
621646
while (n != nill) {
647+
check = opal_interval_tree_compare_node(n, node->low, node->high, node->data);
648+
/* node already exists */
649+
assert (0 != check);
650+
622651
if (n->max < node->high) {
623652
n->max = node->high;
624653
}
625654

626655
parent = n;
627-
n = ((node->low < n->low) ? n->left : n->right);
656+
n = (-1 == check) ? n->left : n->right;
628657
assert (nill == n || n->parent == parent);
629658
}
630659

631660
/* place it on either the left or the right */
632-
if ((node->low < parent->low)) {
661+
if (-1 == check) {
633662
parent->left = node;
634663
} else {
635664
parent->right = node;

0 commit comments

Comments
 (0)