linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/30] Radix tree multiorder fixes
@ 2016-04-06 21:21 Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 01/30] radix-tree: Introduce radix_tree_empty Matthew Wilcox
                   ` (29 more replies)
  0 siblings, 30 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

I must apologise for commit f96d18ff84 which left the impression that
the support for multiorder radix tree entries was functional.  As soon as
Ross tried to use it, it became apparent that my testing was completely
inadequate, and it didn't even work a little bit for orders that were
not a multiple of shift.

This series of patches is the result of about 5 weeks of redesign,
reimplementation, testing, arguing and hair-pulling.  The great news is
that the test-suite is now far better than it was.  That's reflected in
the diffstat for the test-suite alone:
 12 files changed, 427 insertions(+), 28 deletions(-)

The highlight for users of the tree is that the restriction on the order
being >= RADIX_TREE_MAP_SHIFT is now gone; the radix tree now supports
any order between 0 and 64.

For those who are interested in how the tree works, patch 9 is probably
the most interesting one as it introduces the new machinery for handling
sibling entries.

I've tried to be fair in attributing authorship to the person who
contributed the majority of the code in each patch; Ross has been an
invaluable partner in the development of this support and it's fair to
say that each of us has code in every commit.

I should also express my appreciation of the 0day testing.  It prompted
me that I was bloating the tinyconfig in an unacceptable way, and it
bisected to a commit which contained a rahter nasty memory-corruption bug.

Matthew Wilcox (20):
  radix-tree: Introduce radix_tree_empty
  radix tree test suite: Fix build
  radix tree test suite: Add tests for radix_tree_locate_item()
  Introduce CONFIG_RADIX_TREE_MULTIORDER
  radix-tree: Add missing sibling entry functionality
  radix-tree: Fix sibling entry insertion
  radix-tree: Fix deleting a multi-order entry through an alias
  radix-tree: Remove restriction on multi-order entries
  radix-tree: Introduce radix_tree_load_root()
  radix-tree: Fix extending the tree for multi-order entries at offset 0
  radix-tree: Fix several shrinking bugs with multiorder entries
  radix tree test suite: Start adding multiorder tests
  radix-tree: Rewrite __radix_tree_lookup
  radix-tree: Fix multiorder BUG_ON in radix_tree_insert
  radix-tree: add support for multi-order iterating
  radix tree test suite: Add multiorder shrinking test
  radix-tree: Fix radix_tree_create for sibling entries
  radix-tree: Rewrite radix_tree_locate_item
  radix-tree: Fix two bugs in radix_tree_range_tag_if_tagged()
  radix-tree: Add copyright statements

Ross Zwisler (10):
  radix tree test suite: Allow testing other fan-out values
  radix tree test suite: keep regression test runs short
  radix tree test suite: rebuild when headers change
  radix-tree: remove unused looping macros
  radix tree test suite: multi-order iteration test
  radix-tree: Rewrite radix_tree_tag_set
  radix-tree: Rewrite radix_tree_tag_clear
  radix-tree: Rewrite radix_tree_tag_get
  radix-tree test suite: add multi-order tag test
  radix-tree: Fix radix_tree_dump() for multi-order entries

 include/linux/radix-tree.h                    | 103 +++--
 kernel/irq/irqdomain.c                        |   7 +-
 lib/Kconfig                                   |   3 +
 lib/radix-tree.c                              | 545 +++++++++++++++-----------
 mm/Kconfig                                    |   1 +
 tools/testing/radix-tree/Makefile             |   4 +-
 tools/testing/radix-tree/generated/autoconf.h |   3 +
 tools/testing/radix-tree/linux/init.h         |   0
 tools/testing/radix-tree/linux/kernel.h       |  15 +-
 tools/testing/radix-tree/linux/slab.h         |   1 -
 tools/testing/radix-tree/linux/types.h        |   7 +-
 tools/testing/radix-tree/main.c               |  71 +++-
 tools/testing/radix-tree/multiorder.c         | 317 +++++++++++++++
 tools/testing/radix-tree/regression2.c        |   7 -
 tools/testing/radix-tree/tag_check.c          |  10 +
 tools/testing/radix-tree/test.c               |  13 +-
 tools/testing/radix-tree/test.h               |   7 +-
 17 files changed, 807 insertions(+), 307 deletions(-)
 create mode 100644 tools/testing/radix-tree/generated/autoconf.h
 create mode 100644 tools/testing/radix-tree/linux/init.h
 create mode 100644 tools/testing/radix-tree/multiorder.c

-- 
2.8.0.rc3

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 01/30] radix-tree: Introduce radix_tree_empty
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 02/30] radix tree test suite: Fix build Matthew Wilcox
                   ` (28 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

The irqdomain code was checking for 0 or 1 entries, not 0 entries like
the comment said they were.  Introduce a new helper that will actually
check for an empty tree.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 include/linux/radix-tree.h | 5 +++++
 kernel/irq/irqdomain.c     | 7 +------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index 51a97ac8bfbf..83f708e5db59 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -136,6 +136,11 @@ do {									\
 	(root)->rnode = NULL;						\
 } while (0)
 
+static inline bool radix_tree_empty(struct radix_tree_root *root)
+{
+	return root->rnode == NULL;
+}
+
 /**
  * Radix-tree synchronization
  *
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 3a519a01118b..ba3f60d8df2f 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -139,12 +139,7 @@ void irq_domain_remove(struct irq_domain *domain)
 {
 	mutex_lock(&irq_domain_mutex);
 
-	/*
-	 * radix_tree_delete() takes care of destroying the root
-	 * node when all entries are removed. Shout if there are
-	 * any mappings left.
-	 */
-	WARN_ON(domain->revmap_tree.height);
+	WARN_ON(!radix_tree_empty(&domain->revmap_tree));
 
 	list_del(&domain->link);
 
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 02/30] radix tree test suite: Fix build
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 01/30] radix-tree: Introduce radix_tree_empty Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 03/30] radix tree test suite: Add tests for radix_tree_locate_item() Matthew Wilcox
                   ` (27 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

Add an empty linux/init.h, and definitions for a few parts of the kernel
API either in use now, or to be used in the near future.  Start using
the common definitions in tools/include/linux, although more work needs
to be done here.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 tools/testing/radix-tree/linux/init.h   |  0
 tools/testing/radix-tree/linux/kernel.h | 12 ++++++++++--
 tools/testing/radix-tree/linux/slab.h   |  1 -
 tools/testing/radix-tree/linux/types.h  |  7 ++-----
 4 files changed, 12 insertions(+), 8 deletions(-)
 create mode 100644 tools/testing/radix-tree/linux/init.h

diff --git a/tools/testing/radix-tree/linux/init.h b/tools/testing/radix-tree/linux/init.h
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
index ae013b0160ac..6d0cdf618084 100644
--- a/tools/testing/radix-tree/linux/kernel.h
+++ b/tools/testing/radix-tree/linux/kernel.h
@@ -7,19 +7,25 @@
 #include <stddef.h>
 #include <limits.h>
 
+#include "../../include/linux/compiler.h"
+
 #ifndef NULL
 #define NULL	0
 #endif
 
 #define BUG_ON(expr)	assert(!(expr))
+#define WARN_ON(expr)	assert(!(expr))
 #define __init
 #define __must_check
 #define panic(expr)
 #define printk printf
 #define __force
-#define likely(c) (c)
-#define unlikely(c) (c)
 #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
+#define pr_debug printk
+
+#define smp_rmb()	barrier()
+#define smp_wmb()	barrier()
+#define cpu_relax()	barrier()
 
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 
@@ -28,6 +34,8 @@
 	(type *)( (char *)__mptr - offsetof(type, member) );})
 #define min(a, b) ((a) < (b) ? (a) : (b))
 
+#define cond_resched()	sched_yield()
+
 static inline int in_interrupt(void)
 {
 	return 0;
diff --git a/tools/testing/radix-tree/linux/slab.h b/tools/testing/radix-tree/linux/slab.h
index 57282506c21d..6d5a34770fd4 100644
--- a/tools/testing/radix-tree/linux/slab.h
+++ b/tools/testing/radix-tree/linux/slab.h
@@ -3,7 +3,6 @@
 
 #include <linux/types.h>
 
-#define GFP_KERNEL 1
 #define SLAB_HWCACHE_ALIGN 1
 #define SLAB_PANIC 2
 #define SLAB_RECLAIM_ACCOUNT    0x00020000UL            /* Objects are reclaimable */
diff --git a/tools/testing/radix-tree/linux/types.h b/tools/testing/radix-tree/linux/types.h
index 72a9d85f6c76..faa0b6ff9ca8 100644
--- a/tools/testing/radix-tree/linux/types.h
+++ b/tools/testing/radix-tree/linux/types.h
@@ -1,15 +1,13 @@
 #ifndef _TYPES_H
 #define _TYPES_H
 
+#include "../../include/linux/types.h"
+
 #define __rcu
 #define __read_mostly
 
 #define BITS_PER_LONG (sizeof(long) * 8)
 
-struct list_head {
-	struct list_head *next, *prev;
-};
-
 static inline void INIT_LIST_HEAD(struct list_head *list)
 {
 	list->next = list;
@@ -22,7 +20,6 @@ typedef struct {
 
 #define uninitialized_var(x) x = x
 
-typedef unsigned gfp_t;
 #include <linux/gfp.h>
 
 #endif
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 03/30] radix tree test suite: Add tests for radix_tree_locate_item()
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 01/30] radix-tree: Introduce radix_tree_empty Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 02/30] radix tree test suite: Fix build Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 04/30] radix tree test suite: Allow testing other fan-out values Matthew Wilcox
                   ` (26 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

Fairly simple tests; add various items to the tree, then make sure we
can find them again.  Also check that a pointer that we know isn't in
the tree is not found.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 tools/testing/radix-tree/linux/kernel.h |  3 +++
 tools/testing/radix-tree/main.c         | 41 +++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
index 6d0cdf618084..76a88f35fdc4 100644
--- a/tools/testing/radix-tree/linux/kernel.h
+++ b/tools/testing/radix-tree/linux/kernel.h
@@ -9,6 +9,9 @@
 
 #include "../../include/linux/compiler.h"
 
+#define CONFIG_SHMEM
+#define CONFIG_SWAP
+
 #ifndef NULL
 #define NULL	0
 #endif
diff --git a/tools/testing/radix-tree/main.c b/tools/testing/radix-tree/main.c
index 0e83cad27a9f..71c5272443b1 100644
--- a/tools/testing/radix-tree/main.c
+++ b/tools/testing/radix-tree/main.c
@@ -232,10 +232,51 @@ void copy_tag_check(void)
 	item_kill_tree(&tree);
 }
 
+void __locate_check(struct radix_tree_root *tree, unsigned long index)
+{
+	struct item *item;
+	unsigned long index2;
+
+	item_insert(tree, index);
+	item = item_lookup(tree, index);
+	index2 = radix_tree_locate_item(tree, item);
+	if (index != index2) {
+		printf("index %ld inserted; found %ld\n",
+			index, index2);
+		abort();
+	}
+}
+
+static void locate_check(void)
+{
+	RADIX_TREE(tree, GFP_KERNEL);
+	unsigned long offset, index;
+
+	for (offset = 0; offset < (1 << 3); offset++) {
+		for (index = 0; index < (1UL << 5); index++) {
+			__locate_check(&tree, index + offset);
+		}
+		if (radix_tree_locate_item(&tree, &tree) != -1)
+			abort();
+
+		item_kill_tree(&tree);
+	}
+
+	if (radix_tree_locate_item(&tree, &tree) != -1)
+		abort();
+	__locate_check(&tree, -1);
+	if (radix_tree_locate_item(&tree, &tree) != -1)
+		abort();
+	item_kill_tree(&tree);
+}
+
 static void single_thread_tests(void)
 {
 	int i;
 
+	printf("starting single_thread_tests: %d allocated\n", nr_allocated);
+	locate_check();
+	printf("after locate_check: %d allocated\n", nr_allocated);
 	tag_check();
 	printf("after tag_check: %d allocated\n", nr_allocated);
 	gang_check();
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 04/30] radix tree test suite: Allow testing other fan-out values
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (2 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 03/30] radix tree test suite: Add tests for radix_tree_locate_item() Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 05/30] radix tree test suite: keep regression test runs short Matthew Wilcox
                   ` (25 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

The defines in regression2.c are already in radix-tree.h and duplicating
them in the test case makes experimenting with other values for the
fan-out harder than necessary.  Allow the user of the radix tree to decide
what the fan-out should be rather than fixing it to 8 for non-kernel uses.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 include/linux/radix-tree.h              | 4 +---
 tools/testing/radix-tree/linux/kernel.h | 2 ++
 tools/testing/radix-tree/regression2.c  | 7 -------
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index 83f708e5db59..5ce5a1e0ecc5 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -70,10 +70,8 @@ static inline int radix_tree_is_indirect_ptr(void *ptr)
 
 #define RADIX_TREE_MAX_TAGS 3
 
-#ifdef __KERNEL__
+#ifndef RADIX_TREE_MAP_SHIFT
 #define RADIX_TREE_MAP_SHIFT	(CONFIG_BASE_SMALL ? 4 : 6)
-#else
-#define RADIX_TREE_MAP_SHIFT	3	/* For more stressful testing */
 #endif
 
 #define RADIX_TREE_MAP_SIZE	(1UL << RADIX_TREE_MAP_SHIFT)
diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
index 76a88f35fdc4..31fe2c77d7ae 100644
--- a/tools/testing/radix-tree/linux/kernel.h
+++ b/tools/testing/radix-tree/linux/kernel.h
@@ -12,6 +12,8 @@
 #define CONFIG_SHMEM
 #define CONFIG_SWAP
 
+#define RADIX_TREE_MAP_SHIFT	3
+
 #ifndef NULL
 #define NULL	0
 #endif
diff --git a/tools/testing/radix-tree/regression2.c b/tools/testing/radix-tree/regression2.c
index 5d2fa28cdca3..63bf347aaf33 100644
--- a/tools/testing/radix-tree/regression2.c
+++ b/tools/testing/radix-tree/regression2.c
@@ -51,13 +51,6 @@
 
 #include "regression.h"
 
-#ifdef __KERNEL__
-#define RADIX_TREE_MAP_SHIFT    (CONFIG_BASE_SMALL ? 4 : 6)
-#else
-#define RADIX_TREE_MAP_SHIFT    3       /* For more stressful testing */
-#endif
-
-#define RADIX_TREE_MAP_SIZE     (1UL << RADIX_TREE_MAP_SHIFT)
 #define PAGECACHE_TAG_DIRTY     0
 #define PAGECACHE_TAG_WRITEBACK 1
 #define PAGECACHE_TAG_TOWRITE   2
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 05/30] radix tree test suite: keep regression test runs short
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (3 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 04/30] radix tree test suite: Allow testing other fan-out values Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 06/30] radix tree test suite: rebuild when headers change Matthew Wilcox
                   ` (24 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

Currently the full suite of regression tests take upwards of 30 minutes to
run on my development machine.  The vast majority of this time is taken by
the big_gang_check() and copy_tag_check() tests, which each run their tests
through thousands of iterations...does this have value?

Without big_gang_check() and copy_tag_check(), the test suite runs in
around 15 seconds on my box.

Honestly the first time I ever ran through the entire test suite was to
gather the timings for this email - it simply takes too long to be useful
on a normal basis.

Instead, hide the excessive iterations through big_gang_check() and
copy_tag_check() tests behind an '-l' flag (for "long run") in case they
are still useful, but allow the regression test suite to complete in a
reasonable amount of time.  We still run each of these tests a few times (3
at present) to try and keep the test coverage.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 tools/testing/radix-tree/main.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/tools/testing/radix-tree/main.c b/tools/testing/radix-tree/main.c
index 71c5272443b1..122c8b9be17e 100644
--- a/tools/testing/radix-tree/main.c
+++ b/tools/testing/radix-tree/main.c
@@ -61,11 +61,11 @@ void __big_gang_check(void)
 	} while (!wrapped);
 }
 
-void big_gang_check(void)
+void big_gang_check(bool long_run)
 {
 	int i;
 
-	for (i = 0; i < 1000; i++) {
+	for (i = 0; i < (long_run ? 1000 : 3); i++) {
 		__big_gang_check();
 		srand(time(0));
 		printf("%d ", i);
@@ -270,7 +270,7 @@ static void locate_check(void)
 	item_kill_tree(&tree);
 }
 
-static void single_thread_tests(void)
+static void single_thread_tests(bool long_run)
 {
 	int i;
 
@@ -285,9 +285,9 @@ static void single_thread_tests(void)
 	printf("after add_and_check: %d allocated\n", nr_allocated);
 	dynamic_height_check();
 	printf("after dynamic_height_check: %d allocated\n", nr_allocated);
-	big_gang_check();
+	big_gang_check(long_run);
 	printf("after big_gang_check: %d allocated\n", nr_allocated);
-	for (i = 0; i < 2000; i++) {
+	for (i = 0; i < (long_run ? 2000 : 3); i++) {
 		copy_tag_check();
 		printf("%d ", i);
 		fflush(stdout);
@@ -295,15 +295,23 @@ static void single_thread_tests(void)
 	printf("after copy_tag_check: %d allocated\n", nr_allocated);
 }
 
-int main(void)
+int main(int argc, char **argv)
 {
+	bool long_run = false;
+	int opt;
+
+	while ((opt = getopt(argc, argv, "l")) != -1) {
+		if (opt == 'l')
+			long_run = true;
+	}
+
 	rcu_register_thread();
 	radix_tree_init();
 
 	regression1_test();
 	regression2_test();
 	regression3_test();
-	single_thread_tests();
+	single_thread_tests(long_run);
 
 	sleep(1);
 	printf("after sleep(1): %d allocated\n", nr_allocated);
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 06/30] radix tree test suite: rebuild when headers change
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (4 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 05/30] radix tree test suite: keep regression test runs short Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 07/30] radix-tree: remove unused looping macros Matthew Wilcox
                   ` (23 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

When we make changes to radix-tree.h in the regular kernel source
(include/linux/radix-tree.h), we really want our test code to be rebuilt.

We also include a few other headers from tools/include and probably want
to rebuild if these have been changed.

Update the makefile so that all of our objects will be rebuilt when any
of the headers we depend on are changed.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 tools/testing/radix-tree/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile
index 604212db9d4b..43febba864bd 100644
--- a/tools/testing/radix-tree/Makefile
+++ b/tools/testing/radix-tree/Makefile
@@ -13,7 +13,7 @@ main:	$(OFILES)
 clean:
 	$(RM) -f $(TARGETS) *.o radix-tree.c
 
-$(OFILES): *.h */*.h
+$(OFILES): *.h */*.h ../../../include/linux/radix-tree.h ../../include/linux/*.h
 
 radix-tree.c: ../../../lib/radix-tree.c
 	sed -e 's/^static //' -e 's/__always_inline //' -e 's/inline //' < $< > $@
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 07/30] radix-tree: remove unused looping macros
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (5 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 06/30] radix tree test suite: rebuild when headers change Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 08/30] Introduce CONFIG_RADIX_TREE_MULTIORDER Matthew Wilcox
                   ` (22 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

radix_tree_for_each_chunk() and radix_tree_for_each_chunk_slot()
have never been used in the kernel since their introduction in 2012,
so remove them.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 include/linux/radix-tree.h | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index 5ce5a1e0ecc5..e1512a607709 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -479,34 +479,6 @@ radix_tree_next_slot(void **slot, struct radix_tree_iter *iter, unsigned flags)
 }
 
 /**
- * radix_tree_for_each_chunk - iterate over chunks
- *
- * @slot:	the void** variable for pointer to chunk first slot
- * @root:	the struct radix_tree_root pointer
- * @iter:	the struct radix_tree_iter pointer
- * @start:	iteration starting index
- * @flags:	RADIX_TREE_ITER_* and tag index
- *
- * Locks can be released and reacquired between iterations.
- */
-#define radix_tree_for_each_chunk(slot, root, iter, start, flags)	\
-	for (slot = radix_tree_iter_init(iter, start) ;			\
-	      (slot = radix_tree_next_chunk(root, iter, flags)) ;)
-
-/**
- * radix_tree_for_each_chunk_slot - iterate over slots in one chunk
- *
- * @slot:	the void** variable, at the beginning points to chunk first slot
- * @iter:	the struct radix_tree_iter pointer
- * @flags:	RADIX_TREE_ITER_*, should be constant
- *
- * This macro is designed to be nested inside radix_tree_for_each_chunk().
- * @slot points to the radix tree slot, @iter->index contains its index.
- */
-#define radix_tree_for_each_chunk_slot(slot, iter, flags)		\
-	for (; slot ; slot = radix_tree_next_slot(slot, iter, flags))
-
-/**
  * radix_tree_for_each_slot - iterate over non-empty slots
  *
  * @slot:	the void** variable for pointer to slot
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 08/30] Introduce CONFIG_RADIX_TREE_MULTIORDER
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (6 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 07/30] radix-tree: remove unused looping macros Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 09/30] radix-tree: Add missing sibling entry functionality Matthew Wilcox
                   ` (21 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

I've been receiving increasingly concerned notes from 0day about how much
my recent changes have been bloating the radix tree.  Make it happier by
only including multiorder support if CONFIG_TRANSPARENT_HUGEPAGES is set.
This is an independent Kconfig option, so other radix tree users can
also set it if they have a need.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/Kconfig                             |  3 +++
 lib/radix-tree.c                        | 25 +++++++++++++++++--------
 mm/Kconfig                              |  1 +
 tools/testing/radix-tree/linux/kernel.h |  1 +
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index 3cca1222578e..56403a6fba41 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -362,6 +362,9 @@ config INTERVAL_TREE
 
 	  for more information.
 
+config RADIX_TREE_MULTIORDER
+	bool
+
 config ASSOCIATIVE_ARRAY
 	bool
 	help
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 1624c4117961..ae53ad56e01e 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -484,6 +484,7 @@ int __radix_tree_create(struct radix_tree_root *root, unsigned long index,
 		slot = node->slots[offset];
 	}
 
+#ifdef CONFIG_RADIX_TREE_MULTIORDER
 	/* Insert pointers to the canonical entry */
 	if ((shift - order) > 0) {
 		int i, n = 1 << (shift - order);
@@ -499,6 +500,7 @@ int __radix_tree_create(struct radix_tree_root *root, unsigned long index,
 			node->count++;
 		}
 	}
+#endif
 
 	if (nodep)
 		*nodep = node;
@@ -1469,6 +1471,19 @@ bool __radix_tree_delete_node(struct radix_tree_root *root,
 	return deleted;
 }
 
+static inline void delete_sibling_entries(struct radix_tree_node *node,
+					void *ptr, unsigned offset)
+{
+#ifdef CONFIG_RADIX_TREE_MULTIORDER
+	int i;
+	for (i = 1; offset + i < RADIX_TREE_MAP_SIZE; i++) {
+		if (node->slots[offset + i] != ptr)
+			break;
+		node->slots[offset + i] = NULL;
+		node->count--;
+	}
+#endif
+}
 /**
  *	radix_tree_delete_item    -    delete an item from a radix tree
  *	@root:		radix tree root
@@ -1484,7 +1499,7 @@ void *radix_tree_delete_item(struct radix_tree_root *root,
 			     unsigned long index, void *item)
 {
 	struct radix_tree_node *node;
-	unsigned int offset, i;
+	unsigned int offset;
 	void **slot;
 	void *entry;
 	int tag;
@@ -1513,13 +1528,7 @@ void *radix_tree_delete_item(struct radix_tree_root *root,
 			radix_tree_tag_clear(root, index, tag);
 	}
 
-	/* Delete any sibling slots pointing to this slot */
-	for (i = 1; offset + i < RADIX_TREE_MAP_SIZE; i++) {
-		if (node->slots[offset + i] != ptr_to_indirect(slot))
-			break;
-		node->slots[offset + i] = NULL;
-		node->count--;
-	}
+	delete_sibling_entries(node, ptr_to_indirect(slot), offset);
 	node->slots[offset] = NULL;
 	node->count--;
 
diff --git a/mm/Kconfig b/mm/Kconfig
index 989f8f3d77e0..6d5b39e6d326 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -393,6 +393,7 @@ config TRANSPARENT_HUGEPAGE
 	bool "Transparent Hugepage Support"
 	depends on HAVE_ARCH_TRANSPARENT_HUGEPAGE
 	select COMPACTION
+	select RADIX_TREE_MULTIORDER
 	help
 	  Transparent Hugepages allows the kernel to use huge pages and
 	  huge tlb transparently to the applications whenever possible.
diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
index 31fe2c77d7ae..8ea0ed450810 100644
--- a/tools/testing/radix-tree/linux/kernel.h
+++ b/tools/testing/radix-tree/linux/kernel.h
@@ -9,6 +9,7 @@
 
 #include "../../include/linux/compiler.h"
 
+#define CONFIG_RADIX_TREE_MULTIORDER
 #define CONFIG_SHMEM
 #define CONFIG_SWAP
 
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 09/30] radix-tree: Add missing sibling entry functionality
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (7 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 08/30] Introduce CONFIG_RADIX_TREE_MULTIORDER Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 10/30] radix-tree: Fix sibling entry insertion Matthew Wilcox
                   ` (20 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

The code I previously added to enable multiorder radix tree entries was
untested and hence buggy.  This commit adds the support functions that
Ross and I decided were necessary over a four-week period of iterating
various designs.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index ae53ad56e01e..40343f28a705 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -80,6 +80,46 @@ static inline void *indirect_to_ptr(void *ptr)
 	return (void *)((unsigned long)ptr & ~RADIX_TREE_INDIRECT_PTR);
 }
 
+#ifdef CONFIG_RADIX_TREE_MULTIORDER
+/* Sibling slots point directly to another slot in the same node */
+static inline bool is_sibling_entry(struct radix_tree_node *parent, void *node)
+{
+	void **ptr = node;
+	return (parent->slots <= ptr) &&
+			(ptr < parent->slots + RADIX_TREE_MAP_SIZE);
+}
+#else
+static inline bool is_sibling_entry(struct radix_tree_node *parent, void *node)
+{
+	return false;
+}
+#endif
+
+static inline unsigned get_sibling_offset(struct radix_tree_node *parent,
+						 void **slot)
+{
+	return slot - parent->slots;
+}
+
+static unsigned radix_tree_descend(struct radix_tree_node *parent,
+				struct radix_tree_node **nodep, unsigned offset)
+{
+	void **entry = rcu_dereference_raw(parent->slots[offset]);
+
+#ifdef CONFIG_RADIX_TREE_MULTIORDER
+	if (radix_tree_is_indirect_ptr(entry)) {
+		uintptr_t siboff = entry - parent->slots;
+		if (siboff < RADIX_TREE_MAP_SIZE) {
+			offset = siboff;
+			entry = rcu_dereference_raw(parent->slots[offset]);
+		}
+	}
+#endif
+
+	*nodep = (void *)entry;
+	return offset;
+}
+
 static inline gfp_t root_gfp_mask(struct radix_tree_root *root)
 {
 	return root->gfp_mask & __GFP_BITS_MASK;
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 10/30] radix-tree: Fix sibling entry insertion
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (8 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 09/30] radix-tree: Add missing sibling entry functionality Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 11/30] radix-tree: Fix deleting a multi-order entry through an alias Matthew Wilcox
                   ` (19 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

The subtraction was the wrong way round, leading to undefined behaviour
(shift by an amount larger than the size of the type).

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 40343f28a705..42a0492b2ba2 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -526,8 +526,8 @@ int __radix_tree_create(struct radix_tree_root *root, unsigned long index,
 
 #ifdef CONFIG_RADIX_TREE_MULTIORDER
 	/* Insert pointers to the canonical entry */
-	if ((shift - order) > 0) {
-		int i, n = 1 << (shift - order);
+	if (order > shift) {
+		int i, n = 1 << (order - shift);
 		offset = offset & ~(n - 1);
 		slot = ptr_to_indirect(&node->slots[offset]);
 		for (i = 0; i < n; i++) {
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 11/30] radix-tree: Fix deleting a multi-order entry through an alias
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (9 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 10/30] radix-tree: Fix sibling entry insertion Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 12/30] radix-tree: Remove restriction on multi-order entries Matthew Wilcox
                   ` (18 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

If we deleted an entry through an index which looked up a sibling
pointer, we'd end up zeroing out the wrong slots in the node.
Use get_sibling_offset() to find the right slot.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 42a0492b2ba2..554986599c63 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -1557,7 +1557,7 @@ void *radix_tree_delete_item(struct radix_tree_root *root,
 		return entry;
 	}
 
-	offset = index & RADIX_TREE_MAP_MASK;
+	offset = get_sibling_offset(node, slot);
 
 	/*
 	 * Clear all tags associated with the item to be deleted.
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 12/30] radix-tree: Remove restriction on multi-order entries
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (10 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 11/30] radix-tree: Fix deleting a multi-order entry through an alias Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 13/30] radix-tree: Introduce radix_tree_load_root() Matthew Wilcox
                   ` (17 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

Now that sibling pointers are handled explicitly, there is no purpose
served by restricting the order to be >= RADIX_TREE_MAP_SHIFT.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 554986599c63..f2a314cf42cc 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -483,8 +483,6 @@ int __radix_tree_create(struct radix_tree_root *root, unsigned long index,
 	unsigned int height, shift, offset;
 	int error;
 
-	BUG_ON((0 < order) && (order < RADIX_TREE_MAP_SHIFT));
-
 	/* Make sure the tree is high enough.  */
 	if (index > radix_tree_maxindex(root->height)) {
 		error = radix_tree_extend(root, index, order);
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 13/30] radix-tree: Introduce radix_tree_load_root()
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (11 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 12/30] radix-tree: Remove restriction on multi-order entries Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 14/30] radix-tree: Fix extending the tree for multi-order entries at offset 0 Matthew Wilcox
                   ` (16 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

All the tree walking functions start with some variant of this code;
centralise it in one place so we're not chasing subtly different bugs
everywhere.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index f2a314cf42cc..b3a7e6cd5773 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -405,6 +405,29 @@ static inline unsigned long radix_tree_maxindex(unsigned int height)
 	return height_to_maxindex[height];
 }
 
+static inline unsigned long node_maxindex(struct radix_tree_node *node)
+{
+	return radix_tree_maxindex(node->path & RADIX_TREE_HEIGHT_MASK);
+}
+
+static unsigned radix_tree_load_root(struct radix_tree_root *root,
+		struct radix_tree_node **nodep, unsigned long *maxindex)
+{
+	struct radix_tree_node *node = rcu_dereference_raw(root->rnode);
+
+	*nodep = node;
+
+	if (likely(radix_tree_is_indirect_ptr(node))) {
+		node = indirect_to_ptr(node);
+		*maxindex = node_maxindex(node);
+		return (node->path & RADIX_TREE_HEIGHT_MASK) *
+			RADIX_TREE_MAP_SHIFT;
+	}
+
+	*maxindex = 0;
+	return 0;
+}
+
 /*
  *	Extend a radix tree so it can store key @index.
  */
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 14/30] radix-tree: Fix extending the tree for multi-order entries at offset 0
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (12 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 13/30] radix-tree: Introduce radix_tree_load_root() Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 15/30] radix-tree: Fix several shrinking bugs with multiorder entries Matthew Wilcox
                   ` (15 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

The current code will insert entries at each level, as if we're going
to add a new entry at the bottom level, so we then get an -EEXIST when
we try to insert the entry into the tree.  The best way to fix this is
to not check 'order' when inserting into an empty tree.

We still need to 'extend' the tree to the height necessary for the
maximum index corresponding to this entry, so pass that value to
radix_tree_extend() rather than the index we're asked to create, or we
won't create a tree that's deep enough.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index b3a7e6cd5773..abe7730a63af 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -432,7 +432,7 @@ static unsigned radix_tree_load_root(struct radix_tree_root *root,
  *	Extend a radix tree so it can store key @index.
  */
 static int radix_tree_extend(struct radix_tree_root *root,
-				unsigned long index, unsigned order)
+				unsigned long index)
 {
 	struct radix_tree_node *node;
 	struct radix_tree_node *slot;
@@ -444,7 +444,7 @@ static int radix_tree_extend(struct radix_tree_root *root,
 	while (index > radix_tree_maxindex(height))
 		height++;
 
-	if ((root->rnode == NULL) && (order == 0)) {
+	if (root->rnode == NULL) {
 		root->height = height;
 		goto out;
 	}
@@ -467,7 +467,7 @@ static int radix_tree_extend(struct radix_tree_root *root,
 		node->count = 1;
 		node->parent = NULL;
 		slot = root->rnode;
-		if (radix_tree_is_indirect_ptr(slot) && newheight > 1) {
+		if (radix_tree_is_indirect_ptr(slot)) {
 			slot = indirect_to_ptr(slot);
 			slot->parent = node;
 			slot = ptr_to_indirect(slot);
@@ -478,7 +478,7 @@ static int radix_tree_extend(struct radix_tree_root *root,
 		root->height = newheight;
 	} while (height > root->height);
 out:
-	return 0;
+	return height * RADIX_TREE_MAP_SHIFT;
 }
 
 /**
@@ -503,20 +503,26 @@ int __radix_tree_create(struct radix_tree_root *root, unsigned long index,
 			void ***slotp)
 {
 	struct radix_tree_node *node = NULL, *slot;
+	unsigned long maxindex;
 	unsigned int height, shift, offset;
-	int error;
+	unsigned long max = index | ((1UL << order) - 1);
+
+	shift = radix_tree_load_root(root, &slot, &maxindex);
 
 	/* Make sure the tree is high enough.  */
-	if (index > radix_tree_maxindex(root->height)) {
-		error = radix_tree_extend(root, index, order);
-		if (error)
+	if (max > maxindex) {
+		int error = radix_tree_extend(root, max);
+		if (error < 0)
 			return error;
+		shift = error;
+		slot = root->rnode;
+		if (order == shift) {
+			shift += RADIX_TREE_MAP_SHIFT;
+			root->height++;
+		}
 	}
 
-	slot = root->rnode;
-
 	height = root->height;
-	shift = height * RADIX_TREE_MAP_SHIFT;
 
 	offset = 0;			/* uninitialised var warning */
 	while (shift > order) {
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 15/30] radix-tree: Fix several shrinking bugs with multiorder entries
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (13 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 14/30] radix-tree: Fix extending the tree for multi-order entries at offset 0 Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 16/30] radix tree test suite: Start adding multiorder tests Matthew Wilcox
                   ` (14 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

Setting the indirect bit on the user data entry used to be unambiguous
because the tree walking code knew not to expect internal nodes in the
last level of the tree.  Multiorder entries can appear at any level of
the tree, and a leaf with the indirect bit set is indistinguishable from
a pointer to a node.

Introduce a special entry (RADIX_TREE_RETRY) which is neither a valid
user entry, nor a valid pointer to a node.  The radix_tree_deref_retry()
function continues to work the same way, but tree walking code can
distinguish it from a pointer to a node.

Also fix the condition for setting slot->parent to NULL; it does not
matter what height the tree is, it only matters whether slot is an
indirect pointer.  Move this code above the comment which is referring
to the assignment to root->rnode.

Also fix the condition for preventing the tree from shrinking to a single
entry if it's a multiorder entry.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index abe7730a63af..3dc52433a9cc 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -80,6 +80,8 @@ static inline void *indirect_to_ptr(void *ptr)
 	return (void *)((unsigned long)ptr & ~RADIX_TREE_INDIRECT_PTR);
 }
 
+#define RADIX_TREE_RETRY	ptr_to_indirect(NULL)
+
 #ifdef CONFIG_RADIX_TREE_MULTIORDER
 /* Sibling slots point directly to another slot in the same node */
 static inline bool is_sibling_entry(struct radix_tree_node *parent, void *node)
@@ -1443,6 +1445,14 @@ static inline void radix_tree_shrink(struct radix_tree_root *root)
 		slot = to_free->slots[0];
 		if (!slot)
 			break;
+		if (!radix_tree_is_indirect_ptr(slot) && (root->height > 1))
+			break;
+
+		if (radix_tree_is_indirect_ptr(slot)) {
+			slot = indirect_to_ptr(slot);
+			slot->parent = NULL;
+			slot = ptr_to_indirect(slot);
+		}
 
 		/*
 		 * We don't need rcu_assign_pointer(), since we are simply
@@ -1451,14 +1461,6 @@ static inline void radix_tree_shrink(struct radix_tree_root *root)
 		 * (to_free->slots[0]), it will be safe to dereference the new
 		 * one (root->rnode) as far as dependent read barriers go.
 		 */
-		if (root->height > 1) {
-			if (!radix_tree_is_indirect_ptr(slot))
-				break;
-
-			slot = indirect_to_ptr(slot);
-			slot->parent = NULL;
-			slot = ptr_to_indirect(slot);
-		}
 		root->rnode = slot;
 		root->height--;
 
@@ -1480,9 +1482,8 @@ static inline void radix_tree_shrink(struct radix_tree_root *root)
 		 * also results in a stale slot). So tag the slot as indirect
 		 * to force callers to retry.
 		 */
-		if (root->height == 0)
-			*((unsigned long *)&to_free->slots[0]) |=
-						RADIX_TREE_INDIRECT_PTR;
+		if (!radix_tree_is_indirect_ptr(slot))
+			to_free->slots[0] = RADIX_TREE_RETRY;
 
 		radix_tree_node_free(to_free);
 	}
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 16/30] radix tree test suite: Start adding multiorder tests
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (14 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 15/30] radix-tree: Fix several shrinking bugs with multiorder entries Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 17/30] radix-tree: Rewrite __radix_tree_lookup Matthew Wilcox
                   ` (13 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

Test suite infrastructure for working with multiorder entries.

The test itself is pretty basic: Add an entry, check that all expected
indices return that entry and that indices around that entry don't return
an entry. Then delete the entry and check no index returns that entry.
Tests a few edge conditions including the multiorder entry at index 0
and at a higher index.  Also tests deleting through an alias as well as
through the canonical index.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 tools/testing/radix-tree/Makefile     |  2 +-
 tools/testing/radix-tree/main.c       |  2 ++
 tools/testing/radix-tree/multiorder.c | 58 +++++++++++++++++++++++++++++++++++
 tools/testing/radix-tree/test.c       | 13 ++++++--
 tools/testing/radix-tree/test.h       |  6 +++-
 5 files changed, 76 insertions(+), 5 deletions(-)
 create mode 100644 tools/testing/radix-tree/multiorder.c

diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile
index 43febba864bd..3b530467148e 100644
--- a/tools/testing/radix-tree/Makefile
+++ b/tools/testing/radix-tree/Makefile
@@ -3,7 +3,7 @@ CFLAGS += -I. -g -Wall -D_LGPL_SOURCE
 LDFLAGS += -lpthread -lurcu
 TARGETS = main
 OFILES = main.o radix-tree.o linux.o test.o tag_check.o find_next_bit.o \
-	 regression1.o regression2.o regression3.o
+	 regression1.o regression2.o regression3.o multiorder.o
 
 targets: $(TARGETS)
 
diff --git a/tools/testing/radix-tree/main.c b/tools/testing/radix-tree/main.c
index 122c8b9be17e..b6a700b00cce 100644
--- a/tools/testing/radix-tree/main.c
+++ b/tools/testing/radix-tree/main.c
@@ -275,6 +275,8 @@ static void single_thread_tests(bool long_run)
 	int i;
 
 	printf("starting single_thread_tests: %d allocated\n", nr_allocated);
+	multiorder_checks();
+	printf("after multiorder_check: %d allocated\n", nr_allocated);
 	locate_check();
 	printf("after locate_check: %d allocated\n", nr_allocated);
 	tag_check();
diff --git a/tools/testing/radix-tree/multiorder.c b/tools/testing/radix-tree/multiorder.c
new file mode 100644
index 000000000000..cfe718c78eb6
--- /dev/null
+++ b/tools/testing/radix-tree/multiorder.c
@@ -0,0 +1,58 @@
+/*
+ * multiorder.c: Multi-order radix tree entry testing
+ * Copyright (c) 2016 Intel Corporation
+ * Author: Ross Zwisler <ross.zwisler@linux.intel.com>
+ * Author: Matthew Wilcox <matthew.r.wilcox@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+#include <linux/radix-tree.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+
+#include "test.h"
+
+static void multiorder_check(unsigned long index, int order)
+{
+	unsigned long i;
+	unsigned long min = index & ~((1UL << order) - 1);
+	unsigned long max = min + (1UL << order);
+	RADIX_TREE(tree, GFP_KERNEL);
+
+	printf("Multiorder index %ld, order %d\n", index, order);
+
+	assert(item_insert_order(&tree, index, order) == 0);
+
+	for (i = min; i < max; i++) {
+		struct item *item = item_lookup(&tree, i);
+		assert(item != 0);
+		assert(item->index == index);
+	}
+	for (i = 0; i < min; i++)
+		item_check_absent(&tree, i);
+	for (i = max; i < 2*max; i++)
+		item_check_absent(&tree, i);
+
+	assert(item_delete(&tree, index) != 0);
+
+	for (i = 0; i < 2*max; i++)
+		item_check_absent(&tree, i);
+}
+
+void multiorder_checks(void)
+{
+	int i;
+
+	for (i = 0; i < 20; i++) {
+		multiorder_check(200, i);
+		multiorder_check(0, i);
+		multiorder_check((1UL << i) + 1, i);
+	}
+}
diff --git a/tools/testing/radix-tree/test.c b/tools/testing/radix-tree/test.c
index 2bebf34cdc27..da54f11e8ba7 100644
--- a/tools/testing/radix-tree/test.c
+++ b/tools/testing/radix-tree/test.c
@@ -24,14 +24,21 @@ int item_tag_get(struct radix_tree_root *root, unsigned long index, int tag)
 	return radix_tree_tag_get(root, index, tag);
 }
 
-int __item_insert(struct radix_tree_root *root, struct item *item)
+int __item_insert(struct radix_tree_root *root, struct item *item,
+			unsigned order)
 {
-	return radix_tree_insert(root, item->index, item);
+	return __radix_tree_insert(root, item->index, order, item);
 }
 
 int item_insert(struct radix_tree_root *root, unsigned long index)
 {
-	return __item_insert(root, item_create(index));
+	return __item_insert(root, item_create(index), 0);
+}
+
+int item_insert_order(struct radix_tree_root *root, unsigned long index,
+			unsigned order)
+{
+	return __item_insert(root, item_create(index), order);
 }
 
 int item_delete(struct radix_tree_root *root, unsigned long index)
diff --git a/tools/testing/radix-tree/test.h b/tools/testing/radix-tree/test.h
index 4e1d95faaa94..53cb595db44a 100644
--- a/tools/testing/radix-tree/test.h
+++ b/tools/testing/radix-tree/test.h
@@ -8,8 +8,11 @@ struct item {
 };
 
 struct item *item_create(unsigned long index);
-int __item_insert(struct radix_tree_root *root, struct item *item);
+int __item_insert(struct radix_tree_root *root, struct item *item,
+			unsigned order);
 int item_insert(struct radix_tree_root *root, unsigned long index);
+int item_insert_order(struct radix_tree_root *root, unsigned long index,
+			unsigned order);
 int item_delete(struct radix_tree_root *root, unsigned long index);
 struct item *item_lookup(struct radix_tree_root *root, unsigned long index);
 
@@ -23,6 +26,7 @@ void item_full_scan(struct radix_tree_root *root, unsigned long start,
 void item_kill_tree(struct radix_tree_root *root);
 
 void tag_check(void);
+void multiorder_checks(void);
 
 struct item *
 item_tag_set(struct radix_tree_root *root, unsigned long index, int tag);
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 17/30] radix-tree: Rewrite __radix_tree_lookup
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (15 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 16/30] radix tree test suite: Start adding multiorder tests Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 18/30] radix-tree: Fix multiorder BUG_ON in radix_tree_insert Matthew Wilcox
                   ` (12 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

Use the new multi-order support functions to rewrite __radix_tree_lookup()

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c | 45 +++++++++++++++++----------------------------
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 3dc52433a9cc..2ca2b416b476 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -634,44 +634,33 @@ void *__radix_tree_lookup(struct radix_tree_root *root, unsigned long index,
 			  struct radix_tree_node **nodep, void ***slotp)
 {
 	struct radix_tree_node *node, *parent;
-	unsigned int height, shift;
+	unsigned long maxindex;
+	unsigned int shift;
 	void **slot;
 
-	node = rcu_dereference_raw(root->rnode);
-	if (node == NULL)
+ restart:
+	parent = NULL;
+	slot = (void **)&root->rnode;
+	shift = radix_tree_load_root(root, &node, &maxindex);
+	if (index > maxindex)
 		return NULL;
 
-	if (!radix_tree_is_indirect_ptr(node)) {
-		if (index > 0)
-			return NULL;
-
-		if (nodep)
-			*nodep = NULL;
-		if (slotp)
-			*slotp = (void **)&root->rnode;
-		return node;
-	}
-	node = indirect_to_ptr(node);
-
-	height = node->path & RADIX_TREE_HEIGHT_MASK;
-	if (index > radix_tree_maxindex(height))
-		return NULL;
-
-	shift = (height-1) * RADIX_TREE_MAP_SHIFT;
+	for (;;) {
+		unsigned offset;
 
-	do {
-		parent = node;
-		slot = node->slots + ((index >> shift) & RADIX_TREE_MAP_MASK);
-		node = rcu_dereference_raw(*slot);
-		if (node == NULL)
+		if (!node)
 			return NULL;
+		if (node == RADIX_TREE_RETRY)
+			goto restart;
 		if (!radix_tree_is_indirect_ptr(node))
 			break;
-		node = indirect_to_ptr(node);
 
+		parent = indirect_to_ptr(node);
 		shift -= RADIX_TREE_MAP_SHIFT;
-		height--;
-	} while (height > 0);
+		offset = (index >> shift) & RADIX_TREE_MAP_MASK;
+		offset = radix_tree_descend(parent, &node, offset);
+		slot = parent->slots + offset;
+	}
 
 	if (nodep)
 		*nodep = parent;
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 18/30] radix-tree: Fix multiorder BUG_ON in radix_tree_insert
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (16 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 17/30] radix-tree: Rewrite __radix_tree_lookup Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 19/30] radix-tree: add support for multi-order iterating Matthew Wilcox
                   ` (11 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

These BUG_ON tests are to ensure that all the tags are clear when
inserting a new entry.  If we insert a multiorder entry, we'll end up
looking at the tags for a different node, and so the BUG_ON can end up
triggering spuriously.

Also, we now have three tags, not two, so check all three are clear,
and check all the root tags with a single call to BUG_ON since the bits
are stored contiguously.

Include a test-case to ensure this problem does not reoccur.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c                      | 16 ++++++++++++----
 tools/testing/radix-tree/multiorder.c | 13 +++++++++++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 2ca2b416b476..33f91f207587 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -165,6 +165,11 @@ static inline int root_tag_get(struct radix_tree_root *root, unsigned int tag)
 	return (__force unsigned)root->gfp_mask & (1 << (tag + __GFP_BITS_SHIFT));
 }
 
+static inline unsigned root_tags_get(struct radix_tree_root *root)
+{
+	return (__force unsigned)root->gfp_mask >> __GFP_BITS_SHIFT;
+}
+
 /*
  * Returns 1 if any slot in the node has this tag set.
  * Otherwise returns 0.
@@ -604,12 +609,15 @@ int __radix_tree_insert(struct radix_tree_root *root, unsigned long index,
 	rcu_assign_pointer(*slot, item);
 
 	if (node) {
+		unsigned shift = ((node->path - 1) & RADIX_TREE_HEIGHT_MASK) *
+					RADIX_TREE_MAP_SHIFT;
+		unsigned offset = (index >> shift) & RADIX_TREE_MAP_MASK;
 		node->count++;
-		BUG_ON(tag_get(node, 0, index & RADIX_TREE_MAP_MASK));
-		BUG_ON(tag_get(node, 1, index & RADIX_TREE_MAP_MASK));
+		BUG_ON(tag_get(node, 0, offset));
+		BUG_ON(tag_get(node, 1, offset));
+		BUG_ON(tag_get(node, 2, offset));
 	} else {
-		BUG_ON(root_tag_get(root, 0));
-		BUG_ON(root_tag_get(root, 1));
+		BUG_ON(root_tags_get(root));
 	}
 
 	return 0;
diff --git a/tools/testing/radix-tree/multiorder.c b/tools/testing/radix-tree/multiorder.c
index cfe718c78eb6..606bfe04b104 100644
--- a/tools/testing/radix-tree/multiorder.c
+++ b/tools/testing/radix-tree/multiorder.c
@@ -46,6 +46,17 @@ static void multiorder_check(unsigned long index, int order)
 		item_check_absent(&tree, i);
 }
 
+static void multiorder_insert_bug(void)
+{
+	RADIX_TREE(tree, GFP_KERNEL);
+
+	item_insert(&tree, 0);
+	radix_tree_tag_set(&tree, 0, 0);
+	item_insert_order(&tree, 3 << 6, 6);
+
+	item_kill_tree(&tree);
+}
+
 void multiorder_checks(void)
 {
 	int i;
@@ -55,4 +66,6 @@ void multiorder_checks(void)
 		multiorder_check(0, i);
 		multiorder_check((1UL << i) + 1, i);
 	}
+
+	multiorder_insert_bug();
 }
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 19/30] radix-tree: add support for multi-order iterating
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (17 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 18/30] radix-tree: Fix multiorder BUG_ON in radix_tree_insert Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 20/30] radix tree test suite: multi-order iteration test Matthew Wilcox
                   ` (10 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

This enables the macros radix_tree_for_each_slot() and friends to be
used with multi-order entries.

The way that this works is that we treat all entries in a given slots[]
array as a single chunk.  If the index given to radix_tree_next_chunk()
happens to point us to a sibling entry, we will back up iter->index so
that it points to the canonical entry, and that will be the place where
we start our iteration.

As we're processing a chunk in radix_tree_next_slot(), we process
canonical entries, skip over sibling entries, and restart the chunk
lookup if we find a non-sibling indirect pointer.  This drops back to
the radix_tree_next_chunk() code, which will re-walk the tree and look
for another chunk.

This allows us to properly handle multi-order entries mixed with other
entries that are at various heights in the radix tree.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 include/linux/radix-tree.h                    | 66 +++++++++++++++++++++++----
 lib/radix-tree.c                              | 60 +++++++++++++-----------
 tools/testing/radix-tree/generated/autoconf.h |  3 ++
 tools/testing/radix-tree/linux/kernel.h       |  5 +-
 4 files changed, 95 insertions(+), 39 deletions(-)
 create mode 100644 tools/testing/radix-tree/generated/autoconf.h

diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index e1512a607709..1d6686638591 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -330,8 +330,9 @@ static inline void radix_tree_preload_end(void)
  * struct radix_tree_iter - radix tree iterator state
  *
  * @index:	index of current slot
- * @next_index:	next-to-last index for this chunk
+ * @next_index:	one beyond the last index for this chunk
  * @tags:	bit-mask for tag-iterating
+ * @shift:	shift for the node that holds our slots
  *
  * This radix tree iterator works in terms of "chunks" of slots.  A chunk is a
  * subinterval of slots contained within one radix tree leaf node.  It is
@@ -344,6 +345,9 @@ struct radix_tree_iter {
 	unsigned long	index;
 	unsigned long	next_index;
 	unsigned long	tags;
+#ifdef CONFIG_RADIX_TREE_MULTIORDER
+	unsigned int	shift;
+#endif
 };
 
 #define RADIX_TREE_ITER_TAG_MASK	0x00FF	/* tag index in lower byte */
@@ -405,6 +409,16 @@ void **radix_tree_iter_retry(struct radix_tree_iter *iter)
 	return NULL;
 }
 
+static inline unsigned long
+__radix_tree_iter_add(struct radix_tree_iter *iter, unsigned long slots)
+{
+#ifdef CONFIG_RADIX_TREE_MULTIORDER
+	return iter->index + (slots << iter->shift);
+#else
+	return iter->index + slots;
+#endif
+}
+
 /**
  * radix_tree_iter_next - resume iterating when the chunk may be invalid
  * @iter:	iterator state
@@ -416,7 +430,7 @@ void **radix_tree_iter_retry(struct radix_tree_iter *iter)
 static inline __must_check
 void **radix_tree_iter_next(struct radix_tree_iter *iter)
 {
-	iter->next_index = iter->index + 1;
+	iter->next_index = __radix_tree_iter_add(iter, 1);
 	iter->tags = 0;
 	return NULL;
 }
@@ -430,7 +444,16 @@ void **radix_tree_iter_next(struct radix_tree_iter *iter)
 static __always_inline long
 radix_tree_chunk_size(struct radix_tree_iter *iter)
 {
+#ifdef CONFIG_RADIX_TREE_MULTIORDER
+	return (iter->next_index - iter->index) >> iter->shift;
+#else
 	return iter->next_index - iter->index;
+#endif
+}
+
+static inline void *indirect_to_ptr(void *ptr)
+{
+	return (void *)((unsigned long)ptr & ~RADIX_TREE_INDIRECT_PTR);
 }
 
 /**
@@ -448,24 +471,51 @@ static __always_inline void **
 radix_tree_next_slot(void **slot, struct radix_tree_iter *iter, unsigned flags)
 {
 	if (flags & RADIX_TREE_ITER_TAGGED) {
+		void *canon = slot;
+
 		iter->tags >>= 1;
+		if (unlikely(!iter->tags))
+			return NULL;
+		while (IS_ENABLED(CONFIG_RADIX_TREE_MULTIORDER) &&
+					radix_tree_is_indirect_ptr(slot[1])) {
+			if (indirect_to_ptr(slot[1]) == canon) {
+				iter->tags >>= 1;
+				iter->index = __radix_tree_iter_add(iter, 1);
+				slot++;
+				continue;
+			}
+			iter->next_index = __radix_tree_iter_add(iter, 1);
+			return NULL;
+		}
 		if (likely(iter->tags & 1ul)) {
-			iter->index++;
+			iter->index = __radix_tree_iter_add(iter, 1);
 			return slot + 1;
 		}
-		if (!(flags & RADIX_TREE_ITER_CONTIG) && likely(iter->tags)) {
+		if (!(flags & RADIX_TREE_ITER_CONTIG)) {
 			unsigned offset = __ffs(iter->tags);
 
 			iter->tags >>= offset;
-			iter->index += offset + 1;
+			iter->index = __radix_tree_iter_add(iter, offset + 1);
 			return slot + offset + 1;
 		}
 	} else {
-		long size = radix_tree_chunk_size(iter);
+		long count = radix_tree_chunk_size(iter);
+		void *canon = slot;
 
-		while (--size > 0) {
+		while (--count > 0) {
 			slot++;
-			iter->index++;
+			iter->index = __radix_tree_iter_add(iter, 1);
+
+			if (IS_ENABLED(CONFIG_RADIX_TREE_MULTIORDER) &&
+			    radix_tree_is_indirect_ptr(*slot)) {
+				if (indirect_to_ptr(*slot) == canon)
+					continue;
+				else {
+					iter->next_index = iter->index;
+					break;
+				}
+			}
+
 			if (likely(*slot))
 				return slot;
 			if (flags & RADIX_TREE_ITER_CONTIG) {
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 33f91f207587..efcb8ed4b96b 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -75,11 +75,6 @@ static inline void *ptr_to_indirect(void *ptr)
 	return (void *)((unsigned long)ptr | RADIX_TREE_INDIRECT_PTR);
 }
 
-static inline void *indirect_to_ptr(void *ptr)
-{
-	return (void *)((unsigned long)ptr & ~RADIX_TREE_INDIRECT_PTR);
-}
-
 #define RADIX_TREE_RETRY	ptr_to_indirect(NULL)
 
 #ifdef CONFIG_RADIX_TREE_MULTIORDER
@@ -892,6 +887,13 @@ int radix_tree_tag_get(struct radix_tree_root *root,
 }
 EXPORT_SYMBOL(radix_tree_tag_get);
 
+static inline void __set_iter_shift(struct radix_tree_iter*iter, unsigned shift)
+{
+#ifdef CONFIG_RADIX_TREE_MULTIORDER
+	iter->shift = shift;
+#endif
+}
+
 /**
  * radix_tree_next_chunk - find next chunk of slots for iteration
  *
@@ -905,7 +907,7 @@ void **radix_tree_next_chunk(struct radix_tree_root *root,
 {
 	unsigned shift, tag = flags & RADIX_TREE_ITER_TAG_MASK;
 	struct radix_tree_node *rnode, *node;
-	unsigned long index, offset, height;
+	unsigned long index, offset, maxindex;
 
 	if ((flags & RADIX_TREE_ITER_TAGGED) && !root_tag_get(root, tag))
 		return NULL;
@@ -923,33 +925,39 @@ void **radix_tree_next_chunk(struct radix_tree_root *root,
 	if (!index && iter->index)
 		return NULL;
 
-	rnode = rcu_dereference_raw(root->rnode);
+restart:
+	shift = radix_tree_load_root(root, &rnode, &maxindex);
+	if (index > maxindex)
+		return NULL;
+
 	if (radix_tree_is_indirect_ptr(rnode)) {
 		rnode = indirect_to_ptr(rnode);
-	} else if (rnode && !index) {
+	} else if (rnode) {
 		/* Single-slot tree */
-		iter->index = 0;
-		iter->next_index = 1;
+		iter->index = index;
+		iter->next_index = maxindex + 1;
 		iter->tags = 1;
+		__set_iter_shift(iter, shift);
 		return (void **)&root->rnode;
 	} else
 		return NULL;
 
-restart:
-	height = rnode->path & RADIX_TREE_HEIGHT_MASK;
-	shift = (height - 1) * RADIX_TREE_MAP_SHIFT;
+	shift -= RADIX_TREE_MAP_SHIFT;
 	offset = index >> shift;
 
-	/* Index outside of the tree */
-	if (offset >= RADIX_TREE_MAP_SIZE)
-		return NULL;
-
 	node = rnode;
 	while (1) {
 		struct radix_tree_node *slot;
+		unsigned new_off = radix_tree_descend(node, &slot, offset);
+
+		if (new_off < offset) {
+			offset = new_off;
+			index &= ~((RADIX_TREE_MAP_SIZE << shift) - 1);
+			index |= offset << shift;
+		}
+
 		if ((flags & RADIX_TREE_ITER_TAGGED) ?
-				!test_bit(offset, node->tags[tag]) :
-				!node->slots[offset]) {
+				!tag_get(node, tag, offset) : !slot) {
 			/* Hole detected */
 			if (flags & RADIX_TREE_ITER_CONTIG)
 				return NULL;
@@ -971,25 +979,23 @@ restart:
 				return NULL;
 			if (offset == RADIX_TREE_MAP_SIZE)
 				goto restart;
+			slot = rcu_dereference_raw(node->slots[offset]);
 		}
 
-		/* This is leaf-node */
-		if (!shift)
-			break;
-
-		slot = rcu_dereference_raw(node->slots[offset]);
-		if (slot == NULL)
+		if ((slot == NULL) || (slot == RADIX_TREE_RETRY))
 			goto restart;
 		if (!radix_tree_is_indirect_ptr(slot))
 			break;
+
 		node = indirect_to_ptr(slot);
 		shift -= RADIX_TREE_MAP_SHIFT;
 		offset = (index >> shift) & RADIX_TREE_MAP_MASK;
 	}
 
 	/* Update the iterator state */
-	iter->index = index;
-	iter->next_index = (index | RADIX_TREE_MAP_MASK) + 1;
+	iter->index = index & ~((1 << shift) - 1);
+	iter->next_index = (index | ((RADIX_TREE_MAP_SIZE << shift) - 1)) + 1;
+	__set_iter_shift(iter, shift);
 
 	/* Construct iter->tags bit-mask from node->tags[tag] array */
 	if (flags & RADIX_TREE_ITER_TAGGED) {
diff --git a/tools/testing/radix-tree/generated/autoconf.h b/tools/testing/radix-tree/generated/autoconf.h
new file mode 100644
index 000000000000..ad18cf5a2a3a
--- /dev/null
+++ b/tools/testing/radix-tree/generated/autoconf.h
@@ -0,0 +1,3 @@
+#define CONFIG_RADIX_TREE_MULTIORDER 1
+#define CONFIG_SHMEM 1
+#define CONFIG_SWAP 1
diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
index 8ea0ed450810..be98a47b4e1b 100644
--- a/tools/testing/radix-tree/linux/kernel.h
+++ b/tools/testing/radix-tree/linux/kernel.h
@@ -8,10 +8,7 @@
 #include <limits.h>
 
 #include "../../include/linux/compiler.h"
-
-#define CONFIG_RADIX_TREE_MULTIORDER
-#define CONFIG_SHMEM
-#define CONFIG_SWAP
+#include "../../../include/linux/kconfig.h"
 
 #define RADIX_TREE_MAP_SHIFT	3
 
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 20/30] radix tree test suite: multi-order iteration test
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (18 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 19/30] radix-tree: add support for multi-order iterating Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 21/30] radix tree test suite: Add multiorder shrinking test Matthew Wilcox
                   ` (9 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

Add a unit test to verify that we can iterate over multi-order entries
properly via a radix_tree_for_each_slot() loop.

This was done with a single, somewhat complicated configuration that was
meant to test many of the various corner cases having to do with
multi-order entries:

- An iteration could begin at a sibling entry, and we need to return the
  canonical entry.
- We could have entries of various orders in the same slots[] array.
- We could have multi-order entries at a nonzero height, followed by
  indirect pointers to more radix tree nodes later in that same slots[]
  array.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 tools/testing/radix-tree/multiorder.c | 92 +++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/tools/testing/radix-tree/multiorder.c b/tools/testing/radix-tree/multiorder.c
index 606bfe04b104..583c5127fbcf 100644
--- a/tools/testing/radix-tree/multiorder.c
+++ b/tools/testing/radix-tree/multiorder.c
@@ -57,6 +57,96 @@ static void multiorder_insert_bug(void)
 	item_kill_tree(&tree);
 }
 
+void multiorder_iteration(void)
+{
+	RADIX_TREE(tree, GFP_KERNEL);
+	struct radix_tree_iter iter;
+	void **slot;
+	int i, err;
+
+	printf("Multiorder iteration test\n");
+
+#define NUM_ENTRIES 11
+	int index[NUM_ENTRIES] = {0, 2, 4, 8, 16, 32, 34, 36, 64, 72, 128};
+	int order[NUM_ENTRIES] = {1, 1, 2, 3,  4,  1,  0,  1,  3,  0, 7};
+
+	for (i = 0; i < NUM_ENTRIES; i++) {
+		err = item_insert_order(&tree, index[i], order[i]);
+		assert(!err);
+	}
+
+	i = 0;
+	/* start from index 1 to verify we find the multi-order entry at 0 */
+	radix_tree_for_each_slot(slot, &tree, &iter, 1) {
+		int height = order[i] / RADIX_TREE_MAP_SHIFT;
+		int shift = height * RADIX_TREE_MAP_SHIFT;
+
+		assert(iter.index == index[i]);
+		assert(iter.shift == shift);
+		i++;
+	}
+
+	/*
+	 * Now iterate through the tree starting at an elevated multi-order
+	 * entry, beginning at an index in the middle of the range.
+	 */
+	i = 8;
+	radix_tree_for_each_slot(slot, &tree, &iter, 70) {
+		int height = order[i] / RADIX_TREE_MAP_SHIFT;
+		int shift = height * RADIX_TREE_MAP_SHIFT;
+
+		assert(iter.index == index[i]);
+		assert(iter.shift == shift);
+		i++;
+	}
+
+	item_kill_tree(&tree);
+}
+
+void multiorder_tagged_iteration(void)
+{
+	RADIX_TREE(tree, GFP_KERNEL);
+	struct radix_tree_iter iter;
+	void **slot;
+	int i;
+
+	printf("Multiorder tagged iteration test\n");
+
+#define MT_NUM_ENTRIES 9
+	int index[MT_NUM_ENTRIES] = {0, 2, 4, 16, 32, 40, 64, 72, 128};
+	int order[MT_NUM_ENTRIES] = {1, 0, 2, 4,  3,  1,  3,  0,   7};
+
+#define TAG_ENTRIES 7
+	int tag_index[TAG_ENTRIES] = {0, 4, 16, 40, 64, 72, 128};
+
+	for (i = 0; i < MT_NUM_ENTRIES; i++)
+		assert(!item_insert_order(&tree, index[i], order[i]));
+
+	assert(!radix_tree_tagged(&tree, 1));
+
+	for (i = 0; i < TAG_ENTRIES; i++)
+		assert(radix_tree_tag_set(&tree, tag_index[i], 1));
+
+	i = 0;
+	/* start from index 1 to verify we find the multi-order entry at 0 */
+	radix_tree_for_each_tagged(slot, &tree, &iter, 1, 1) {
+		assert(iter.index == tag_index[i]);
+		i++;
+	}
+
+	/*
+	 * Now iterate through the tree starting at an elevated multi-order
+	 * entry, beginning at an index in the middle of the range.
+	 */
+	i = 4;
+	radix_tree_for_each_slot(slot, &tree, &iter, 70) {
+		assert(iter.index == tag_index[i]);
+		i++;
+	}
+
+	item_kill_tree(&tree);
+}
+
 void multiorder_checks(void)
 {
 	int i;
@@ -67,5 +157,7 @@ void multiorder_checks(void)
 		multiorder_check((1UL << i) + 1, i);
 	}
 
+	multiorder_iteration();
+	multiorder_tagged_iteration();
 	multiorder_insert_bug();
 }
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 21/30] radix tree test suite: Add multiorder shrinking test
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (19 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 20/30] radix tree test suite: multi-order iteration test Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 22/30] radix-tree: Rewrite radix_tree_tag_set Matthew Wilcox
                   ` (8 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

Ensure that the tree goes back down to the same height when an item is
inserted & removed from the tree at a higher index.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 tools/testing/radix-tree/multiorder.c | 38 +++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/testing/radix-tree/multiorder.c b/tools/testing/radix-tree/multiorder.c
index 583c5127fbcf..10b9708a71f9 100644
--- a/tools/testing/radix-tree/multiorder.c
+++ b/tools/testing/radix-tree/multiorder.c
@@ -46,6 +46,41 @@ static void multiorder_check(unsigned long index, int order)
 		item_check_absent(&tree, i);
 }
 
+static void multiorder_shrink(unsigned long index, int order)
+{
+	unsigned long i;
+	unsigned long max = 1 << order;
+	RADIX_TREE(tree, GFP_KERNEL);
+	struct radix_tree_node *node;
+
+	printf("Multiorder shrink index %ld, order %d\n", index, order);
+
+	assert(item_insert_order(&tree, 0, order) == 0);
+
+	node = tree.rnode;
+
+	assert(item_insert(&tree, index) == 0);
+	assert(node != tree.rnode);
+
+	assert(item_delete(&tree, index) != 0);
+	assert(node == tree.rnode);
+
+	for (i = 0; i < max; i++) {
+		struct item *item = item_lookup(&tree, i);
+		assert(item != 0);
+		assert(item->index == 0);
+	}
+	for (i = max; i < 2*max; i++)
+		item_check_absent(&tree, i);
+
+	if (!item_delete(&tree, 0)) {
+		printf("failed to delete index %ld (order %d)\n", index, order);		abort();
+	}
+
+	for (i = 0; i < 2*max; i++)
+		item_check_absent(&tree, i);
+}
+
 static void multiorder_insert_bug(void)
 {
 	RADIX_TREE(tree, GFP_KERNEL);
@@ -157,6 +192,9 @@ void multiorder_checks(void)
 		multiorder_check((1UL << i) + 1, i);
 	}
 
+	for (i = 0; i < 15; i++)
+		multiorder_shrink((1UL << (i + RADIX_TREE_MAP_SHIFT)), i);
+
 	multiorder_iteration();
 	multiorder_tagged_iteration();
 	multiorder_insert_bug();
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 22/30] radix-tree: Rewrite radix_tree_tag_set
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (20 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 21/30] radix tree test suite: Add multiorder shrinking test Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 23/30] radix-tree: Rewrite radix_tree_tag_clear Matthew Wilcox
                   ` (7 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

Use the new multi-order support functions to rewrite radix_tree_tag_set()

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 lib/radix-tree.c | 37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index efcb8ed4b96b..6d9b328705a1 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -729,35 +729,32 @@ EXPORT_SYMBOL(radix_tree_lookup);
 void *radix_tree_tag_set(struct radix_tree_root *root,
 			unsigned long index, unsigned int tag)
 {
-	unsigned int height, shift;
-	struct radix_tree_node *slot;
-
-	height = root->height;
-	BUG_ON(index > radix_tree_maxindex(height));
+	struct radix_tree_node *node, *parent;
+	unsigned long maxindex;
+	unsigned int shift;
 
-	slot = indirect_to_ptr(root->rnode);
-	shift = (height - 1) * RADIX_TREE_MAP_SHIFT;
+	shift = radix_tree_load_root(root, &node, &maxindex);
+	BUG_ON(index > maxindex);
 
-	while (height > 0) {
-		int offset;
+	while (radix_tree_is_indirect_ptr(node)) {
+		unsigned offset;
 
-		offset = (index >> shift) & RADIX_TREE_MAP_MASK;
-		if (!tag_get(slot, tag, offset))
-			tag_set(slot, tag, offset);
-		slot = slot->slots[offset];
-		BUG_ON(slot == NULL);
-		if (!radix_tree_is_indirect_ptr(slot))
-			break;
-		slot = indirect_to_ptr(slot);
 		shift -= RADIX_TREE_MAP_SHIFT;
-		height--;
+		offset = (index >> shift) & RADIX_TREE_MAP_MASK;
+
+		parent = indirect_to_ptr(node);
+		offset = radix_tree_descend(parent, &node, offset);
+		BUG_ON(!node);
+
+		if (!tag_get(parent, tag, offset))
+			tag_set(parent, tag, offset);
 	}
 
 	/* set the root's tag bit */
-	if (slot && !root_tag_get(root, tag))
+	if (!root_tag_get(root, tag))
 		root_tag_set(root, tag);
 
-	return slot;
+	return node;
 }
 EXPORT_SYMBOL(radix_tree_tag_set);
 
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 23/30] radix-tree: Rewrite radix_tree_tag_clear
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (21 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 22/30] radix-tree: Rewrite radix_tree_tag_set Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 24/30] radix-tree: Rewrite radix_tree_tag_get Matthew Wilcox
                   ` (6 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

Use the new multi-order support functions to rewrite
radix_tree_tag_clear()

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 lib/radix-tree.c | 44 ++++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 6d9b328705a1..aca7b2814d26 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -775,44 +775,40 @@ EXPORT_SYMBOL(radix_tree_tag_set);
 void *radix_tree_tag_clear(struct radix_tree_root *root,
 			unsigned long index, unsigned int tag)
 {
-	struct radix_tree_node *node = NULL;
-	struct radix_tree_node *slot = NULL;
-	unsigned int height, shift;
+	struct radix_tree_node *node, *parent;
+	unsigned long maxindex;
+	unsigned int shift;
 	int uninitialized_var(offset);
 
-	height = root->height;
-	if (index > radix_tree_maxindex(height))
-		goto out;
-
-	shift = height * RADIX_TREE_MAP_SHIFT;
-	slot = root->rnode;
+	shift = radix_tree_load_root(root, &node, &maxindex);
+	if (index > maxindex)
+		return NULL;
 
-	while (shift) {
-		if (slot == NULL)
-			goto out;
-		if (!radix_tree_is_indirect_ptr(slot))
-			break;
-		slot = indirect_to_ptr(slot);
+	parent = NULL;
 
+	while (radix_tree_is_indirect_ptr(node)) {
 		shift -= RADIX_TREE_MAP_SHIFT;
 		offset = (index >> shift) & RADIX_TREE_MAP_MASK;
-		node = slot;
-		slot = slot->slots[offset];
+
+		parent = indirect_to_ptr(node);
+		offset = radix_tree_descend(parent, &node, offset);
 	}
 
-	if (slot == NULL)
+	if (node == NULL)
 		goto out;
 
-	while (node) {
-		if (!tag_get(node, tag, offset))
+	index >>= shift;
+
+	while (parent) {
+		if (!tag_get(parent, tag, offset))
 			goto out;
-		tag_clear(node, tag, offset);
-		if (any_tag_set(node, tag))
+		tag_clear(parent, tag, offset);
+		if (any_tag_set(parent, tag))
 			goto out;
 
 		index >>= RADIX_TREE_MAP_SHIFT;
 		offset = index & RADIX_TREE_MAP_MASK;
-		node = node->parent;
+		parent = parent->parent;
 	}
 
 	/* clear the root's tag bit */
@@ -820,7 +816,7 @@ void *radix_tree_tag_clear(struct radix_tree_root *root,
 		root_tag_clear(root, tag);
 
 out:
-	return slot;
+	return node;
 }
 EXPORT_SYMBOL(radix_tree_tag_clear);
 
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 24/30] radix-tree: Rewrite radix_tree_tag_get
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (22 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 23/30] radix-tree: Rewrite radix_tree_tag_clear Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 25/30] radix-tree test suite: add multi-order tag test Matthew Wilcox
                   ` (5 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

Use the new multi-order support functions to rewrite radix_tree_tag_get()

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 lib/radix-tree.c | 44 ++++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index aca7b2814d26..d894654b5ecc 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -838,45 +838,37 @@ EXPORT_SYMBOL(radix_tree_tag_clear);
 int radix_tree_tag_get(struct radix_tree_root *root,
 			unsigned long index, unsigned int tag)
 {
-	unsigned int height, shift;
-	struct radix_tree_node *node;
+	struct radix_tree_node *node, *parent;
+	unsigned long maxindex;
+	unsigned int shift;
 
-	/* check the root's tag bit */
 	if (!root_tag_get(root, tag))
 		return 0;
 
-	node = rcu_dereference_raw(root->rnode);
+	shift = radix_tree_load_root(root, &node, &maxindex);
+	if (index > maxindex)
+		return 0;
 	if (node == NULL)
 		return 0;
 
-	if (!radix_tree_is_indirect_ptr(node))
-		return (index == 0);
-	node = indirect_to_ptr(node);
-
-	height = node->path & RADIX_TREE_HEIGHT_MASK;
-	if (index > radix_tree_maxindex(height))
-		return 0;
+	while (radix_tree_is_indirect_ptr(node)) {
+		int offset;
 
-	shift = (height - 1) * RADIX_TREE_MAP_SHIFT;
+		shift -= RADIX_TREE_MAP_SHIFT;
+		offset = (index >> shift) & RADIX_TREE_MAP_MASK;
 
-	for ( ; ; ) {
-		int offset;
+		parent = indirect_to_ptr(node);
+		offset = radix_tree_descend(parent, &node, offset);
 
-		if (node == NULL)
+		if (!node)
 			return 0;
-		node = indirect_to_ptr(node);
-
-		offset = (index >> shift) & RADIX_TREE_MAP_MASK;
-		if (!tag_get(node, tag, offset))
+		if (!tag_get(parent, tag, offset))
 			return 0;
-		if (height == 1)
-			return 1;
-		node = rcu_dereference_raw(node->slots[offset]);
-		if (!radix_tree_is_indirect_ptr(node))
-			return 1;
-		shift -= RADIX_TREE_MAP_SHIFT;
-		height--;
+		if (node == RADIX_TREE_RETRY)
+			break;
 	}
+
+	return 1;
 }
 EXPORT_SYMBOL(radix_tree_tag_get);
 
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 25/30] radix-tree test suite: add multi-order tag test
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (23 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 24/30] radix-tree: Rewrite radix_tree_tag_get Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 26/30] radix-tree: Fix radix_tree_create for sibling entries Matthew Wilcox
                   ` (4 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

Add a generic test for multi-order tag verification, and call it using
several different configurations.

This test creates a multi-order radix tree using the given index and order,
and then sets, checks and clears tags using the indices covered by the
single multi-order radix tree entry.

With the various calls done by this test we verify root multi-order entries
without siblings, multi-order entries without siblings in a radix tree
node, as well as multi-order entries with siblings of various sizes.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 tools/testing/radix-tree/multiorder.c | 97 +++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/tools/testing/radix-tree/multiorder.c b/tools/testing/radix-tree/multiorder.c
index 10b9708a71f9..de3d0e96f987 100644
--- a/tools/testing/radix-tree/multiorder.c
+++ b/tools/testing/radix-tree/multiorder.c
@@ -19,6 +19,102 @@
 
 #include "test.h"
 
+#define for_each_index(i, base, order) \
+	for (i = base; i < base + (1 << order); i++)
+
+static void __multiorder_tag_test(int index, int order)
+{
+	RADIX_TREE(tree, GFP_KERNEL);
+	int base, err, i;
+
+	/* our canonical entry */
+	base = index & ~((1 << order) - 1);
+
+	printf("Multiorder tag test with index %d, canonical entry %d\n",
+			index, base);
+
+	err = item_insert_order(&tree, index, order);
+	assert(!err);
+
+	/*
+	 * Verify we get collisions for covered indices.  We try and fail to
+	 * insert an exceptional entry so we don't leak memory via
+	 * item_insert_order().
+	 */
+	for_each_index(i, base, order) {
+		err = __radix_tree_insert(&tree, i, order,
+				(void *)(0xA0 | RADIX_TREE_EXCEPTIONAL_ENTRY));
+		assert(err == -EEXIST);
+	}
+
+	for_each_index(i, base, order) {
+		assert(!radix_tree_tag_get(&tree, i, 0));
+		assert(!radix_tree_tag_get(&tree, i, 1));
+	}
+
+	assert(radix_tree_tag_set(&tree, index, 0));
+
+	for_each_index(i, base, order) {
+		assert(radix_tree_tag_get(&tree, i, 0));
+		assert(!radix_tree_tag_get(&tree, i, 1));
+	}
+
+	assert(radix_tree_tag_clear(&tree, index, 0));
+
+	for_each_index(i, base, order) {
+		assert(!radix_tree_tag_get(&tree, i, 0));
+		assert(!radix_tree_tag_get(&tree, i, 1));
+	}
+
+	assert(!radix_tree_tagged(&tree, 0));
+	assert(!radix_tree_tagged(&tree, 1));
+
+	item_kill_tree(&tree);
+}
+
+static void multiorder_tag_tests(void)
+{
+	/* test multi-order entry for indices 0-7 with no sibling pointers */
+	__multiorder_tag_test(0, 3);
+	__multiorder_tag_test(5, 3);
+
+	/* test multi-order entry for indices 8-15 with no sibling pointers */
+	__multiorder_tag_test(8, 3);
+	__multiorder_tag_test(15, 3);
+
+	/*
+	 * Our order 5 entry covers indices 0-31 in a tree with height=2.
+	 * This is broken up as follows:
+	 * 0-7:		canonical entry
+	 * 8-15:	sibling 1
+	 * 16-23:	sibling 2
+	 * 24-31:	sibling 3
+	 */
+	__multiorder_tag_test(0, 5);
+	__multiorder_tag_test(29, 5);
+
+	/* same test, but with indices 32-63 */
+	__multiorder_tag_test(32, 5);
+	__multiorder_tag_test(44, 5);
+
+	/*
+	 * Our order 8 entry covers indices 0-255 in a tree with height=3.
+	 * This is broken up as follows:
+	 * 0-63:	canonical entry
+	 * 64-127:	sibling 1
+	 * 128-191:	sibling 2
+	 * 192-255:	sibling 3
+	 */
+	__multiorder_tag_test(0, 8);
+	__multiorder_tag_test(190, 8);
+
+	/* same test, but with indices 256-511 */
+	__multiorder_tag_test(256, 8);
+	__multiorder_tag_test(300, 8);
+
+	__multiorder_tag_test(0x12345678UL, 8);
+}
+
 static void multiorder_check(unsigned long index, int order)
 {
 	unsigned long i;
@@ -195,6 +291,7 @@ void multiorder_checks(void)
 	for (i = 0; i < 15; i++)
 		multiorder_shrink((1UL << (i + RADIX_TREE_MAP_SHIFT)), i);
 
+	multiorder_tag_tests();
 	multiorder_iteration();
 	multiorder_tagged_iteration();
 	multiorder_insert_bug();
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 26/30] radix-tree: Fix radix_tree_create for sibling entries
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (24 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 25/30] radix-tree test suite: add multi-order tag test Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 27/30] radix-tree: Rewrite radix_tree_locate_item Matthew Wilcox
                   ` (3 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

If the radix tree user attempted to insert a colliding entry with an
existing multiorder entry, then radix_tree_create() could encounter
a sibling entry when walking down the tree to look for a slot.
Use radix_tree_descend() to fix the problem, and add a test-case to make
sure the problem doesn't come back in future.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c                      | 4 ++--
 tools/testing/radix-tree/multiorder.c | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index d894654b5ecc..4291f344cb95 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -548,9 +548,9 @@ int __radix_tree_create(struct radix_tree_root *root, unsigned long index,
 		/* Go a level down */
 		height--;
 		shift -= RADIX_TREE_MAP_SHIFT;
-		offset = (index >> shift) & RADIX_TREE_MAP_MASK;
 		node = indirect_to_ptr(slot);
-		slot = node->slots[offset];
+		offset = (index >> shift) & RADIX_TREE_MAP_MASK;
+		offset = radix_tree_descend(node, &slot, offset);
 	}
 
 #ifdef CONFIG_RADIX_TREE_MULTIORDER
diff --git a/tools/testing/radix-tree/multiorder.c b/tools/testing/radix-tree/multiorder.c
index de3d0e96f987..0229e6fb590f 100644
--- a/tools/testing/radix-tree/multiorder.c
+++ b/tools/testing/radix-tree/multiorder.c
@@ -135,6 +135,11 @@ static void multiorder_check(unsigned long index, int order)
 		item_check_absent(&tree, i);
 	for (i = max; i < 2*max; i++)
 		item_check_absent(&tree, i);
+	for (i = min; i < max; i++) {
+		static void *entry = (void *)
+					(0xA0 | RADIX_TREE_EXCEPTIONAL_ENTRY);
+		assert(radix_tree_insert(&tree, i, entry) == -EEXIST);
+	}
 
 	assert(item_delete(&tree, index) != 0);
 
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 27/30] radix-tree: Rewrite radix_tree_locate_item
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (25 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 26/30] radix-tree: Fix radix_tree_create for sibling entries Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 28/30] radix-tree: Fix two bugs in radix_tree_range_tag_if_tagged() Matthew Wilcox
                   ` (2 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

Use the new multi-order support functions to rewrite
radix_tree_locate_item().  Modify the locate tests to test multiorder
entries too.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c                | 90 ++++++++++++++++++++---------------------
 tools/testing/radix-tree/main.c | 30 ++++++++------
 2 files changed, 63 insertions(+), 57 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 4291f344cb95..3ff25745896d 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -1306,58 +1306,55 @@ EXPORT_SYMBOL(radix_tree_gang_lookup_tag_slot);
 #if defined(CONFIG_SHMEM) && defined(CONFIG_SWAP)
 #include <linux/sched.h> /* for cond_resched() */
 
+struct locate_info {
+	unsigned long found_index;
+	bool stop;
+};
+
 /*
  * This linear search is at present only useful to shmem_unuse_inode().
  */
 static unsigned long __locate(struct radix_tree_node *slot, void *item,
-			      unsigned long index, unsigned long *found_index)
+			      unsigned long index, struct locate_info *info)
 {
 	unsigned int shift, height;
-	unsigned long i;
+	unsigned long base, i;
 
 	height = slot->path & RADIX_TREE_HEIGHT_MASK;
-	shift = (height-1) * RADIX_TREE_MAP_SHIFT;
+	shift = height * RADIX_TREE_MAP_SHIFT;
 
-	for ( ; height > 1; height--) {
-		i = (index >> shift) & RADIX_TREE_MAP_MASK;
-		for (;;) {
-			if (slot->slots[i] != NULL)
-				break;
-			index &= ~((1UL << shift) - 1);
-			index += 1UL << shift;
-			if (index == 0)
-				goto out;	/* 32-bit wraparound */
-			i++;
-			if (i == RADIX_TREE_MAP_SIZE)
+	do {
+		shift -= RADIX_TREE_MAP_SHIFT;
+		base = index & ~((1UL << shift) - 1);
+
+		for (i = (index >> shift) & RADIX_TREE_MAP_MASK;
+		     i < RADIX_TREE_MAP_SIZE;
+		     i++, index = base + (i << shift)) {
+			struct radix_tree_node *node =
+					rcu_dereference_raw(slot->slots[i]);
+			if (node == RADIX_TREE_RETRY)
 				goto out;
-		}
-
-		slot = rcu_dereference_raw(slot->slots[i]);
-		if (slot == NULL)
-			goto out;
-		if (!radix_tree_is_indirect_ptr(slot)) {
-			if (slot == item) {
-				*found_index = index + i;
-				index = 0;
-			} else {
-				index += shift;
+			if (!radix_tree_is_indirect_ptr(node)) {
+				if (node == item) {
+					info->found_index = index;
+					info->stop = true;
+					goto out;
+				}
+				continue;
 			}
-			goto out;
+			node = indirect_to_ptr(node);
+			if (is_sibling_entry(slot, node))
+				continue;
+			slot = node;
+			break;
 		}
-		slot = indirect_to_ptr(slot);
-		shift -= RADIX_TREE_MAP_SHIFT;
-	}
+		if (i == RADIX_TREE_MAP_SIZE)
+			break;
+	} while (shift);
 
-	/* Bottom level: check items */
-	for (i = 0; i < RADIX_TREE_MAP_SIZE; i++) {
-		if (slot->slots[i] == item) {
-			*found_index = index + i;
-			index = 0;
-			goto out;
-		}
-	}
-	index += RADIX_TREE_MAP_SIZE;
 out:
+	if ((index == 0) && (i == RADIX_TREE_MAP_SIZE))
+		info->stop = true;
 	return index;
 }
 
@@ -1375,7 +1372,10 @@ unsigned long radix_tree_locate_item(struct radix_tree_root *root, void *item)
 	struct radix_tree_node *node;
 	unsigned long max_index;
 	unsigned long cur_index = 0;
-	unsigned long found_index = -1;
+	struct locate_info info = {
+		.found_index = -1,
+		.stop = false,
+	};
 
 	do {
 		rcu_read_lock();
@@ -1383,24 +1383,24 @@ unsigned long radix_tree_locate_item(struct radix_tree_root *root, void *item)
 		if (!radix_tree_is_indirect_ptr(node)) {
 			rcu_read_unlock();
 			if (node == item)
-				found_index = 0;
+				info.found_index = 0;
 			break;
 		}
 
 		node = indirect_to_ptr(node);
-		max_index = radix_tree_maxindex(node->path &
-						RADIX_TREE_HEIGHT_MASK);
+
+		max_index = node_maxindex(node);
 		if (cur_index > max_index) {
 			rcu_read_unlock();
 			break;
 		}
 
-		cur_index = __locate(node, item, cur_index, &found_index);
+		cur_index = __locate(node, item, cur_index, &info);
 		rcu_read_unlock();
 		cond_resched();
-	} while (cur_index != 0 && cur_index <= max_index);
+	} while (!info.stop && cur_index <= max_index);
 
-	return found_index;
+	return info.found_index;
 }
 #else
 unsigned long radix_tree_locate_item(struct radix_tree_root *root, void *item)
diff --git a/tools/testing/radix-tree/main.c b/tools/testing/radix-tree/main.c
index b6a700b00cce..65231e9ba3e8 100644
--- a/tools/testing/radix-tree/main.c
+++ b/tools/testing/radix-tree/main.c
@@ -232,17 +232,18 @@ void copy_tag_check(void)
 	item_kill_tree(&tree);
 }
 
-void __locate_check(struct radix_tree_root *tree, unsigned long index)
+void __locate_check(struct radix_tree_root *tree, unsigned long index,
+			unsigned order)
 {
 	struct item *item;
 	unsigned long index2;
 
-	item_insert(tree, index);
+	item_insert_order(tree, index, order);
 	item = item_lookup(tree, index);
 	index2 = radix_tree_locate_item(tree, item);
 	if (index != index2) {
-		printf("index %ld inserted; found %ld\n",
-			index, index2);
+		printf("index %ld order %d inserted; found %ld\n",
+			index, order, index2);
 		abort();
 	}
 }
@@ -250,21 +251,26 @@ void __locate_check(struct radix_tree_root *tree, unsigned long index)
 static void locate_check(void)
 {
 	RADIX_TREE(tree, GFP_KERNEL);
+	unsigned order;
 	unsigned long offset, index;
 
-	for (offset = 0; offset < (1 << 3); offset++) {
-		for (index = 0; index < (1UL << 5); index++) {
-			__locate_check(&tree, index + offset);
-		}
-		if (radix_tree_locate_item(&tree, &tree) != -1)
-			abort();
+	for (order = 0; order < 20; order++) {
+		for (offset = 0; offset < (1 << (order + 3));
+		     offset += (1UL << order)) {
+			for (index = 0; index < (1UL << (order + 5));
+			     index += (1UL << order)) {
+				__locate_check(&tree, index + offset, order);
+			}
+			if (radix_tree_locate_item(&tree, &tree) != -1)
+				abort();
 
-		item_kill_tree(&tree);
+			item_kill_tree(&tree);
+		}
 	}
 
 	if (radix_tree_locate_item(&tree, &tree) != -1)
 		abort();
-	__locate_check(&tree, -1);
+	__locate_check(&tree, -1, 0);
 	if (radix_tree_locate_item(&tree, &tree) != -1)
 		abort();
 	item_kill_tree(&tree);
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 28/30] radix-tree: Fix two bugs in radix_tree_range_tag_if_tagged()
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (26 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 27/30] radix-tree: Rewrite radix_tree_locate_item Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 29/30] radix-tree: Fix radix_tree_dump() for multi-order entries Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 30/30] radix-tree: Add copyright statements Matthew Wilcox
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

I had previously decided that tagging a single multiorder entry would
count as tagging 2^order entries for the purposes of 'nr_to_tag'.
I now believe that decision to be a mistake, and it should count as a
single entry.  That's more likely to be what callers expect.

When walking back up the tree from a newly-tagged entry, the current
code assumed we were starting from the lowest level of the tree; if we
have a multiorder entry with an order at least RADIX_TREE_MAP_SHIFT in
size then we need to shift the index by 'shift' before we start walking
back up the tree, or we will end up not setting tags on higher entries,
and then mistakenly thinking that entries below a certain point in the
tree are not tagged.

Also convert to using radix_tree_load_root(), and add several tests for
radix_tree_range_tag_if_tagged().  At least one of these tests would
fail without the changes to the radix tree in this commit.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c                      | 20 ++++++++++----------
 tools/testing/radix-tree/multiorder.c | 16 +++++++++++++++-
 tools/testing/radix-tree/tag_check.c  | 10 ++++++++++
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 3ff25745896d..fa7842cb814e 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -1036,14 +1036,13 @@ unsigned long radix_tree_range_tag_if_tagged(struct radix_tree_root *root,
 		unsigned long nr_to_tag,
 		unsigned int iftag, unsigned int settag)
 {
-	unsigned int height = root->height;
-	struct radix_tree_node *node = NULL;
-	struct radix_tree_node *slot;
-	unsigned int shift;
+	struct radix_tree_node *slot, *node = NULL;
+	unsigned long maxindex;
+	unsigned int shift = radix_tree_load_root(root, &slot, &maxindex);
 	unsigned long tagged = 0;
 	unsigned long index = *first_indexp;
 
-	last_index = min(last_index, radix_tree_maxindex(height));
+	last_index = min(last_index, maxindex);
 	if (index > last_index)
 		return 0;
 	if (!nr_to_tag)
@@ -1052,14 +1051,14 @@ unsigned long radix_tree_range_tag_if_tagged(struct radix_tree_root *root,
 		*first_indexp = last_index + 1;
 		return 0;
 	}
-	if (height == 0) {
+	if (!radix_tree_is_indirect_ptr(slot)) {
 		*first_indexp = last_index + 1;
 		root_tag_set(root, settag);
 		return 1;
 	}
 
-	shift = (height - 1) * RADIX_TREE_MAP_SHIFT;
-	slot = indirect_to_ptr(root->rnode);
+	shift -= RADIX_TREE_MAP_SHIFT;
+	slot = indirect_to_ptr(slot);
 
 	for (;;) {
 		unsigned long upindex;
@@ -1075,6 +1074,7 @@ unsigned long radix_tree_range_tag_if_tagged(struct radix_tree_root *root,
 			slot = slot->slots[offset];
 			if (radix_tree_is_indirect_ptr(slot)) {
 				slot = indirect_to_ptr(slot);
+				/* Sibling slots are never tagged */
 				shift -= RADIX_TREE_MAP_SHIFT;
 				continue;
 			} else {
@@ -1084,11 +1084,11 @@ unsigned long radix_tree_range_tag_if_tagged(struct radix_tree_root *root,
 		}
 
 		/* tag the leaf */
-		tagged += 1 << shift;
+		tagged++;
 		tag_set(slot, settag, offset);
 
 		/* walk back up the path tagging interior nodes */
-		upindex = index;
+		upindex = index >> shift;
 		while (node) {
 			upindex >>= RADIX_TREE_MAP_SHIFT;
 			offset = upindex & RADIX_TREE_MAP_MASK;
diff --git a/tools/testing/radix-tree/multiorder.c b/tools/testing/radix-tree/multiorder.c
index 0229e6fb590f..5bc8870c13a8 100644
--- a/tools/testing/radix-tree/multiorder.c
+++ b/tools/testing/radix-tree/multiorder.c
@@ -26,6 +26,7 @@ static void __multiorder_tag_test(int index, int order)
 {
 	RADIX_TREE(tree, GFP_KERNEL);
 	int base, err, i;
+	unsigned long first = 0;
 
 	/* our canonical entry */
 	base = index & ~((1 << order) - 1);
@@ -59,13 +60,16 @@ static void __multiorder_tag_test(int index, int order)
 		assert(!radix_tree_tag_get(&tree, i, 1));
 	}
 
+	assert(radix_tree_range_tag_if_tagged(&tree, &first, ~0UL, 10, 0, 1) == 1);
 	assert(radix_tree_tag_clear(&tree, index, 0));
 
 	for_each_index(i, base, order) {
 		assert(!radix_tree_tag_get(&tree, i, 0));
-		assert(!radix_tree_tag_get(&tree, i, 1));
+		assert(radix_tree_tag_get(&tree, i, 1));
 	}
 
+	assert(radix_tree_tag_clear(&tree, index, 1));
+
 	assert(!radix_tree_tagged(&tree, 0));
 	assert(!radix_tree_tagged(&tree, 1));
 
@@ -244,6 +248,7 @@ void multiorder_tagged_iteration(void)
 	RADIX_TREE(tree, GFP_KERNEL);
 	struct radix_tree_iter iter;
 	void **slot;
+	unsigned long first = 0;
 	int i;
 
 	printf("Multiorder tagged iteration test\n");
@@ -280,6 +285,15 @@ void multiorder_tagged_iteration(void)
 		i++;
 	}
 
+	radix_tree_range_tag_if_tagged(&tree, &first, ~0UL,
+					MT_NUM_ENTRIES, 1, 2);
+
+	i = 0;
+	radix_tree_for_each_tagged(slot, &tree, &iter, 1, 2) {
+		assert(iter.index == tag_index[i]);
+		i++;
+	}
+
 	item_kill_tree(&tree);
 }
 
diff --git a/tools/testing/radix-tree/tag_check.c b/tools/testing/radix-tree/tag_check.c
index 83136be552a0..b7447ceb75e9 100644
--- a/tools/testing/radix-tree/tag_check.c
+++ b/tools/testing/radix-tree/tag_check.c
@@ -12,6 +12,7 @@
 static void
 __simple_checks(struct radix_tree_root *tree, unsigned long index, int tag)
 {
+	unsigned long first = 0;
 	int ret;
 
 	item_check_absent(tree, index);
@@ -22,6 +23,10 @@ __simple_checks(struct radix_tree_root *tree, unsigned long index, int tag)
 	item_tag_set(tree, index, tag);
 	ret = item_tag_get(tree, index, tag);
 	assert(ret != 0);
+	ret = radix_tree_range_tag_if_tagged(tree, &first, ~0UL, 10, tag, !tag);
+	assert(ret == 1);
+	ret = item_tag_get(tree, index, !tag);
+	assert(ret != 0);
 	ret = item_delete(tree, index);
 	assert(ret != 0);
 	item_insert(tree, index);
@@ -304,6 +309,7 @@ static void single_check(void)
 	struct item *items[BATCH];
 	RADIX_TREE(tree, GFP_KERNEL);
 	int ret;
+	unsigned long first = 0;
 
 	item_insert(&tree, 0);
 	item_tag_set(&tree, 0, 0);
@@ -313,6 +319,10 @@ static void single_check(void)
 	assert(ret == 0);
 	verify_tag_consistency(&tree, 0);
 	verify_tag_consistency(&tree, 1);
+	ret = radix_tree_range_tag_if_tagged(&tree, &first, 10, 10, 0, 1);
+	assert(ret == 1);
+	ret = radix_tree_gang_lookup_tag(&tree, (void **)items, 0, BATCH, 1);
+	assert(ret == 1);
 	item_kill_tree(&tree);
 }
 
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 29/30] radix-tree: Fix radix_tree_dump() for multi-order entries
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (27 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 28/30] radix-tree: Fix two bugs in radix_tree_range_tag_if_tagged() Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  2016-04-06 21:21 ` [PATCH 30/30] radix-tree: Add copyright statements Matthew Wilcox
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Ross Zwisler, linux-mm, linux-fsdevel, Konstantin Khlebnikov,
	Kirill Shutemov, Jan Kara, Neil Brown, Matthew Wilcox

From: Ross Zwisler <ross.zwisler@linux.intel.com>

 - Print which indices are covered by every leaf entry
 - Print sibling entries
 - Print the node pointer instead of the slot entry
 - Build by default in userspace, and make it accessible to the test-suite

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 lib/radix-tree.c                | 46 +++++++++++++++++++++++++----------------
 tools/testing/radix-tree/test.h |  1 +
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index fa7842cb814e..0402c4f1a344 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -215,27 +215,36 @@ radix_tree_find_next_bit(const unsigned long *addr,
 	return size;
 }
 
-#if 0
-static void dump_node(void *slot, int height, int offset)
+#ifndef __KERNEL__
+static void dump_node(struct radix_tree_node *node, unsigned offset,
+				unsigned shift, unsigned long index)
 {
-	struct radix_tree_node *node;
 	int i;
 
-	if (!slot)
-		return;
-
-	if (height == 0) {
-		pr_debug("radix entry %p offset %d\n", slot, offset);
-		return;
-	}
-
-	node = indirect_to_ptr(slot);
 	pr_debug("radix node: %p offset %d tags %lx %lx %lx path %x count %d parent %p\n",
-		slot, offset, node->tags[0][0], node->tags[1][0],
-		node->tags[2][0], node->path, node->count, node->parent);
-
-	for (i = 0; i < RADIX_TREE_MAP_SIZE; i++)
-		dump_node(node->slots[i], height - 1, i);
+		node, offset,
+		node->tags[0][0], node->tags[1][0], node->tags[2][0],
+		node->path, node->count, node->parent);
+
+	for (i = 0; i < RADIX_TREE_MAP_SIZE; i++) {
+		unsigned long first = index | (i << shift);
+		unsigned long last = first | ((1 << shift) - 1);
+		void *entry = node->slots[i];
+		if (!entry)
+			continue;
+		if (is_sibling_entry(node, entry)) {
+			pr_debug("radix sblng %p offset %d val %p indices %ld-%ld\n",
+					entry, i,
+					*(void **)indirect_to_ptr(entry),
+					first, last);
+		} else if (!radix_tree_is_indirect_ptr(entry)) {
+			pr_debug("radix entry %p offset %d indices %ld-%ld\n",
+					entry, offset, first, last);
+		} else {
+			dump_node(indirect_to_ptr(entry), i,
+					shift - RADIX_TREE_MAP_SHIFT, first);
+		}
+	}
 }
 
 /* For debug */
@@ -246,7 +255,8 @@ static void radix_tree_dump(struct radix_tree_root *root)
 			root->gfp_mask >> __GFP_BITS_SHIFT);
 	if (!radix_tree_is_indirect_ptr(root->rnode))
 		return;
-	dump_node(root->rnode, root->height, 0);
+	dump_node(indirect_to_ptr(root->rnode), 0,
+				(root->height - 1) * RADIX_TREE_MAP_SHIFT, 0);
 }
 #endif
 
diff --git a/tools/testing/radix-tree/test.h b/tools/testing/radix-tree/test.h
index 53cb595db44a..67217c93fe95 100644
--- a/tools/testing/radix-tree/test.h
+++ b/tools/testing/radix-tree/test.h
@@ -40,5 +40,6 @@ extern int nr_allocated;
 
 /* Normally private parts of lib/radix-tree.c */
 void *indirect_to_ptr(void *ptr);
+void radix_tree_dump(struct radix_tree_root *root);
 int root_tag_get(struct radix_tree_root *root, unsigned int tag);
 unsigned long radix_tree_maxindex(unsigned int height);
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 30/30] radix-tree: Add copyright statements
  2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
                   ` (28 preceding siblings ...)
  2016-04-06 21:21 ` [PATCH 29/30] radix-tree: Fix radix_tree_dump() for multi-order entries Matthew Wilcox
@ 2016-04-06 21:21 ` Matthew Wilcox
  29 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2016-04-06 21:21 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton
  Cc: Matthew Wilcox, Ross Zwisler, linux-mm, linux-fsdevel,
	Konstantin Khlebnikov, Kirill Shutemov, Jan Kara, Neil Brown

The multiorder support is a sufficiently large feature to be worth adding
copyrigt lines for.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 lib/radix-tree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 0402c4f1a344..edaf4771feb0 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -4,6 +4,8 @@
  * Copyright (C) 2005 SGI, Christoph Lameter
  * Copyright (C) 2006 Nick Piggin
  * Copyright (C) 2012 Konstantin Khlebnikov
+ * Copyright (C) 2016 Intel, Matthew Wilcox
+ * Copyright (C) 2016 Intel, Ross Zwisler
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
-- 
2.8.0.rc3

^ permalink raw reply related	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2016-04-06 21:31 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-06 21:21 [PATCH 00/30] Radix tree multiorder fixes Matthew Wilcox
2016-04-06 21:21 ` [PATCH 01/30] radix-tree: Introduce radix_tree_empty Matthew Wilcox
2016-04-06 21:21 ` [PATCH 02/30] radix tree test suite: Fix build Matthew Wilcox
2016-04-06 21:21 ` [PATCH 03/30] radix tree test suite: Add tests for radix_tree_locate_item() Matthew Wilcox
2016-04-06 21:21 ` [PATCH 04/30] radix tree test suite: Allow testing other fan-out values Matthew Wilcox
2016-04-06 21:21 ` [PATCH 05/30] radix tree test suite: keep regression test runs short Matthew Wilcox
2016-04-06 21:21 ` [PATCH 06/30] radix tree test suite: rebuild when headers change Matthew Wilcox
2016-04-06 21:21 ` [PATCH 07/30] radix-tree: remove unused looping macros Matthew Wilcox
2016-04-06 21:21 ` [PATCH 08/30] Introduce CONFIG_RADIX_TREE_MULTIORDER Matthew Wilcox
2016-04-06 21:21 ` [PATCH 09/30] radix-tree: Add missing sibling entry functionality Matthew Wilcox
2016-04-06 21:21 ` [PATCH 10/30] radix-tree: Fix sibling entry insertion Matthew Wilcox
2016-04-06 21:21 ` [PATCH 11/30] radix-tree: Fix deleting a multi-order entry through an alias Matthew Wilcox
2016-04-06 21:21 ` [PATCH 12/30] radix-tree: Remove restriction on multi-order entries Matthew Wilcox
2016-04-06 21:21 ` [PATCH 13/30] radix-tree: Introduce radix_tree_load_root() Matthew Wilcox
2016-04-06 21:21 ` [PATCH 14/30] radix-tree: Fix extending the tree for multi-order entries at offset 0 Matthew Wilcox
2016-04-06 21:21 ` [PATCH 15/30] radix-tree: Fix several shrinking bugs with multiorder entries Matthew Wilcox
2016-04-06 21:21 ` [PATCH 16/30] radix tree test suite: Start adding multiorder tests Matthew Wilcox
2016-04-06 21:21 ` [PATCH 17/30] radix-tree: Rewrite __radix_tree_lookup Matthew Wilcox
2016-04-06 21:21 ` [PATCH 18/30] radix-tree: Fix multiorder BUG_ON in radix_tree_insert Matthew Wilcox
2016-04-06 21:21 ` [PATCH 19/30] radix-tree: add support for multi-order iterating Matthew Wilcox
2016-04-06 21:21 ` [PATCH 20/30] radix tree test suite: multi-order iteration test Matthew Wilcox
2016-04-06 21:21 ` [PATCH 21/30] radix tree test suite: Add multiorder shrinking test Matthew Wilcox
2016-04-06 21:21 ` [PATCH 22/30] radix-tree: Rewrite radix_tree_tag_set Matthew Wilcox
2016-04-06 21:21 ` [PATCH 23/30] radix-tree: Rewrite radix_tree_tag_clear Matthew Wilcox
2016-04-06 21:21 ` [PATCH 24/30] radix-tree: Rewrite radix_tree_tag_get Matthew Wilcox
2016-04-06 21:21 ` [PATCH 25/30] radix-tree test suite: add multi-order tag test Matthew Wilcox
2016-04-06 21:21 ` [PATCH 26/30] radix-tree: Fix radix_tree_create for sibling entries Matthew Wilcox
2016-04-06 21:21 ` [PATCH 27/30] radix-tree: Rewrite radix_tree_locate_item Matthew Wilcox
2016-04-06 21:21 ` [PATCH 28/30] radix-tree: Fix two bugs in radix_tree_range_tag_if_tagged() Matthew Wilcox
2016-04-06 21:21 ` [PATCH 29/30] radix-tree: Fix radix_tree_dump() for multi-order entries Matthew Wilcox
2016-04-06 21:21 ` [PATCH 30/30] radix-tree: Add copyright statements Matthew Wilcox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).