From 807d3aa8e370f76f30ee805d7f7f3c2f558b7aa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Fri, 4 Apr 2008 14:02:30 +0000 Subject: [PATCH] * If BFS's Journal::_WriteTransactionToLog() noticed there wasn't enough free space left for the new log entry, it did call cache_sync_transaction(), and then just assumed the space would be ready. But since the transaction could have been written before that call by the block writer, and since the _TransactionWritten() hook is now called asynchronously, cache_sync_transaction() actually has to flush all pending TRANSACTION_WRITTEN notifications before returning to the caller. * To implement this, block_cache now publishs a condition variable, and wait_for_notifications() adds a fake notification that signals that one. Since the notifications are handled in FIFO order, this guarantees that the previous TRANSACTION_WRITTEN hook is done. * notify_transaction_listeners() could accidently delete notifications that still had pending signals. Now, it will defer the deletion to the notification thread instead in that case. This should fix bug #2008. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@24792 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/system/kernel/cache/block_cache.cpp | 52 +++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/src/system/kernel/cache/block_cache.cpp b/src/system/kernel/cache/block_cache.cpp index e6172bde96..dc2bb667f6 100644 --- a/src/system/kernel/cache/block_cache.cpp +++ b/src/system/kernel/cache/block_cache.cpp @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -114,6 +115,7 @@ struct block_cache : DoublyLinkedListLinkImpl { bool read_only; NotificationList pending_notifications; + ConditionVariable condition_variable; bool deleting; block_cache(int fd, off_t numBlocks, size_t blockSize, bool readOnly); @@ -351,7 +353,7 @@ set_notification(cache_transaction *transaction, cache_notification ¬ification, int32 events, transaction_notification_hook hook, void *data) { - notification.transaction_id = transaction->id; + notification.transaction_id = transaction != NULL ? transaction->id : -1; notification.events_pending = 0; notification.events = events; notification.hook = hook; @@ -414,8 +416,14 @@ notify_transaction_listeners(block_cache *cache, cache_transaction *transaction, if ((listener->events & event) != 0) add_notification(cache, listener, event, removeListeners); - else if (removeListeners) - delete listener; + else if (removeListeners) { + // we might need to defer the deletion if its currently in use + MutexLocker locker(sNotificationsLock); + if (listener->events_pending != 0) + listener->delete_after_event = true; + else + delete listener; + } } } @@ -552,6 +560,8 @@ block_cache::block_cache(int _fd, off_t numBlocks, size_t blockSize, sCaches.Add(this); mutex_unlock(&sCachesLock); + condition_variable.Publish(this, "cache transaction sync"); + buffer_cache = create_object_cache_etc("block cache buffers", blockSize, 8, 0, CACHE_LARGE_SLAB, NULL, NULL, NULL, NULL); if (buffer_cache == NULL) @@ -574,6 +584,7 @@ block_cache::block_cache(int _fd, off_t numBlocks, size_t blockSize, } +//! Should be called with the cache's lock held. block_cache::~block_cache() { deleting = true; @@ -582,6 +593,8 @@ block_cache::~block_cache() benaphore_destroy(&lock); + condition_variable.Unpublish(); + mutex_lock(&sCachesLock); sCaches.Remove(this); mutex_unlock(&sCachesLock); @@ -1340,6 +1353,30 @@ dump_caches(int argc, char **argv) #endif // DEBUG_BLOCK_CACHE +static void +notify_sync(int32 transactionID, int32 event, void *_cache) +{ + block_cache *cache = (block_cache *)_cache; + + cache->condition_variable.NotifyOne(); +} + + +static void +wait_for_notifications(block_cache *cache) +{ + // add sync notification + cache_notification notification; + set_notification(NULL, notification, TRANSACTION_WRITTEN, notify_sync, + cache); + add_notification(cache, ¬ification, TRANSACTION_WRITTEN, false); + + // wait for condition + ConditionVariableEntry entry; + entry.Wait(cache); +} + + static block_cache * get_next_locked_block_cache(block_cache *last) { @@ -1417,6 +1454,10 @@ flush_pending_notifications(block_cache *cache) } +/*! Flushes all pending notifications by calling the appropriate hook + functions. + Must not be called with a cache lock held. +*/ static void flush_pending_notifications() { @@ -1644,6 +1685,11 @@ cache_sync_transaction(void *_cache, int32 id) } hash_close(cache->transaction_hash, &iterator, false); + locker.Unlock(); + + wait_for_notifications(cache); + // make sure that all pending TRANSACTION_WRITTEN notifications + // are handled after we return return B_OK; }