diff --git a/headers/private/kernel/thread_types.h b/headers/private/kernel/thread_types.h index 5aac26716b..9b84006308 100644 --- a/headers/private/kernel/thread_types.h +++ b/headers/private/kernel/thread_types.h @@ -152,6 +152,8 @@ struct thread { int32 flags; } sem; + struct PrivateConditionVariable *condition_variable; + struct { sem_id write_sem; sem_id read_sem; diff --git a/headers/private/kernel/vm_types.h b/headers/private/kernel/vm_types.h index 3b5b9440ac..7268361807 100644 --- a/headers/private/kernel/vm_types.h +++ b/headers/private/kernel/vm_types.h @@ -133,6 +133,16 @@ enum { CACHE_TYPE_NULL }; +#ifdef __cplusplus + +#include + +struct vm_dummy_page : vm_page { + ConditionVariable busy_condition; +}; + +#endif // __cplusplus + // vm_cache typedef struct vm_cache { mutex lock; diff --git a/src/system/kernel/cache/file_cache.cpp b/src/system/kernel/cache/file_cache.cpp index 51e3a0118f..d10f62a8a0 100644 --- a/src/system/kernel/cache/file_cache.cpp +++ b/src/system/kernel/cache/file_cache.cpp @@ -9,13 +9,14 @@ #include #include -#include +#include #include +#include +#include #include #include #include #include -#include #include #include @@ -30,7 +31,7 @@ #endif // maximum number of iovecs per request -#define MAX_IO_VECS 64 // 256 kB +#define MAX_IO_VECS 32 // 128 kB #define MAX_FILE_IO_VECS 32 #define MAX_TEMP_IO_VECS 8 @@ -522,10 +523,13 @@ read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t numBytes, vm_cache *cache = ref->cache; + // TODO: We're using way too much stack! Rather allocate a sufficiently + // large chunk on the heap. iovec vecs[MAX_IO_VECS]; int32 vecCount = 0; vm_page *pages[MAX_IO_VECS]; + ConditionVariable busyConditions[MAX_IO_VECS]; int32 pageIndex = 0; // allocate pages for the cache and mark them busy @@ -534,7 +538,7 @@ read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t numBytes, if (page == NULL) panic("no more pages!"); - page->state = PAGE_STATE_BUSY; + busyConditions[pageIndex - 1].Publish(page, "page"); vm_cache_insert_page(cache, page, offset + pos); @@ -569,6 +573,7 @@ read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t numBytes, mutex_lock(&cache->lock); for (int32 i = 0; i < pageIndex; i++) { + busyConditions[i].Unpublish(); vm_cache_remove_page(cache, pages[i]); vm_page_set_state(pages[i], PAGE_STATE_FREE); } @@ -599,8 +604,10 @@ read_chunk_into_cache(file_cache_ref *ref, off_t offset, size_t numBytes, mutex_lock(&cache->lock); // make the pages accessible in the cache - for (int32 i = pageIndex; i-- > 0;) + for (int32 i = pageIndex; i-- > 0;) { pages[i]->state = PAGE_STATE_ACTIVE; + busyConditions[i].Unpublish(); + } return B_OK; } @@ -664,11 +671,14 @@ static inline status_t write_chunk_to_cache(file_cache_ref *ref, off_t offset, size_t numBytes, int32 pageOffset, addr_t buffer, size_t bufferSize) { + // TODO: We're using way too much stack! Rather allocate a sufficiently + // large chunk on the heap. iovec vecs[MAX_IO_VECS]; int32 vecCount = 0; vm_page *pages[MAX_IO_VECS]; int32 pageIndex = 0; status_t status = B_OK; + ConditionVariable busyConditions[MAX_IO_VECS]; // ToDo: this should be settable somewhere bool writeThrough = false; @@ -679,7 +689,7 @@ write_chunk_to_cache(file_cache_ref *ref, off_t offset, size_t numBytes, // big - shouldn't we better steal the pages directly in that case? // (a working set like approach for the file cache) vm_page *page = pages[pageIndex++] = vm_page_allocate_page(PAGE_STATE_FREE); - page->state = PAGE_STATE_BUSY; + busyConditions[pageIndex - 1].Publish(page, "page"); vm_cache_insert_page(ref->cache, page, offset + pos); @@ -769,6 +779,8 @@ write_chunk_to_cache(file_cache_ref *ref, off_t offset, size_t numBytes, // make the pages accessible in the cache for (int32 i = pageIndex; i-- > 0;) { + busyConditions[i].Unpublish(); + if (writeThrough) pages[i]->state = PAGE_STATE_ACTIVE; else @@ -914,9 +926,10 @@ cache_io(void *_cacheRef, off_t offset, addr_t buffer, size_t *_size, bool doWri return B_NO_MEMORY; } } else { + ConditionVariableEntry entry; + entry.Add(page); mutex_unlock(&cache->lock); - // ToDo: don't wait forever! - snooze(20000); + entry.Wait(); mutex_lock(&cache->lock); goto restart; } @@ -974,9 +987,10 @@ cache_io(void *_cacheRef, off_t offset, addr_t buffer, size_t *_size, bool doWri // we let the other party add our page currentPage->queue_next = page; } else { + ConditionVariableEntry entry; + entry.Add(page); mutex_unlock(&cache->lock); - // ToDo: don't wait forever! - snooze(20000); + entry.Wait(); mutex_lock(&cache->lock); goto restart_dummy_lookup; } @@ -1113,9 +1127,11 @@ cache_prefetch_vnode(void *vnode, off_t offset, size_t size) vm_page *page = vm_cache_lookup_page(cache, offset); if (page != NULL) { if (page->state == PAGE_STATE_BUSY) { - // if busy retry again a little later + // if busy retry again later + ConditionVariableEntry entry; + entry.Add(page); mutex_unlock(&cache->lock); - snooze(20000); + entry.Wait(); mutex_lock(&cache->lock); goto restart; diff --git a/src/system/kernel/main.c b/src/system/kernel/main.c index 5a6e2f8570..402584a32e 100644 --- a/src/system/kernel/main.c +++ b/src/system/kernel/main.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -131,6 +132,7 @@ _start(kernel_args *bootKernelArgs, int currentCPU) TRACE("init semaphores\n"); sem_init(&sKernelArgs); + condition_variable_init(); // now we can create and use semaphores TRACE("init VM semaphores\n"); diff --git a/src/system/kernel/thread.c b/src/system/kernel/thread.c index b5959efece..6244c84243 100644 --- a/src/system/kernel/thread.c +++ b/src/system/kernel/thread.c @@ -210,6 +210,7 @@ create_thread_struct(struct thread *inthread, const char *name, thread->team = NULL; thread->cpu = cpu; thread->sem.blocking = -1; + thread->condition_variable = NULL; thread->fault_handler = 0; thread->page_faults_allowed = 1; thread->kernel_stack_area = -1; @@ -723,6 +724,7 @@ _dump_thread_info(struct thread *thread) kprintf(" sem.count: 0x%lx\n", thread->sem.count); kprintf(" sem.acquire_status: 0x%lx\n", thread->sem.acquire_status); kprintf(" sem.flags: 0x%lx\n", thread->sem.flags); + kprintf("condition variable: %p\n", thread->condition_variable); kprintf("fault_handler: %p\n", (void *)thread->fault_handler); kprintf("args: %p %p\n", thread->args1, thread->args2); kprintf("entry: %p\n", (void *)thread->entry); @@ -826,7 +828,8 @@ dump_thread_list(int argc, char **argv) kprintf("ignoring invalid team argument.\n"); } - kprintf("thread id state sem cpu pri stack team name\n"); + kprintf("thread id state sem/cv cpu pri stack team " + "name\n"); hash_open(sThreadHash, &i); while ((thread = hash_next(sThreadHash, &i)) != NULL) { @@ -840,11 +843,14 @@ dump_thread_list(int argc, char **argv) kprintf("%p %6lx %-9s", thread, thread->id, state_to_text(thread, thread->state)); - // does it block on a semaphore? - if (thread->state == B_THREAD_WAITING) - kprintf("%6lx ", thread->sem.blocking); - else - kprintf(" - "); + // does it block on a semaphore or a condition variable? + if (thread->state == B_THREAD_WAITING) { + if (thread->condition_variable) + kprintf("%p ", thread->condition_variable); + else + kprintf("%10lx ", thread->sem.blocking); + } else + kprintf(" - "); // on which CPU does it run? if (thread->cpu) diff --git a/src/system/kernel/vm/Jamfile b/src/system/kernel/vm/Jamfile index 5f757ff0e4..ad2ec3dfdd 100644 --- a/src/system/kernel/vm/Jamfile +++ b/src/system/kernel/vm/Jamfile @@ -6,7 +6,7 @@ KernelMergeObject kernel_vm.o : vm_cache.cpp vm_daemons.c vm_low_memory.cpp - vm_page.c + vm_page.cpp vm_store_anonymous_noswap.c vm_store_device.c vm_store_null.c diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index 1106e66ac4..e3c8a8ca4d 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -3496,11 +3497,13 @@ retry: away by grabbing a reference to it. */ static inline void -fault_insert_dummy_page(vm_cache *cache, vm_page &dummyPage, off_t cacheOffset) +fault_insert_dummy_page(vm_cache *cache, vm_dummy_page &dummyPage, + off_t cacheOffset) { dummyPage.state = PAGE_STATE_BUSY; vm_cache_acquire_ref(cache); vm_cache_insert_page(cache, &dummyPage, cacheOffset); + dummyPage.busy_condition.Publish(&dummyPage, "page"); } @@ -3509,7 +3512,7 @@ fault_insert_dummy_page(vm_cache *cache, vm_page &dummyPage, off_t cacheOffset) the cache. */ static inline void -fault_remove_dummy_page(vm_page &dummyPage, bool isLocked) +fault_remove_dummy_page(vm_dummy_page &dummyPage, bool isLocked) { vm_cache *cache = dummyPage.cache; if (!isLocked) @@ -3518,6 +3521,7 @@ fault_remove_dummy_page(vm_page &dummyPage, bool isLocked) if (dummyPage.state == PAGE_STATE_BUSY) { vm_cache_remove_page(cache, &dummyPage); dummyPage.state = PAGE_STATE_INACTIVE; + dummyPage.busy_condition.Unpublish(); } if (!isLocked) @@ -3537,8 +3541,8 @@ fault_remove_dummy_page(vm_page &dummyPage, bool isLocked) */ static inline status_t fault_find_page(vm_translation_map *map, vm_cache *topCache, - off_t cacheOffset, bool isWrite, vm_page &dummyPage, vm_cache **_pageCache, - vm_page** _page, bool* _restart) + off_t cacheOffset, bool isWrite, vm_dummy_page &dummyPage, + vm_cache **_pageCache, vm_page** _page, bool* _restart) { *_restart = false; vm_cache *cache = topCache; @@ -3560,24 +3564,36 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, for (;;) { page = vm_cache_lookup_page(cache, cacheOffset); if (page != NULL && page->state != PAGE_STATE_BUSY) { + // Note: We set the page state to busy, but we don't need a + // condition variable here, since we keep the cache locked + // till we mark the page unbusy again (in fault_get_page() + // the source page, or in vm_soft_fault() when mapping the + // page), so no one will ever know the page was busy in the + // first place. vm_page_set_state(page, PAGE_STATE_BUSY); break; } if (page == NULL || page == &dummyPage) break; - // page must be busy - // ToDo: don't wait forever! - mutex_unlock(&cache->lock); - thread_yield(); - mutex_lock(&cache->lock); + // page must be busy -- wait for it to become unbusy + { + ConditionVariableEntry entry; + entry.Add(page); + mutex_unlock(&cache->lock); + entry.Wait(); + mutex_lock(&cache->lock); + } if (cache->busy) { // The cache became busy, which means, it is about to be // removed by vm_cache_remove_consumer(). We start again with // the top cache. + ConditionVariableEntry entry; + entry.Add(cache); mutex_unlock(&cache->lock); vm_cache_release_ref(cache); + entry.Wait(); *_restart = true; return B_OK; } @@ -3595,9 +3611,11 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, // insert a fresh page and mark it busy -- we're going to read it in page = vm_page_allocate_page(PAGE_STATE_FREE); - page->state = PAGE_STATE_BUSY; vm_cache_insert_page(cache, page, cacheOffset); + ConditionVariable busyCondition; + busyCondition.Publish(page, "page"); + mutex_unlock(&cache->lock); // get a virtual address for the page @@ -3620,6 +3638,7 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, dprintf("reading page from store %p (cache %p) returned: %s!\n", store, cache, strerror(status)); + busyCondition.Unpublish(); vm_cache_remove_page(cache, page); vm_page_set_state(page, PAGE_STATE_FREE); @@ -3628,6 +3647,7 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, // mark the page unbusy again page->state = PAGE_STATE_ACTIVE; + busyCondition.Unpublish(); break; } @@ -3651,8 +3671,11 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, // The cache became busy, which means, it is about to be // removed by vm_cache_remove_consumer(). We start again with // the top cache. + ConditionVariableEntry entry; + entry.Add(cache); mutex_unlock(&cache->lock); vm_cache_release_ref(cache); + entry.Wait(); *_restart = true; return B_OK; } @@ -3682,8 +3705,11 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, // The cache became busy, which means, it is about to be // removed by vm_cache_remove_consumer(). We start again with // the top cache. + ConditionVariableEntry entry; + entry.Add(cache); mutex_unlock(&cache->lock); vm_cache_release_ref(cache); + entry.Wait(); *_restart = true; } else { vm_page* newPage = vm_cache_lookup_page(cache, cacheOffset); @@ -3718,8 +3744,8 @@ fault_find_page(vm_translation_map *map, vm_cache *topCache, to it, and has also locked it on exit. */ static inline status_t -fault_get_page(vm_translation_map *map, vm_cache *topCache, - off_t cacheOffset, bool isWrite, vm_page &dummyPage, vm_cache **_sourceCache, +fault_get_page(vm_translation_map *map, vm_cache *topCache, off_t cacheOffset, + bool isWrite, vm_dummy_page &dummyPage, vm_cache **_sourceCache, vm_cache **_copiedSource, vm_page** _page) { vm_cache *cache; @@ -3848,8 +3874,10 @@ if (cacheOffset == 0x12000) break; // The page is busy, wait till it becomes unbusy. + ConditionVariableEntry entry; + entry.Add(newPage); mutex_unlock(&topCache->lock); - snooze(10000); + entry.Wait(); mutex_lock(&topCache->lock); } @@ -3968,7 +3996,7 @@ vm_soft_fault(addr_t originalAddress, bool isWrite, bool isUser) // already have the page we're searching for (we're going from top to bottom) vm_translation_map *map = &addressSpace->translation_map; - vm_page dummyPage; + vm_dummy_page dummyPage; dummyPage.cache = NULL; dummyPage.state = PAGE_STATE_INACTIVE; dummyPage.type = PAGE_TYPE_DUMMY; diff --git a/src/system/kernel/vm/vm_cache.cpp b/src/system/kernel/vm/vm_cache.cpp index 903e1ba66a..d25d6dce9c 100644 --- a/src/system/kernel/vm/vm_cache.cpp +++ b/src/system/kernel/vm/vm_cache.cpp @@ -6,23 +6,24 @@ * Distributed under the terms of the NewOS License. */ - -#include -#include -#include #include -#include -#include -#include -#include -#include -#include -#include -#include #include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + //#define TRACE_VM_CACHE #ifdef TRACE_VM_CACHE # define TRACE(x) dprintf x @@ -479,9 +480,12 @@ vm_cache_remove_consumer(vm_cache *cache, vm_cache *consumer) } } + ConditionVariable busyCondition; + if (merge) { // But since we need to keep the locking order upper->lower cache, we // need to unlock our cache now + busyCondition.Publish(cache, "cache"); cache->busy = true; mutex_unlock(&cache->lock); @@ -496,6 +500,7 @@ vm_cache_remove_consumer(vm_cache *cache, vm_cache *consumer) "not merging it\n", cache); merge = false; cache->busy = false; + busyCondition.Unpublish(); mutex_unlock(&consumer->lock); vm_cache_release_ref(consumer); } @@ -535,6 +540,7 @@ if (consumer->virtual_base == 0x11000) //dprintf("%ld: merged busy page %p, cache %p, offset %ld\n", find_thread(NULL), page, cacheRef->cache, page->cache_offset); vm_cache_remove_page(consumer, consumerPage); consumerPage->state = PAGE_STATE_INACTIVE; + ((vm_dummy_page*)consumerPage)->busy_condition.Unpublish(); vm_cache_remove_page(cache, page); vm_cache_insert_page(consumer, page, @@ -582,6 +588,9 @@ panic("cacheRef %p ref count too low!\n", cache); mutex_unlock(&consumer->lock); vm_cache_release_ref(consumer); } + + if (cache->busy) + busyCondition.Unpublish(); } mutex_unlock(&cache->lock); diff --git a/src/system/kernel/vm/vm_page.c b/src/system/kernel/vm/vm_page.cpp similarity index 97% rename from src/system/kernel/vm/vm_page.c rename to src/system/kernel/vm/vm_page.cpp index 9b92062328..f9046a6088 100644 --- a/src/system/kernel/vm/vm_page.c +++ b/src/system/kernel/vm/vm_page.cpp @@ -10,16 +10,17 @@ #include #include -#include #include +#include +#include +#include +#include #include #include #include #include #include #include -#include -#include #include #include @@ -215,7 +216,6 @@ find_page(int argc, char **argv) static int dump_page(int argc, char **argv) { - struct vm_page_mapping *mapping; struct vm_page *page; addr_t address; bool physical = false; @@ -277,8 +277,9 @@ dump_page(int argc, char **argv) #endif // DEBUG_PAGE_CACHE_TRANSITIONS kprintf("area mappings:\n"); - mapping = page->mappings; - while (mapping != NULL) { + vm_page_mappings::Iterator iterator = page->mappings.GetIterator(); + vm_page_mapping *mapping; + while ((mapping = iterator.Next()) != NULL) { kprintf(" %p (%#lx)\n", mapping->area, mapping->area->id); mapping = mapping->page_link.next; } @@ -676,6 +677,7 @@ vm_page_write_modified(vm_cache *cache, bool fsReenter) off_t pageOffset; status_t status; vm_area *area; + ConditionVariable busyCondition; cpu_status state = disable_interrupts(); acquire_spinlock(&sPageLock); @@ -683,20 +685,22 @@ vm_page_write_modified(vm_cache *cache, bool fsReenter) if (page->state == PAGE_STATE_MODIFIED) { remove_page_from_queue(&page_modified_queue, page); page->state = PAGE_STATE_BUSY; + busyCondition.Publish(page, "page"); gotPage = true; } release_spinlock(&sPageLock); restore_interrupts(state); - // We may have a modified page - however, while we're writing it back, the page - // is still mapped. In order not to lose any changes to the page, we mark it clean - // before actually writing it back; if writing the page fails for some reason, we - // just keep it in the modified page list, but that should happen only rarely. + // We may have a modified page - however, while we're writing it back, + // the page is still mapped. In order not to lose any changes to the + // page, we mark it clean before actually writing it back; if writing + // the page fails for some reason, we just keep it in the modified page + // list, but that should happen only rarely. - // If the page is changed after we cleared the dirty flag, but before we had - // the chance to write it back, then we'll write it again later - that will - // probably not happen that often, though. + // If the page is changed after we cleared the dirty flag, but before we + // had the chance to write it back, then we'll write it again later - + // that will probably not happen that often, though. pageOffset = (off_t)page->cache_offset << PAGE_SHIFT; @@ -713,6 +717,7 @@ vm_page_write_modified(vm_cache *cache, bool fsReenter) map->ops->query(map, pageOffset - area->cache_offset + area->base, &physicalAddress, &flags); if (flags & PAGE_MODIFIED) { +// TODO: Mark busy? gotPage = true; dequeuedPage = false; } @@ -742,11 +747,13 @@ vm_page_write_modified(vm_cache *cache, bool fsReenter) state = disable_interrupts(); acquire_spinlock(&sPageLock); - if (page->mappings != NULL || page->wired_count) + if (!page->mappings.IsEmpty() || page->wired_count) page->state = PAGE_STATE_ACTIVE; else page->state = PAGE_STATE_INACTIVE; + busyCondition.Unpublish(); + enqueue_page(&page_active_queue, page); release_spinlock(&sPageLock); @@ -762,6 +769,8 @@ vm_page_write_modified(vm_cache *cache, bool fsReenter) page->state = PAGE_STATE_MODIFIED; enqueue_page(&page_modified_queue, page); + busyCondition.Unpublish(); + release_spinlock(&sPageLock); restore_interrupts(state); } @@ -828,7 +837,7 @@ vm_page_init(kernel_args *args) sPages[i].physical_page_number = sPhysicalPageOffset + i; sPages[i].type = PAGE_TYPE_PHYSICAL; sPages[i].state = PAGE_STATE_FREE; - sPages[i].mappings = NULL; + new(&sPages[i].mappings) vm_page_mappings(); sPages[i].wired_count = 0; sPages[i].usage_count = 0; sPages[i].cache = NULL;