PERFORCE change 191046 for review

John Baldwin jhb at FreeBSD.org
Mon Apr 4 19:52:40 UTC 2011


http://p4web.freebsd.org/@@191046?ac=10

Change 191046 by jhb at jhb_jhbbsd on 2011/04/04 19:52:05

	- Fix rman_manage_region() to honor rm_start and rm_end.  This
	  exposed several bugs where rm_end was set to ~0u instead of
	  ~0ul.  Also, many rmans don't set rm_start and rm_end at all,
	  so make rman_init() set them to the full range if they are both
	  zero.
	- Fix bugs in rman_release_region() and its regression tests (the
	  tests now work properly).
	- Add rman_fetch_{first,last}_free_region() routines and regression
	  tests (which pass).
	- Compile.

Affected files ...

.. //depot/projects/pci/sys/arm/arm/nexus.c#2 edit
.. //depot/projects/pci/sys/dev/fdt/fdtbus.c#2 edit
.. //depot/projects/pci/sys/ia64/ia64/nexus.c#2 edit
.. //depot/projects/pci/sys/kern/subr_bus.c#3 edit
.. //depot/projects/pci/sys/kern/subr_rman.c#6 edit
.. //depot/projects/pci/sys/mips/mips/mainbus.c#2 edit
.. //depot/projects/pci/sys/mips/mips/nexus.c#2 edit
.. //depot/projects/pci/sys/mips/rmi/xlr_pci.c#2 edit
.. //depot/projects/pci/sys/modules/rman/rman.c#9 edit
.. //depot/projects/pci/sys/sys/rman.h#4 edit
.. //depot/projects/pci/sys/x86/x86/nexus.c#3 edit

Differences ...

==== //depot/projects/pci/sys/arm/arm/nexus.c#2 (text+ko) ====

@@ -138,10 +138,10 @@
 {
 
 	mem_rman.rm_start = 0;
-	mem_rman.rm_end = ~0u;
+	mem_rman.rm_end = ~0ul;
 	mem_rman.rm_type = RMAN_ARRAY;
 	mem_rman.rm_descr = "I/O memory addresses";
-	if (rman_init(&mem_rman) || rman_manage_region(&mem_rman, 0, ~0u))
+	if (rman_init(&mem_rman) || rman_manage_region(&mem_rman, 0, ~0))
 		panic("nexus_probe mem_rman");
 
 	/*

==== //depot/projects/pci/sys/dev/fdt/fdtbus.c#2 (text+ko) ====

@@ -206,7 +206,7 @@
 	 * Mem-mapped I/O space rman.
 	 */
 	start = 0;
-	end = ~0u;
+	end = ~0ul;
 	sc->sc_mem.rm_start = start;
 	sc->sc_mem.rm_end = end;
 	sc->sc_mem.rm_type = RMAN_ARRAY;

==== //depot/projects/pci/sys/ia64/ia64/nexus.c#2 (text+ko) ====

@@ -174,7 +174,7 @@
 		panic("nexus_probe port_rman");
 
 	mem_rman.rm_start = 0;
-	mem_rman.rm_end = ~0u;
+	mem_rman.rm_end = ~0ul;
 	mem_rman.rm_type = RMAN_ARRAY;
 	mem_rman.rm_descr = "I/O memory addresses";
 	if (rman_init(&mem_rman)

==== //depot/projects/pci/sys/kern/subr_bus.c#3 (text+ko) ====

@@ -3652,7 +3652,7 @@
  * BUS_ADJUST_RESOURCE() method of the parent of @p dev.
  */
 int
-bus_generic_adjust_resource(device_t bus, device_t child, int type,
+bus_generic_adjust_resource(device_t dev, device_t child, int type,
     struct resource *r, u_long start, u_long end)
 {
 	/* Propagate up the bus hierarchy until someone handles it. */

==== //depot/projects/pci/sys/kern/subr_rman.c#6 (text+ko) ====

@@ -133,11 +133,14 @@
 	static int once = 0;
 
 	if (once == 0) {
+		/* XXX: Should move to SYSINIT. */
 		once = 1;
 		TAILQ_INIT(&rman_head);
 		mtx_init(&rman_mtx, "rman head", NULL, MTX_DEF);
 	}
 
+	if (rm->rm_start == 0 && rm->rm_end == 0)
+		rm->rm_end = ~0ul;
 	if (rm->rm_type == RMAN_UNINIT)
 		panic("rman_init");
 	if (rm->rm_type == RMAN_GAUGE)
@@ -231,7 +234,7 @@
 int
 rman_release_region(struct rman *rm, u_long start, u_long end)
 {
-	struct resource_i *r, *s;
+	struct resource_i *r, *s, *t;
 
 	DPRINTF(("rman_release_region: <%s> request: start %#lx, end %#lx\n",
 	    rm->rm_descr, start, end));
@@ -242,7 +245,7 @@
 	TAILQ_FOREACH(r, &rm->rm_list, r_link) {
 		if (r->r_end == ULONG_MAX)
 			break;
-		if (r->r_end + 1 >= start)
+		if (r->r_end + 1 > start)
 			break;
 	}
 
@@ -281,16 +284,17 @@
 	 * adjust 'r', possibly splitting it.
 	 */
 	if (r->r_start == start && r->r_end == end) {
-		TAILQ_REMOVE(r, r_link);
+		TAILQ_REMOVE(&rm->rm_list, r, r_link);
 		free(r, M_RMAN);
 	} else if (r->r_start == start) {
-		KASSERT(end > r->r_end, ("resource entry too small"));
+		KASSERT(end < r->r_end, ("resource entry too small"));
 		r->r_start = end + 1;
 	} else if (r->r_end == end) {
-		KASSERT(start < r->r_start, ("resource entry too small"));
+		KASSERT(start > r->r_start, ("resource entry too small"));
 		r->r_end = start - 1;
 	} else {
-		KASSERT(r->r_start < start && end < r->r_end, ("resource entry too small"));
+		KASSERT(r->r_start < start && end < r->r_end,
+		    ("resource entry too small"));
 		s = int_alloc_resource(M_NOWAIT);
 		if (s == NULL) {
 			mtx_unlock(rm->rm_mtx);
@@ -349,6 +353,42 @@
 	return 0;
 }
 
+int
+rman_first_free_region(struct rman *rm, u_long *start, u_long *end)
+{
+	struct resource_i *r;
+
+	mtx_lock(rm->rm_mtx);
+	TAILQ_FOREACH(r, &rm->rm_list, r_link) {
+		if (!(r->r_flags & RF_ALLOCATED)) {
+			*start = r->r_start;
+			*end = r->r_end;
+			mtx_unlock(rm->rm_mtx);
+			return (0);
+		}
+	}
+	mtx_unlock(rm->rm_mtx);
+	return (ENOENT);
+}
+
+int
+rman_last_free_region(struct rman *rm, u_long *start, u_long *end)
+{
+	struct resource_i *r;
+
+	mtx_lock(rm->rm_mtx);
+	TAILQ_FOREACH_REVERSE(r, &rm->rm_list, resource_head, r_link) {
+		if (!(r->r_flags & RF_ALLOCATED)) {
+			*start = r->r_start;
+			*end = r->r_end;
+			mtx_unlock(rm->rm_mtx);
+			return (0);
+		}
+	}
+	mtx_unlock(rm->rm_mtx);
+	return (ENOENT);
+}
+
 /* Shrink or extend one or both ends of an allocated resource. */
 int
 rman_adjust_resource(struct resource *rr, u_long start, u_long end)

==== //depot/projects/pci/sys/mips/mips/mainbus.c#2 (text+ko) ====

@@ -146,7 +146,7 @@
 		panic("mainbus_probe port_rman");
 
 	mem_rman.rm_start = 0;
-	mem_rman.rm_end = ~0u;
+	mem_rman.rm_end = ~0ul;
 	mem_rman.rm_type = RMAN_ARRAY;
 	mem_rman.rm_descr = "I/O memory addresses";
 	if (rman_init(&mem_rman) || rman_manage_region(&mem_rman, 0, ~0))

==== //depot/projects/pci/sys/mips/mips/nexus.c#2 (text+ko) ====

@@ -151,7 +151,7 @@
 	}
 
 	mem_rman.rm_start = 0;
-	mem_rman.rm_end = ~0u;
+	mem_rman.rm_end = ~0ul;
 	mem_rman.rm_type = RMAN_ARRAY;
 	mem_rman.rm_descr = "Memory addresses";
 	if (rman_init(&mem_rman) != 0 ||

==== //depot/projects/pci/sys/mips/rmi/xlr_pci.c#2 (text+ko) ====

@@ -126,7 +126,7 @@
 		panic("pci_init_resources irq_rman");
 
 	port_rman.rm_start = 0;
-	port_rman.rm_end = ~0u;
+	port_rman.rm_end = ~0ul;
 	port_rman.rm_type = RMAN_ARRAY;
 	port_rman.rm_descr = "I/O ports";
 	if (rman_init(&port_rman)
@@ -134,7 +134,7 @@
 		panic("pci_init_resources port_rman");
 
 	mem_rman.rm_start = 0;
-	mem_rman.rm_end = ~0u;
+	mem_rman.rm_end = ~0ul;
 	mem_rman.rm_type = RMAN_ARRAY;
 	mem_rman.rm_descr = "I/O memory";
 	if (rman_init(&mem_rman)

==== //depot/projects/pci/sys/modules/rman/rman.c#9 (text+ko) ====

@@ -293,6 +293,7 @@
 	rman_release_resource(r);
 	r = NULL;
 	assert_rman_ok();
+	printf("%s: finished successfully\n", __func__);
 }
 
 struct region {
@@ -355,7 +356,7 @@
 			/* Next region. */
 			count--;
 			r++;
-			start = r->r_start;
+			start = r->start;
 		} else
 			start = i->r_end + 1;
 		i = TAILQ_NEXT(i, r_link);
@@ -373,12 +374,12 @@
 #define RELEASE_SHOULD_FAIL(start, end, err) do {			\
 	error = rman_release_region(&test, (start), (end));		\
 	if (error == (err))						\
-		printf("Correctly failed to release (%x, %x)\n",	\
-		    (start), (end));					\
+		printf("Correctly failed to release (%lx, %lx)\n",	\
+		    (u_long)(start), (u_long)(end));			\
 	else {								\
 		if (error)						\
-			printf("Failed to release (%x, %x) with %d\n",	\
-			    (start), (end), error);			\
+			printf("Failed to release (%lx, %lx) with %d\n",\
+			    (u_long)(start), (u_long)(end), error);	\
 		else							\
 			printf("Incorrectly released (%lx, %lx)\n",	\
 			    rman_get_start(r), rman_get_end(r));	\
@@ -389,22 +390,22 @@
 #define RELEASE_SHOULD_WORK(start, end) do {				\
 	error = rman_release_region(&test, (start), (end));		\
 	if (error) {							\
-		printf("Failed to release (%x, %x) with %d\n",		\
-		    (start), (end), error);				\
+		printf("Failed to release (%lx, %lx) with %d\n",	\
+		    (u_long)(start), (u_long)(end), error);		\
 		return;							\
 	}								\
-	printf("Released (%x, %x)\n", (start), (end));			\
+	printf("Released (%lx, %lx)\n", (u_long)(start), (u_long)(end)); \
 	assert_rman_hole((start), (end));				\
 } while (0)
 
 #define MANAGE_SHOULD_WORK(start, end) do {				\
 	error = rman_manage_region(&test, (start), (end));		\
 	if (error) {							\
-		printf("Failed to manage (%x, %x) with %d\n",		\
-		    (start), (end), error);				\
+		printf("Failed to manage (%lx, %lx) with %d\n",		\
+		    (u_long)(start), (u_long)(end), error);		\
 		return;							\
 	}								\
-	printf("Managed (%x, %x)\n", (start), (end));			\
+	printf("Managed (%lx, %lx)\n", (u_long)(start), (u_long)(end));	\
 	assert_rman_managed((start), (end));				\
 } while (0)
 
@@ -500,8 +501,172 @@
 	assert_rman_ok();
 
 	rman_release_resource(r);
+	r = NULL;
 	assert_rman_regions(regions, 1);
 	assert_rman_ok();
+	printf("%s: finished successfully\n", __func__);
+}
+
+static void
+fetch_free_regression_tests(void)
+{
+	int error;
+
+#define FIRST_SHOULD_WORK(start, end) do {				\
+	u_long _start, _end;						\
+									\
+	error = rman_first_free_region(&test, &_start, &_end);		\
+	if (error) {							\
+		printf("Failed to fetch free region (%x, %x) with %d\n",\
+		    (start), (end), error);				\
+		return;							\
+	}								\
+	printf("Fetched free region (%x, %x)\n", (start), (end));	\
+} while (0)
+
+#define	FIRST_SHOULD_FAIL(err) do {					\
+	u_long _start, _end;						\
+									\
+	error = rman_first_free_region(&test, &_start, &_end);		\
+	if (error == (err))						\
+		printf("Correctly failed to fetch free region\n");	\
+	else {								\
+		if (error)						\
+			printf("Failed to fetch free region with %d\n",	\
+			    error);					\
+		else							\
+			printf("Incorrectly fetched free region (%lx, %lx)\n",\
+			    _start, _end);				\
+		return;							\
+	}								\
+} while (0)
+
+#define LAST_SHOULD_WORK(start, end) do {				\
+	u_long _start, _end;						\
+									\
+	error = rman_last_free_region(&test, &_start, &_end);		\
+	if (error) {							\
+		printf("Failed to fetch free region (%x, %x) with %d\n",\
+		    (start), (end), error);				\
+		return;							\
+	}								\
+	printf("Fetched free region (%x, %x)\n", (start), (end));	\
+} while (0)
+
+#define	LAST_SHOULD_FAIL(err) do {					\
+	u_long _start, _end;						\
+									\
+	error = rman_last_free_region(&test, &_start, &_end);		\
+	if (error == (err))						\
+		printf("Correctly failed to fetch free region\n");	\
+	else {								\
+		if (error)						\
+			printf("Failed to fetch free region with %d\n",	\
+			    error);					\
+		else							\
+			printf("Incorrectly fetched free region (%lx, %lx)\n",\
+			    _start, _end);				\
+		return;							\
+	}								\
+} while (0)
+
+	/* Clear any released resources. */
+	if (r != NULL) {
+		rman_release_resource(r);
+		r = NULL;
+	}
+	if (s != NULL) {
+		rman_release_resource(s);
+		s = NULL;
+	}
+	assert_rman_ok();
+
+	/* Should have one free region covering the full range. */
+	FIRST_SHOULD_WORK(REGION_START, REGION_END);
+	LAST_SHOULD_WORK(REGION_START, REGION_END);
+
+	/* Allocate 'r' at the beginning. */
+	r = rman_reserve_resource(&test, REGION_START, REGION_START + 0xf,
+	    0x10, 0, NULL);
+	if (r == NULL) {
+		printf("Failed to allocate resource\n");
+		return;
+	}
+	printf("Allocated (%lx, %lx)\n", rman_get_start(r), rman_get_end(r));
+
+	/* Should have one free region at the end. */
+	FIRST_SHOULD_WORK(REGION_START + 0x10, REGION_END);
+	LAST_SHOULD_WORK(REGION_START + 0x10, REGION_END);
+
+	/* Allocate 'r' at the end. */
+	rman_release_resource(r);
+	r = rman_reserve_resource(&test, REGION_END - 0x10, REGION_END,
+	    0x11, 0, NULL);
+	if (r == NULL) {
+		printf("Failed to allocate resource\n");
+		return;
+	}
+	printf("Allocated (%lx, %lx)\n", rman_get_start(r), rman_get_end(r));
+
+	/* Should have one free region at the beginning. */
+	FIRST_SHOULD_WORK(REGION_START, REGION_END - 0x11);
+	FIRST_SHOULD_WORK(REGION_START, REGION_END - 0x11);
+
+	/* Move 'r' to the middle. */
+	rman_release_resource(r);
+	r = rman_reserve_resource(&test, REGION_START + 0x10, REGION_END - 0x10,
+	    REGION_END - 0x10 - (REGION_START + 0x10) + 1, 0, NULL);
+	if (r == NULL) {
+		printf("Failed to allocate resource\n");
+		return;
+	}
+	printf("Allocated (%lx, %lx)\n", rman_get_start(r), rman_get_end(r));
+
+	/* Should have two free regions. */
+	FIRST_SHOULD_WORK(REGION_START, REGION_START + 0xf);
+	LAST_SHOULD_WORK(REGION_END - 0x0f, REGION_END);
+
+	/* Allocate the whole darn thing. */
+	ADJUST_SHOULD_WORK(REGION_START, REGION_END);
+
+	/* Should have no free region. */
+	FIRST_SHOULD_FAIL(ENOENT);
+	LAST_SHOULD_FAIL(ENOENT);
+
+	ADJUST_SHOULD_WORK(REGION_START, REGION_START + 0x1f);
+	s = rman_reserve_resource(&test, REGION_START + 0x20, REGION_END,
+	    REGION_END - (REGION_START + 0x20) + 1, 0, NULL);
+	if (s == NULL) {
+		printf("Failed to allocate resource\n");
+		return;
+	}
+	printf("Allocated (%lx, %lx)\n", rman_get_start(s), rman_get_end(s));
+
+	/* Should have no free region. */
+	FIRST_SHOULD_FAIL(ENOENT);
+	LAST_SHOULD_FAIL(ENOENT);
+
+	rman_release_resource(r);
+	r = NULL;
+	rman_release_resource(s);
+	s = NULL;
+
+	/* Should have one free region covering the full range. */
+	FIRST_SHOULD_WORK(REGION_START, REGION_END);
+	LAST_SHOULD_WORK(REGION_START, REGION_END);
+
+	/* Empty the rman. */
+	RELEASE_SHOULD_WORK(REGION_START, REGION_END);
+	assert_rman_regions(NULL, 0);
+
+	/* Should have no free region. */
+	FIRST_SHOULD_FAIL(ENOENT);
+	LAST_SHOULD_FAIL(ENOENT);
+
+	/* Cleanup. */
+	MANAGE_SHOULD_WORK(REGION_START, REGION_END);
+	assert_rman_ok();
+	printf("%s: finished successfully\n", __func__);
 }
 
 static int
@@ -512,13 +677,16 @@
 	error = sysctl_handle_int(oidp, &i, sizeof(i), req);
 	if (error || req->newptr == NULL || i == 0)
 		return (error);
-	switch (oip->arg2) {
+	switch (oidp->oid_arg2) {
 	case 0:
 		adjust_regression_tests();
 		break;
 	case 1:
 		region_regression_tests();
 		break;
+	case 2:
+		fetch_free_regression_tests();
+		break;
 	}
 	return (error);
 }
@@ -526,18 +694,8 @@
     sysctl_rman_test, "I", "run regression tests for rman_adjust_resource()");
 SYSCTL_PROC(_debug_rman, OID_AUTO, test_region, CTLTYPE_INT | CTLFLAG_RW, 0, 1,
     sysctl_rman_test, "I", "run regression tests for rman_release_region()");
-
-static int
-sysctl_rman_test_region(SYSCTL_HANDLER_ARGS)
-{
-	int error, i = 0;
-
-	error = sysctl_handle_int(oidp, &i, sizeof(i), req);
-	if (error || req->newptr == NULL || i == 0)
-		return (error);
-	region_regression_tests();
-	return (error);
-}
+SYSCTL_PROC(_debug_rman, OID_AUTO, test_fetch, CTLTYPE_INT | CTLFLAG_RW, 0, 2,
+    sysctl_rman_test, "I", "run regression tests for rman_fetch_*_free_region()");
 
 static int
 load(void)

==== //depot/projects/pci/sys/sys/rman.h#4 (text+ko) ====

@@ -118,6 +118,7 @@
 int	rman_activate_resource(struct resource *r);
 int	rman_adjust_resource(struct resource *r, u_long start, u_long end);
 int	rman_await_resource(struct resource *r, int pri, int timo);
+int	rman_first_free_region(struct rman *rm, u_long *start, u_long *end);
 bus_space_handle_t rman_get_bushandle(struct resource *);
 bus_space_tag_t rman_get_bustag(struct resource *);
 u_long	rman_get_end(struct resource *);
@@ -131,6 +132,7 @@
 int	rman_fini(struct rman *rm);
 int	rman_init(struct rman *rm);
 int	rman_init_from_resource(struct rman *rm, struct resource *r);
+int	rman_last_free_region(struct rman *rm, u_long *start, u_long *end);
 uint32_t rman_make_alignment_flags(uint32_t size);
 int	rman_manage_region(struct rman *rm, u_long start, u_long end);
 int	rman_is_region_manager(struct resource *r, struct rman *rm);

==== //depot/projects/pci/sys/x86/x86/nexus.c#3 (text+ko) ====

@@ -259,7 +259,7 @@
 		panic("nexus_init_resources port_rman");
 
 	mem_rman.rm_start = 0;
-	mem_rman.rm_end = ~0u;
+	mem_rman.rm_end = ~0ul;
 	mem_rman.rm_type = RMAN_ARRAY;
 	mem_rman.rm_descr = "I/O memory addresses";
 	if (rman_init(&mem_rman)


More information about the p4-projects mailing list