[Crash-utility] [PATCH for testing only] Make radix tree compatible with 4.20-rc1 xarray

Dominique Martinet asmadeus at codewreck.org
Mon Nov 5 04:55:07 UTC 2018


radix trees got replaced with xarrays in 4.20-rc1's f8d5d0cc145
("xarray: Add definition of struct xarray") and 01959dfe77 ("xarray:
Define struct xa_node")

Attempt to detect and support this.
---
Dave Anderson wrote on Tue, Oct 30, 2018:
> Thanks for the heads-up -- this is a critical change w/respect to
> the crash utility, affecting several other parts as well as the
> PID/task gathering mechanism.  I don't have access to 4.20-rc1
> kernels in-house, or from Fedora, at the moment, so I won't be able
> to do any hands-on work on it for a while.  So given the insights
> you've gained already, by all means please continue if you can!

4.20-rc1 was not released yet so this isn't surprising :)
It came out just this weekend, should probably hit rawhide soon.


Meanwhile I had a second look (thanks for reminding me about the
`--active` switch, I can play with `tree -t radix` more easily now); I
had missed a couple more things with my initial test.
Basically, we need to:
 - fall back radix_tree_root to xarray, radix_tree_node to xa_node,
radix_tree_root_rnode to xarray's xa_head, radix_tree_node_slots to
xa_node's slots, radix_tree_node_shift to xa_node's shift
 - does not seem to be a max_height anymore
 - RADIX_TREE_INTERNAL_NODE changed from 1 to 2 (linux code: that 2 is
hardcoded in include/linux/xarray.h xa_mk_node and xa_to_node...)
The XA_CHUNK_MASK that replaces RADIX_TREE_ENTRY_MASK is bigger but
I didn't need to change that.
To use the same code with both radix tree and xarray, I'm now clearing
the entry mask instead of the interna node bit.
 - the second thing I had missed was `MEMBER_SIZE("radix_tree_node",
"slots")` in do_radix_tree_iter as it wasn't with the other fields, need
to fallback to xa_node's slots here as well.
the check !(nlen = MEMBER_SIZE(...)) isn't correct as MEMBER_SIZE will
return -1 on failure, so comparing to 0 might be better.


So ultimately I can get crash to start with the attached patch, but this
is definitely more of monkeying around than a real fix.
One thing I'm not sure about in particular is if the radix tree/xarray
contains values instead of pointers some of the internal bits changed,
but we do not seem to handle that for radix trees right now so it might
not matter...?

Anyway, patch tested on 4.20-rc1, and fedora's 4.18.16-200.fc28.x86_64.
As said previously it probably warrants more testing on 4.20-rc1 and
possibly older centos/rhel kernels, I don't think any of the tree I
tried was multiple levels deep for example...
Please consider it a "FYI" patch more than anything else :)

 task.c  |  9 +++++++++
 tools.c | 23 +++++++++++------------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/task.c b/task.c
index 1eecfce..34327c2 100644
--- a/task.c
+++ b/task.c
@@ -485,6 +485,15 @@ task_init(void)
 			"radix_tree_node","height");
 		MEMBER_OFFSET_INIT(radix_tree_node_shift,
 			"radix_tree_node","shift");
+	} else {
+		STRUCT_SIZE_INIT(radix_tree_root, "xarray");
+		STRUCT_SIZE_INIT(radix_tree_node, "xa_node");
+		MEMBER_OFFSET_INIT(radix_tree_root_rnode,
+			"xarray","xa_head");
+		MEMBER_OFFSET_INIT(radix_tree_node_slots,
+			"xa_node","slots");
+		MEMBER_OFFSET_INIT(radix_tree_node_shift,
+			"xa_node","shift");
 	}
 
 	if (symbol_exists("pidhash") && symbol_exists("pid_hash") &&
diff --git a/tools.c b/tools.c
index 634aec6..33d3787 100644
--- a/tools.c
+++ b/tools.c
@@ -4415,7 +4415,6 @@ static ulong RADIX_TREE_MAP_SIZE = UNINITIALIZED;
 static ulong RADIX_TREE_MAP_MASK = UNINITIALIZED;
 
 #define RADIX_TREE_ENTRY_MASK		3UL
-#define RADIX_TREE_INTERNAL_NODE	1UL
 
 static void do_radix_tree_iter(ulong node, uint height, char *path,
 			       ulong index, struct radix_tree_ops *ops)
@@ -4436,8 +4435,8 @@ static void do_radix_tree_iter(ulong node, uint height, char *path,
 		if (!slot)
 			continue;
 
-		if (slot & RADIX_TREE_INTERNAL_NODE)
-			slot &= ~RADIX_TREE_INTERNAL_NODE;
+		if (slot & RADIX_TREE_ENTRY_MASK)
+			slot &= ~RADIX_TREE_ENTRY_MASK;
 
 		if (height == 1)
 			ops->entry(node, slot, path, index | off, ops->private);
@@ -4467,13 +4466,13 @@ int do_radix_tree_traverse(ulong ptr, int is_root, struct radix_tree_ops *ops)
 	      !ARRAY_LENGTH(height_to_maxindex)) &&
 	     (!VALID_MEMBER(radix_tree_root_rnode) ||
 	      !VALID_MEMBER(radix_tree_node_shift) ||
-	      !VALID_MEMBER(radix_tree_node_slots) ||
-	      !ARRAY_LENGTH(height_to_maxnodes))))
+	      !VALID_MEMBER(radix_tree_node_slots))))
 		error(FATAL, "radix trees do not exist or have changed "
 			"their format\n");
 
 	if (RADIX_TREE_MAP_SHIFT == UNINITIALIZED) {
-		if (!(nlen = MEMBER_SIZE("radix_tree_node", "slots")))
+		if ((nlen = MEMBER_SIZE("radix_tree_node", "slots")) <= 0 &&
+		    (nlen = MEMBER_SIZE("xa_node", "slots")) <= 0)
 			error(FATAL, "cannot determine length of "
 				     "radix_tree_node.slots[] array\n");
 		nlen /= sizeof(void *);
@@ -4483,7 +4482,7 @@ int do_radix_tree_traverse(ulong ptr, int is_root, struct radix_tree_ops *ops)
 
 		if (ARRAY_LENGTH(height_to_maxindex))
 			max_height = ARRAY_LENGTH(height_to_maxindex);
-		else
+		else if (ARRAY_LENGTH(height_to_maxnodes))
 			max_height = ARRAY_LENGTH(height_to_maxnodes);
 	}
 
@@ -4491,8 +4490,8 @@ int do_radix_tree_traverse(ulong ptr, int is_root, struct radix_tree_ops *ops)
 	if (!is_root) {
 		node_p = ptr;
 
-		if (node_p & RADIX_TREE_INTERNAL_NODE)
-			node_p &= ~RADIX_TREE_INTERNAL_NODE;
+		if (node_p & RADIX_TREE_ENTRY_MASK)
+			node_p &= ~RADIX_TREE_ENTRY_MASK;
 
 		if (VALID_MEMBER(radix_tree_node_height)) {
 			readmem(node_p + OFFSET(radix_tree_node_height), KVADDR,
@@ -4516,9 +4515,9 @@ int do_radix_tree_traverse(ulong ptr, int is_root, struct radix_tree_ops *ops)
 
 		readmem(ptr + OFFSET(radix_tree_root_rnode), KVADDR, &node_p,
 			sizeof(void *), "radix_tree_root rnode", FAULT_ON_ERROR);
-		is_internal = (node_p & RADIX_TREE_INTERNAL_NODE);
-		if (node_p & RADIX_TREE_INTERNAL_NODE)
-			node_p &= ~RADIX_TREE_INTERNAL_NODE;
+		is_internal = (node_p & RADIX_TREE_ENTRY_MASK);
+		if (node_p & RADIX_TREE_ENTRY_MASK)
+			node_p &= ~RADIX_TREE_ENTRY_MASK;
 
 		if (is_internal && VALID_MEMBER(radix_tree_node_shift)) {
 			readmem(node_p + OFFSET(radix_tree_node_shift), KVADDR, &shift,
-- 
2.19.1




More information about the Crash-utility mailing list