diff options
| -rw-r--r-- | libhurd-mm/ChangeLog | 5 | ||||
| -rw-r--r-- | libhurd-mm/mmap.c | 93 | ||||
| -rw-r--r-- | libhurd-mm/pager.h | 6 |
3 files changed, 84 insertions, 20 deletions
diff --git a/libhurd-mm/ChangeLog b/libhurd-mm/ChangeLog index b967f5b..8b32851 100644 --- a/libhurd-mm/ChangeLog +++ b/libhurd-mm/ChangeLog @@ -1,5 +1,10 @@ 2007-12-14 Neal H. Walfield <neal@gnu.org> + * mmap.c (munmap): Rewrite to avoid race. + * pager.h (struct pager): Add field, next. + +2007-12-14 Neal H. Walfield <neal@gnu.org> + * as.c (as_walk): Cache the root capability. 2007-12-14 Neal H. Walfield <neal@gnu.org> diff --git a/libhurd-mm/mmap.c b/libhurd-mm/mmap.c index 3cf7fae..99f0705 100644 --- a/libhurd-mm/mmap.c +++ b/libhurd-mm/mmap.c @@ -98,42 +98,97 @@ munmap (void *addr, size_t length) region.start = PTR_TO_ADDR (addr); region.count = length; - /* We need to keep calling hurd_btree_pager_find rather than - iterating as the destroy function may call munmap. */ - for (;;) + debug (5, "(%p, %x (%p))", addr, length, addr + length - 1); + + /* There is a race. We can't hold PAGERS_LOCK when we call + PAGER->DESTROY. In particular, the destroy function may also + want to unmap some memory. We can't grab one, drop the lock, + destroy and repeat until all the pagers in the region are gone: + another call to mmap could reuse a region we unmaped. Then + we'd unmap that new region. + + To make unmap atomic, we grab the lock, disconnect all pagers in + the region, adding each to a list, and then destroy each pager on + that list. */ + struct pager *list = NULL; + + ss_mutex_lock (&pagers_lock); + + struct pager *pager = hurd_btree_pager_find (&pagers, ®ion); + if (! pager) { - ss_mutex_lock (&pagers_lock); + ss_mutex_unlock (&pagers_lock); + return 0; + } - struct pager *pager = hurd_btree_pager_find (&pagers, ®ion); - if (! pager) - { - ss_mutex_unlock (&pagers_lock); - break; - } + struct pager *prev = hurd_btree_pager_prev (pager); - ss_mutex_lock (&pager->lock); + while (pager) + { + struct pager *next = hurd_btree_pager_next (pager); + + l4_uint64_t pager_start = addr_prefix (pager->region.start); + l4_uint64_t pager_end = pager_start + + (pager->region.count + << (ADDR_BITS - addr_depth (pager->region.start))) - 1; + + if (pager_start > end) + break; + + if (pager_start < start || pager_end > end) + panic ("Attempt to partially unmap pager unsupported"); + + pager_deinstall (pager); + pager->next = list; + list = pager; + + pager = next; + } + + pager = prev; + while (pager) + { + struct pager *prev = hurd_btree_pager_next (pager); - struct pager_region region = pager->region; l4_uint64_t pager_start = addr_prefix (pager->region.start); l4_uint64_t pager_end = pager_start + (pager->region.count << (ADDR_BITS - addr_depth (pager->region.start))) - 1; - debug (0, "Pager %llx-%llx between %x-%x", - pager_start, pager_end, start, end); + if (pager_end < start) + break; if (pager_start < start) - panic ("Attempt to partially unmap pager"); - if (end < pager_end) - panic ("Attempt to partially unmap pager"); + panic ("Attempt to partially unmap pager unsupported"); pager_deinstall (pager); - - ss_mutex_unlock (&pagers_lock); + pager->next = list; + list = pager; + + pager = prev; + } + + ss_mutex_unlock (&pagers_lock); + + pager = list; + while (pager) + { + struct pager *next = pager->next; + + struct pager_region region = pager->region; + + l4_uint64_t pager_start = addr_prefix (pager->region.start); + l4_uint64_t pager_end = pager_start + + (pager->region.count + << (ADDR_BITS - addr_depth (pager->region.start))) - 1; + debug (5, "Detroying pager covering %llx-%llx", + pager_start, pager_end); pager->destroy (pager); as_free (region.start, region.count); + + pager = next; } return 0; diff --git a/libhurd-mm/pager.h b/libhurd-mm/pager.h index 1b83ca1..2161ccd 100644 --- a/libhurd-mm/pager.h +++ b/libhurd-mm/pager.h @@ -63,7 +63,11 @@ enum struct pager { - hurd_btree_node_t node; + union + { + hurd_btree_node_t node; + struct pager *next; + }; /* The virtual addresses this pager covers. If this changes (e.g., if it grows or shrinks), then must be changed using the |