Date: Mon, 4 Apr 2011 19:52:39 GMT From: John Baldwin <jhb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 191046 for review Message-ID: <201104041952.p34JqdcP044148@skunkworks.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@191046?ac=10 Change 191046 by jhb@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)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201104041952.p34JqdcP044148>