History

xarray: Fix early termination of xas_for_each_marked

xas_for_each_marked() is using entry == NULL as a termination condition of the iteration. When xas_for_each_marked() is used protected only by RCU, this can however race with xas_store(xas, NULL) in the following way: TASK1 TASK2 page_cache_delete() find_get_pages_range_tag() xas_for_each_marked() xas_find_marked() off = xas_find_chunk() xas_store(&xas, NULL) xas_init_marks(&xas); ... rcu_assign_pointer(*slot, NULL); entry = xa_entry(off); And thus xas_for_each_marked() terminates prematurely possibly leading to missed entries in the iteration (translating to missing writeback of some pages or a similar problem). If we find a NULL entry that has been marked, skip it (unless we're trying to allocate an entry).

Matthew Wilcox (Oracle) committed 6 years ago

Commit 7e934cf1 parent 34eee836 files · +98 −3Browse files

Unified · Split

6 files
  1. include/linux/xarray.h +5 −1
  2. lib/xarray.c +2 −0
  3. tools/testing/radix-tree/Makefile +2 −2
  4. tools/testing/radix-tree/iteration_check_2.c +87 −0
  5. tools/testing/radix-tree/main.c +1 −0
  6. tools/testing/radix-tree/test.h +1 −0
include/linux/xarray.h+5−1
@@ -1648,6 +1648,7 @@ static inline void *xas_next_marked(struct xa_state *xas, unsigned long max, · 1647 unmodified lines
16481648 xa_mark_t mark)
16491649 {
16501650 struct xa_node *node = xas->xa_node;
1651+ void *entry;
16511652 unsigned int offset;
16521653
16531654 if (unlikely(xas_not_node(node) || node->shift))
@@ -1659,7 +1660,10 @@ static inline void *xas_next_marked(struct xa_state *xas, unsigned long max, · 5 unmodified lines
16591660 return NULL;
16601661 if (offset == XA_CHUNK_SIZE)
16611662 return xas_find_marked(xas, max, mark);
1662− return xa_entry(xas->xa, node, offset);
1663+ entry = xa_entry(xas->xa, node, offset);
1664+ if (!entry)
1665+ return xas_find_marked(xas, max, mark);
1666+ return entry;
16631667 }
16641668
16651669 /*
lib/xarray.c+2−0
@@ -1208,6 +1208,8 @@ void *xas_find_marked(struct xa_state *xas, unsigned long max, xa_mark_t mark) · 1207 unmodified lines
12081208 }
12091209
12101210 entry = xa_entry(xas->xa, xas->xa_node, xas->xa_offset);
1211+ if (!entry && !(xa_track_free(xas->xa) && mark == XA_FREE_MARK))
1212+ continue;
12111213 if (!xa_is_node(entry))
12121214 return entry;
12131215 xas->xa_node = xa_to_node(entry);
tools/testing/radix-tree/Makefile+2−2
@@ -7,8 +7,8 @@ LDLIBS+= -lpthread -lurcu · 6 unmodified lines
77 TARGETS = main idr-test multiorder xarray
88 CORE_OFILES := xarray.o radix-tree.o idr.o linux.o test.o find_bit.o bitmap.o
99 OFILES = main.o $(CORE_OFILES) regression1.o regression2.o regression3.o \
10− regression4.o \
11− tag_check.o multiorder.o idr-test.o iteration_check.o benchmark.o
10+ regression4.o tag_check.o multiorder.o idr-test.o iteration_check.o \
11+ iteration_check_2.o benchmark.o
1212
1313 ifndef SHIFT
1414 SHIFT=3
tools/testing/radix-tree/iteration_check_2.c+87−0
@@ -0,0 +1,87 @@
1+// SPDX-License-Identifier: GPL-2.0-or-later
2+/*
3+ * iteration_check_2.c: Check that deleting a tagged entry doesn't cause
4+ * an RCU walker to finish early.
5+ * Copyright (c) 2020 Oracle
6+ * Author: Matthew Wilcox <willy@infradead.org>
7+ */
8+#include <pthread.h>
9+#include "test.h"
10+
11+static volatile bool test_complete;
12+
13+static void *iterator(void *arg)
14+{
15+ XA_STATE(xas, arg, 0);
16+ void *entry;
17+
18+ rcu_register_thread();
19+
20+ while (!test_complete) {
21+ xas_set(&xas, 0);
22+ rcu_read_lock();
23+ xas_for_each_marked(&xas, entry, ULONG_MAX, XA_MARK_0)
24+ ;
25+ rcu_read_unlock();
26+ assert(xas.xa_index >= 100);
27+ }
28+
29+ rcu_unregister_thread();
30+ return NULL;
31+}
32+
33+static void *throbber(void *arg)
34+{
35+ struct xarray *xa = arg;
36+
37+ rcu_register_thread();
38+
39+ while (!test_complete) {
40+ int i;
41+
42+ for (i = 0; i < 100; i++) {
43+ xa_store(xa, i, xa_mk_value(i), GFP_KERNEL);
44+ xa_set_mark(xa, i, XA_MARK_0);
45+ }
46+ for (i = 0; i < 100; i++)
47+ xa_erase(xa, i);
48+ }
49+
50+ rcu_unregister_thread();
51+ return NULL;
52+}
53+
54+void iteration_test2(unsigned test_duration)
55+{
56+ pthread_t threads[2];
57+ DEFINE_XARRAY(array);
58+ int i;
59+
60+ printv(1, "Running iteration test 2 for %d seconds\n", test_duration);
61+
62+ test_complete = false;
63+
64+ xa_store(&array, 100, xa_mk_value(100), GFP_KERNEL);
65+ xa_set_mark(&array, 100, XA_MARK_0);
66+
67+ if (pthread_create(&threads[0], NULL, iterator, &array)) {
68+ perror("create iterator thread");
69+ exit(1);
70+ }
71+ if (pthread_create(&threads[1], NULL, throbber, &array)) {
72+ perror("create throbber thread");
73+ exit(1);
74+ }
75+
76+ sleep(test_duration);
77+ test_complete = true;
78+
79+ for (i = 0; i < 2; i++) {
80+ if (pthread_join(threads[i], NULL)) {
81+ perror("pthread_join");
82+ exit(1);
83+ }
84+ }
85+
86+ xa_destroy(&array);
87+}
tools/testing/radix-tree/main.c+1−0
@@ -311,6 +311,7 @@ int main(int argc, char **argv) · 310 unmodified lines
311311 regression4_test();
312312 iteration_test(0, 10 + 90 * long_run);
313313 iteration_test(7, 10 + 90 * long_run);
314+ iteration_test2(10 + 90 * long_run);
314315 single_thread_tests(long_run);
315316
316317 /* Free any remaining preallocated nodes */
tools/testing/radix-tree/test.h+1−0
@@ -34,6 +34,7 @@ void xarray_tests(void); · 33 unmodified lines
3434 void tag_check(void);
3535 void multiorder_checks(void);
3636 void iteration_test(unsigned order, unsigned duration);
37+void iteration_test2(unsigned duration);
3738 void benchmark(void);
3839 void idr_checks(void);
3940 void ida_tests(void);