Skip to content

Commit

Permalink
radix tree: Don't return retry entries from lookup
Browse files Browse the repository at this point in the history
Commit 66ee620 ("idr: Permit any valid kernel pointer to be stored")
changed the radix tree lookup so that it stops when reaching the bottom
of the tree.  However, the condition was added in the wrong place,
making it possible to return retry entries to the caller.  Reorder the
tests to check for the retry entry before checking whether we're at the
bottom of the tree.  The retry entry should never be found in the tree
root, so it's safe to defer the check until the end of the loop.

Add a regression test to the test-suite to be sure this doesn't come
back.

Fixes: 66ee620 ("idr: Permit any valid kernel pointer to be stored")
Reported-by: Greg Kurz <[email protected]>
Signed-off-by: Matthew Wilcox <[email protected]>
  • Loading branch information
Matthew Wilcox committed Dec 6, 2018
1 parent cf76c36 commit eff3860
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/radix-tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -784,11 +784,11 @@ void *__radix_tree_lookup(const struct radix_tree_root *root,
while (radix_tree_is_internal_node(node)) {
unsigned offset;

if (node == RADIX_TREE_RETRY)
goto restart;
parent = entry_to_node(node);
offset = radix_tree_descend(parent, &node, index);
slot = parent->slots + offset;
if (node == RADIX_TREE_RETRY)
goto restart;
if (parent->shift == 0)
break;
}
Expand Down
1 change: 1 addition & 0 deletions tools/testing/radix-tree/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ LDLIBS+= -lpthread -lurcu
TARGETS = main idr-test multiorder xarray
CORE_OFILES := xarray.o radix-tree.o idr.o linux.o test.o find_bit.o bitmap.o
OFILES = main.o $(CORE_OFILES) regression1.o regression2.o regression3.o \
regression4.o \
tag_check.o multiorder.o idr-test.o iteration_check.o benchmark.o

ifndef SHIFT
Expand Down
1 change: 1 addition & 0 deletions tools/testing/radix-tree/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ int main(int argc, char **argv)
regression1_test();
regression2_test();
regression3_test();
regression4_test();
iteration_test(0, 10 + 90 * long_run);
iteration_test(7, 10 + 90 * long_run);
single_thread_tests(long_run);
Expand Down
1 change: 1 addition & 0 deletions tools/testing/radix-tree/regression.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
void regression1_test(void);
void regression2_test(void);
void regression3_test(void);
void regression4_test(void);

#endif
79 changes: 79 additions & 0 deletions tools/testing/radix-tree/regression4.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/kernel.h>
#include <linux/gfp.h>
#include <linux/slab.h>
#include <linux/radix-tree.h>
#include <linux/rcupdate.h>
#include <stdlib.h>
#include <pthread.h>
#include <stdio.h>
#include <assert.h>

#include "regression.h"

static pthread_barrier_t worker_barrier;
static int obj0, obj1;
static RADIX_TREE(mt_tree, GFP_KERNEL);

static void *reader_fn(void *arg)
{
int i;
void *entry;

rcu_register_thread();
pthread_barrier_wait(&worker_barrier);

for (i = 0; i < 1000000; i++) {
rcu_read_lock();
entry = radix_tree_lookup(&mt_tree, 0);
rcu_read_unlock();
if (entry != &obj0) {
printf("iteration %d bad entry = %p\n", i, entry);
abort();
}
}

rcu_unregister_thread();

return NULL;
}

static void *writer_fn(void *arg)
{
int i;

rcu_register_thread();
pthread_barrier_wait(&worker_barrier);

for (i = 0; i < 1000000; i++) {
radix_tree_insert(&mt_tree, 1, &obj1);
radix_tree_delete(&mt_tree, 1);
}

rcu_unregister_thread();

return NULL;
}

void regression4_test(void)
{
pthread_t reader, writer;

printv(1, "regression test 4 starting\n");

radix_tree_insert(&mt_tree, 0, &obj0);
pthread_barrier_init(&worker_barrier, NULL, 2);

if (pthread_create(&reader, NULL, reader_fn, NULL) ||
pthread_create(&writer, NULL, writer_fn, NULL)) {
perror("pthread_create");
exit(1);
}

if (pthread_join(reader, NULL) || pthread_join(writer, NULL)) {
perror("pthread_join");
exit(1);
}

printv(1, "regression test 4 passed\n");
}

0 comments on commit eff3860

Please sign in to comment.