From 29218807016bc45a0fba153a255878dd1310066b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Thu, 30 Jun 2005 16:23:56 +0000 Subject: [PATCH] Fixed a bad bug in the journaling code: set_blocks_info() was called before the super block was written back. This had a big race condition when all blocks in the transaction were written back before the super block was written. In that case, the log info part of the super block got out of sync and was plain wrong (eventually causing the disk to be unmountable). Also moved the list code over to the kernel's DoublyLinkedList. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@13366 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- .../kernel/file_systems/bfs/r5/Journal.cpp | 90 +++++++++++-------- .../kernel/file_systems/bfs/r5/Journal.h | 30 +++---- .../kernel/file_systems/bfs/r5/Utility.h | 53 ++--------- 3 files changed, 72 insertions(+), 101 deletions(-) diff --git a/src/tests/add-ons/kernel/file_systems/bfs/r5/Journal.cpp b/src/tests/add-ons/kernel/file_systems/bfs/r5/Journal.cpp index 2fde3995b4..f964d21523 100644 --- a/src/tests/add-ons/kernel/file_systems/bfs/r5/Journal.cpp +++ b/src/tests/add-ons/kernel/file_systems/bfs/r5/Journal.cpp @@ -1,8 +1,8 @@ /* Journal - transaction and logging -** -** Initial version by Axel Dörfler, axeld@pinc-software.de -** This file may be used under the terms of the OpenBeOS License. -*/ + * + * Copyright 2001-2005, Axel Dörfler, axeld@pinc-software.de + * This file may be used under the terms of the MIT License. + */ #include "Journal.h" @@ -13,6 +13,14 @@ #include +struct log_entry : public DoublyLinkedListLinkImpl { + uint16 start; + uint16 length; + uint32 cached_blocks; + Journal *journal; +}; + + Journal::Journal(Volume *volume) : fVolume(volume), @@ -186,9 +194,10 @@ Journal::blockNotify(off_t blockNumber, size_t numBlocks, void *arg) // Set log_start pointer if possible... - if (logEntry == journal->fEntries.head) { - if (logEntry->Next() != NULL) { - int32 length = logEntry->next->start - logEntry->start; + if (logEntry == journal->fEntries.First()) { + log_entry *next = journal->fEntries.GetNext(logEntry); + if (next != NULL) { + int32 length = next->start - logEntry->start; superBlock.log_start = (superBlock.log_start + length) % journal->fLogSize; } else superBlock.log_start = journal->fVolume->LogEnd(); @@ -198,7 +207,7 @@ Journal::blockNotify(off_t blockNumber, size_t numBlocks, void *arg) journal->fUsed -= logEntry->length; journal->fEntriesLock.Lock(); - logEntry->Remove(); + journal->fEntries.Remove(logEntry); journal->fEntriesLock.Unlock(); free(logEntry); @@ -211,7 +220,9 @@ Journal::blockNotify(off_t blockNumber, size_t numBlocks, void *arg) if (superBlock.log_start == superBlock.log_end) superBlock.flags = SUPER_BLOCK_DISK_CLEAN; - journal->fVolume->WriteSuperBlock(); + status_t status = journal->fVolume->WriteSuperBlock(); + if (status != B_OK) + FATAL(("blockNotify: could not write back super block: %s\n", strerror(status))); } } @@ -270,23 +281,42 @@ Journal::WriteLogEntry() logPosition = (logPosition + 1) % fLogSize; } + // create and add log entry + log_entry *logEntry = (log_entry *)malloc(sizeof(log_entry)); - if (logEntry != NULL) { - logEntry->start = logStart; - logEntry->length = TransactionSize(); - logEntry->cached_blocks = array->count; - logEntry->journal = this; - - fEntriesLock.Lock(); - fEntries.Add(logEntry); - fEntriesLock.Unlock(); - - fCurrent = logEntry; - fUsed += logEntry->length; - - set_blocks_info(fVolume->Device(), &array->values[0], array->count, blockNotify, logEntry); + if (logEntry == NULL) { + DIE(("Could not create next log entry (out of memory)\n")); + return B_NO_MEMORY; } + logEntry->start = logStart; + logEntry->length = TransactionSize(); + logEntry->cached_blocks = array->count; + logEntry->journal = this; + + fEntriesLock.Lock(); + fEntries.Add(logEntry); + fEntriesLock.Unlock(); + + fCurrent = logEntry; + fUsed += logEntry->length; + + // Update the log end pointer in the super block + fVolume->SuperBlock().flags = SUPER_BLOCK_DISK_DIRTY; + fVolume->SuperBlock().log_end = logPosition; + fVolume->LogEnd() = logPosition; + + status_t status = fVolume->WriteSuperBlock(); + + // We need to flush the drives own cache here to ensure + // disk consistency. + // If that call fails, we can't do anything about it anyway + ioctl(fVolume->Device(), B_FLUSH_DRIVE_CACHE); + + set_blocks_info(fVolume->Device(), &array->values[0], + array->count, blockNotify, logEntry); + fArray.MakeEmpty(); + // If the log goes to the next round (the log is written as a // circular buffer), all blocks will be flushed out which is // possible because we don't have any locked blocks at this @@ -294,19 +324,7 @@ Journal::WriteLogEntry() if (logPosition < logStart) fVolume->FlushDevice(); - // We need to flush the drives own cache here to ensure - // disk consistency. - // If that call fails, we can't do anything about it anyway - ioctl(fVolume->Device(), B_FLUSH_DRIVE_CACHE); - - fArray.MakeEmpty(); - - // Update the log end pointer in the super block - fVolume->SuperBlock().flags = SUPER_BLOCK_DISK_DIRTY; - fVolume->SuperBlock().log_end = logPosition; - fVolume->LogEnd() = logPosition; - - return fVolume->WriteSuperBlock(); + return status; } diff --git a/src/tests/add-ons/kernel/file_systems/bfs/r5/Journal.h b/src/tests/add-ons/kernel/file_systems/bfs/r5/Journal.h index bbc64f0c0d..67c9a52367 100644 --- a/src/tests/add-ons/kernel/file_systems/bfs/r5/Journal.h +++ b/src/tests/add-ons/kernel/file_systems/bfs/r5/Journal.h @@ -8,6 +8,7 @@ #include +#include #ifdef USER # include @@ -20,13 +21,8 @@ #include "Utility.h" -struct log_entry : node { - uint16 start; - uint16 length; - uint32 cached_blocks; - Journal *journal; -}; - +struct log_entry; +typedef DoublyLinkedList LogEntryList; // Locking policy in BFS: if you need both, the volume lock and the // journal lock, you must lock the volume first - or else you will @@ -67,17 +63,17 @@ class Journal { static void blockNotify(off_t blockNumber, size_t numBlocks, void *arg); status_t TransactionDone(bool success); - Volume *fVolume; + Volume *fVolume; RecursiveLock fLock; - Transaction *fOwner; - BlockArray fArray; - uint32 fLogSize, fMaxTransactionSize, fUsed; - int32 fTransactionsInEntry; - SimpleLock fEntriesLock; - list fEntries; - log_entry *fCurrent; - bool fHasChangedBlocks; - bigtime_t fTimestamp; + Transaction *fOwner; + BlockArray fArray; + uint32 fLogSize, fMaxTransactionSize, fUsed; + int32 fTransactionsInEntry; + SimpleLock fEntriesLock; + LogEntryList fEntries; + log_entry *fCurrent; + bool fHasChangedBlocks; + bigtime_t fTimestamp; }; diff --git a/src/tests/add-ons/kernel/file_systems/bfs/r5/Utility.h b/src/tests/add-ons/kernel/file_systems/bfs/r5/Utility.h index 5304a28b0e..ef6e5df7e5 100644 --- a/src/tests/add-ons/kernel/file_systems/bfs/r5/Utility.h +++ b/src/tests/add-ons/kernel/file_systems/bfs/r5/Utility.h @@ -1,10 +1,10 @@ +/* Utility - some helper classes + * + * Copyright 2001-2005, Axel Dörfler, axeld@pinc-software.de + * This file may be used under the terms of the MIT License. + */ #ifndef UTILITY_H #define UTILITY_H -/* Utility - some helper classes -** -** Initial version by Axel Dörfler, axeld@pinc-software.de -** This file may be used under the terms of the OpenBeOS License. -*/ #include @@ -69,49 +69,6 @@ class BlockArray { }; -// Doubly linked list - -template struct node { - Node *next,*prev; - - void - Remove() - { - prev->next = next; - next->prev = prev; - } - - Node * - Next() - { - if (next && next->next != NULL) - return next; - - return NULL; - } -}; - -template struct list { - Node *head,*tail,*last; - - list() - { - head = (Node *)&tail; - tail = NULL; - last = (Node *)&head; - } - - void - Add(Node *entry) - { - entry->next = (Node *)&tail; - entry->prev = last; - last->next = entry; - last = entry; - } -}; - - // Some atomic operations that are somehow missing in BeOS: // // _atomic_test_and_set(value, newValue, testAgainst)