libfdt: Add FDT_CREATE_FLAG_NO_NAME_DEDUP flag that trades size for speed

Searching for duplicate names scales O(n^2) with the number of names
added to a fdt, which can cause a noticable slowdown with larger device
trees and very slow CPU cores.

Add FDT_CREATE_FLAG_NO_NAME_DEDUP that allow the caller to trade fdt size
for speed in the creation process.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Message-Id: <20190509094122.834-4-npiggin@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

libfdt/fdt_sw.c

@@ -284,6 +284,23 @@ int fdt_end_node(void *fdt)
 	return 0;
 }
 
+static int fdt_add_string_(void *fdt, const char *s)
+{
+	char *strtab = (char *)fdt + fdt_totalsize(fdt);
+	int strtabsize = fdt_size_dt_strings(fdt);
+	int len = strlen(s) + 1;
+	int struct_top, offset;
+
+	offset = -strtabsize - len;
+	struct_top = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
+	if (fdt_totalsize(fdt) + offset < struct_top)
+		return 0; /* no more room :( */
+
+	memcpy(strtab + offset, s, len);
+	fdt_set_size_dt_strings(fdt, strtabsize + len);
+	return offset;
+}
+
 /* Must only be used to roll back in case of error */
 static void fdt_del_last_string_(void *fdt, const char *s)
 {
@@ -296,10 +313,8 @@ static void fdt_del_last_string_(void *fdt, const char *s)
 static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
 {
 	char *strtab = (char *)fdt + fdt_totalsize(fdt);
-	const char *p;
 	int strtabsize = fdt_size_dt_strings(fdt);
-	int len = strlen(s) + 1;
+	const char *p;
-	int struct_top, offset;
 
 	*allocated = 0;
 
@@ -307,17 +322,9 @@ static int fdt_find_add_string_(void *fdt, const char *s, int *allocated)
 	if (p)
 		return p - strtab;
 
-	/* Add it */
 	*allocated = 1;
 
-	offset = -strtabsize - len;
+	return fdt_add_string_(fdt, s);
-	struct_top = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
-	if (fdt_totalsize(fdt) + offset < struct_top)
-		return 0; /* no more room :( */
-
-	memcpy(strtab + offset, s, len);
-	fdt_set_size_dt_strings(fdt, strtabsize + len);
-	return offset;
 }
 
 int fdt_property_placeholder(void *fdt, const char *name, int len, void **valp)
@@ -328,7 +335,13 @@ int fdt_property_placeholder(void *fdt, const char *name, int len, void **valp)
 
 	FDT_SW_PROBE_STRUCT(fdt);
 
-	nameoff = fdt_find_add_string_(fdt, name, &allocated);
+	/* String de-duplication can be slow, _NO_NAME_DEDUP skips it */
+	if (sw_flags(fdt) & FDT_CREATE_FLAG_NO_NAME_DEDUP) {
+		allocated = 1;
+		nameoff = fdt_add_string_(fdt, name);
+	} else {
+		nameoff = fdt_find_add_string_(fdt, name, &allocated);
+	}
 	if (nameoff == 0)
 		return -FDT_ERR_NOSPACE;
 

libfdt/libfdt.h

@@ -1433,7 +1433,13 @@ int fdt_nop_node(void *fdt, int nodeoffset);
 /* Sequential write functions                                         */
 /**********************************************************************/
 
-#define FDT_CREATE_FLAGS_ALL	0
+/* fdt_create_with_flags flags */
+#define FDT_CREATE_FLAG_NO_NAME_DEDUP 0x1
+	/* FDT_CREATE_FLAG_NO_NAME_DEDUP: Do not try to de-duplicate property
+	 * names in the fdt. This can result in faster creation times, but
+	 * a larger fdt. */
+
+#define FDT_CREATE_FLAGS_ALL	(FDT_CREATE_FLAG_NO_NAME_DEDUP)
 
 /**
  * fdt_create_with_flags - begin creation of a new fdt

tests/run_tests.sh

@@ -354,19 +354,21 @@ libfdt_tests () {
     run_dtc_test -I dts -O dtb -o stringlist.test.dtb stringlist.dts
     run_test stringlist stringlist.test.dtb
 
-    # Sequential write tests
+    for flags in default no_name_dedup; do
-    run_test sw_tree1
+        # Sequential write tests
-    tree1_tests sw_tree1.test.dtb
+        run_test sw_tree1 fixed $flags
-    tree1_tests unfinished_tree1.test.dtb
+        tree1_tests sw_tree1.test.dtb
-    run_test dtbs_equal_ordered test_tree1.dtb sw_tree1.test.dtb
+        tree1_tests unfinished_tree1.test.dtb
-    run_test sw_states
+        run_test dtbs_equal_ordered test_tree1.dtb sw_tree1.test.dtb
+        run_test sw_states
 
-    # Resizing tests
+        # Resizing tests
-    for mode in resize realloc newalloc; do
+        for mode in resize realloc newalloc; do
-	run_test sw_tree1 $mode
+            run_test sw_tree1 $mode $flags
-	tree1_tests sw_tree1.test.dtb
+            tree1_tests sw_tree1.test.dtb
-	tree1_tests unfinished_tree1.test.dtb
+            tree1_tests unfinished_tree1.test.dtb
-	run_test dtbs_equal_ordered test_tree1.dtb sw_tree1.test.dtb
+            run_test dtbs_equal_ordered test_tree1.dtb sw_tree1.test.dtb
+        done
     done
 
     # fdt_move tests

tests/sw_tree1.c

@@ -114,14 +114,19 @@ int main(int argc, char *argv[])
 	void *place;
 	const char place_str[] = "this is a placeholder string\0string2";
 	int place_len = sizeof(place_str);
+	int create_flags;
 
 	test_init(argc, argv);
 
-	if (argc == 1) {
+	alloc_mode = FIXED;
-		alloc_mode = FIXED;
+	size = SPACE;
-		size = SPACE;
+	create_flags = 0;
-	} else if (argc == 2) {
+
-		if (streq(argv[1], "resize")) {
+	if (argc == 2 || argc == 3) {
+		if (streq(argv[1], "fixed")) {
+			alloc_mode = FIXED;
+			size = SPACE;
+		} else if (streq(argv[1], "resize")) {
 			alloc_mode = REALLOC;
 			size = 0;
 		} else if (streq(argv[1], "realloc")) {
@@ -140,12 +145,36 @@ int main(int argc, char *argv[])
 				CONFIG("Bad allocation mode \"%s\" specified",
 				       argv[1]);
 		}
-	} else {
+	}
-		CONFIG("sw_tree1 <dtb file> [<allocation mode>]");
+	if (argc == 3) {
+		char *str = argv[2], *saveptr, *tok;
+		bool default_flag = false;
+
+		while ((tok = strtok_r(str, ",", &saveptr)) != NULL) {
+			str = NULL;
+			if (streq(tok, "default")) {
+				default_flag = true;
+			} else if (streq(tok, "no_name_dedup")) {
+				create_flags |= FDT_CREATE_FLAG_NO_NAME_DEDUP;
+			} else if (streq(tok, "bad")) {
+				create_flags |= 0xffffffff;
+			} else {
+				CONFIG("Bad creation flags \"%s\" specified",
+				       argv[2]);
+			}
+		}
+
+		if (default_flag && create_flags != 0)
+			CONFIG("Bad creation flags \"%s\" specified",
+			       argv[2]);
+	}
+
+	if (argc > 3) {
+		CONFIG("sw_tree1 [<allocation mode>] [<create flags>]");
 	}
 
 	fdt = xmalloc(size);
-	CHECK(fdt_create(fdt, size));
+	CHECK(fdt_create_with_flags(fdt, size, create_flags));
 
 	created = true;