[haiku-commits] BRANCH ahenriksson-github.production - src/add-ons/kernel/file_systems/bfs
- From: ahenriksson-github.production <community@xxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Tue, 3 Jul 2012 14:49:08 +0200 (CEST)
added 4 changesets to branch 'refs/remotes/ahenriksson-github/production'
old head: 62e1c717aeab270bf368387c0f768e6e43b1bfc1
new head: debc1307496dc87e1ec14c6bb0630e6d58f852b2
----------------------------------------------------------------------------
6498e87: Tweaks to stream moving code
5b66a74: Explain why we can't use Replace in Index::UpdateInode (duplicates)
5b70193: Don't use 'data' as a data_stream name unless it refers to the Inode
stream
debc130: Logic error in _WriteBufferedRuns() that was hidden by the big buffer
[ ahenriksson <sausageboy@xxxxxxxxx> ]
----------------------------------------------------------------------------
3 files changed, 153 insertions(+), 136 deletions(-)
src/add-ons/kernel/file_systems/bfs/Index.cpp | 4 +-
src/add-ons/kernel/file_systems/bfs/Inode.cpp | 267 +++++++++++----------
src/add-ons/kernel/file_systems/bfs/Inode.h | 18 +-
############################################################################
Commit: 6498e8701c8e6cdf5abc9c4759c5d44587fa6758
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Mon Jul 2 14:29:56 2012 UTC
Tweaks to stream moving code
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
index 882ba9c..356239c 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
@@ -164,13 +164,13 @@ private:
*/
class BlockRunBuffer {
public:
-
BlockRunBuffer(Volume* volume);
+
BlockRunBuffer(Volume* volume,
+ bool
useBlockCache);
~BlockRunBuffer();
- status_t GetBlocks(const uint8**
buffer,
- off_t
blocksRequested,
+ const uint8* GetBlocks(off_t blocksRequested,
off_t&
blocksWritten);
- status_t PutRun(block_run run);
+ void PutRun(block_run run);
off_t NumBlocks() const {
return fNumBlocks; }
bool IsEmpty() const {
return fNumBlocks == 0; }
bool IsFull() const;
@@ -181,6 +181,7 @@ public:
private:
Volume* fVolume;
+ bool fUseBlockCache;
// fields to manage the circular buffer of block_run's
block_run* fBuffer;
@@ -195,9 +196,8 @@ private:
// buffer to store block data as we write it to disk
uint8* fDataBuffer;
- uint32 fDataBufferSize;
- static const int32 kTargetDataBufferSize = 16 *
1024 * 1024;
- // ==
16 MB
+ static const int32 kDataBufferSize = 32 * 1024;
+ // ==
32 kB
};
@@ -336,9 +336,10 @@ InodeAllocator::_TransactionListener(int32 id, int32
event, void* _inode)
// #pragma mark -
-BlockRunBuffer::BlockRunBuffer(Volume* volume)
+BlockRunBuffer::BlockRunBuffer(Volume* volume, bool useBlockCache)
:
fVolume(volume),
+ fUseBlockCache(useBlockCache),
fBuffer(NULL),
fDataBuffer(NULL)
{
@@ -370,33 +371,25 @@ BlockRunBuffer::IsFull() const
}
-status_t
+void
BlockRunBuffer::PutRun(block_run run)
{
- if (fBuffer == NULL || fDataBuffer == NULL)
- return B_NO_INIT;
-
- if (IsFull())
- return B_NO_MEMORY;
+ ASSERT(fBuffer != NULL && fDataBuffer != NULL && !IsFull());
fBuffer[fTail] = run;
fTail = (fTail + 1) % kSize;
fNumBlocks += run.Length();
-
- return B_OK;
}
-status_t
-BlockRunBuffer::GetBlocks(const uint8** buffer, off_t blocksRequested,
- off_t& blocksWritten)
+const uint8*
+BlockRunBuffer::GetBlocks(off_t blocksRequested, off_t& blocksWritten)
{
- if (fBuffer == NULL || fDataBuffer == NULL)
- return B_NO_INIT;
+ ASSERT(fBuffer != NULL && fDataBuffer != NULL);
uint32 blockShift = fVolume->BlockShift();
- uint32 blockSize = fVolume->BlockSize();
+ int32 blockSize = fVolume->BlockSize();
// the amount of blocks we can return is the minimum of the blocks
// requested, the blocks in the buffer and the size of the data buffer
@@ -405,15 +398,28 @@ BlockRunBuffer::GetBlocks(const uint8** buffer, off_t
blocksRequested,
if (blocksWritten > fNumBlocks)
blocksWritten = fNumBlocks;
- if (blocksWritten > (fDataBufferSize >> blockShift))
- blocksWritten = fDataBufferSize >> blockShift;
+ if (blocksWritten > (kDataBufferSize >> blockShift))
+ blocksWritten = kDataBufferSize >> blockShift;
CachedBlock cached(fVolume);
+
for (off_t i = 0; i < blocksWritten; i++) {
- const uint8* data
- = cached.SetTo(fVolume->ToBlock(fBuffer[fHead]) +
fOffset);
+ off_t blockNumber = fVolume->ToBlock(fBuffer[fHead]) + fOffset;
- memcpy(fDataBuffer + (i << blockShift), data, blockSize);
+ if (fUseBlockCache) {
+ const uint8* data = cached.SetTo(blockNumber);
+ if (data == NULL)
+ return NULL;
+
+ memcpy(fDataBuffer + (i << blockShift), data,
blockSize);
+ } else {
+ ssize_t bytesRead = read_pos(fVolume->Device(),
+ blockNumber << blockShift, fDataBuffer + (i <<
blockShift),
+ blockSize);
+
+ if (bytesRead != blockSize)
+ return NULL;
+ }
if (++fOffset == fBuffer[fHead].Length()) {
fHead = (fHead + 1) % kSize;
@@ -424,8 +430,7 @@ BlockRunBuffer::GetBlocks(const uint8** buffer, off_t
blocksRequested,
ASSERT(fNumBlocks >= 0);
}
- *buffer = fDataBuffer;
- return B_OK;
+ return fDataBuffer;
}
@@ -434,26 +439,11 @@ BlockRunBuffer::AllocateBuffers()
{
// allocate block run buffer
fBuffer = new(std::nothrow) block_run[kSize];
- if (fBuffer == NULL)
- return B_NO_MEMORY;
-
- // allocate data buffer
- fDataBufferSize = kTargetDataBufferSize;
-
- while (fDataBufferSize > fVolume->BlockSize()) {
- fDataBuffer = new(std::nothrow) uint8[fDataBufferSize];
- if (fDataBuffer != NULL)
- return B_OK;
-
- fDataBufferSize /= 4;
- }
+ fDataBuffer = new(std::nothrow) uint8[kDataBufferSize];
- // don't allocate a data buffer which is smaller than the block size
- fDataBuffer = new(std::nothrow) uint8[fVolume->BlockSize()];
- if (fDataBuffer == NULL)
+ if (fBuffer == NULL || fDataBuffer == NULL)
return B_NO_MEMORY;
- fDataBufferSize = fVolume->BlockSize();
return B_OK;
}
@@ -2733,18 +2723,18 @@ Inode::_WriteBufferedRuns(Transaction& transaction,
BlockRunBuffer& buffer,
}
// copy the stream data to our run
- for (off_t i = 0; i < blocksToWrite; ) {
+ for (off_t blocksWritten = 0; blocksWritten < blocksToWrite; ) {
off_t blocksRead;
- const uint8* sourceData;
- status = buffer.GetBlocks(&sourceData, blocksToWrite,
blocksRead);
- if (status != B_OK)
- return status;
+ const uint8* sourceData
+ = buffer.GetBlocks(blocksToWrite, blocksRead);
+ if (sourceData == NULL)
+ return B_IO_ERROR;
// Don't use the journal to copy file data. These
blocks are
// currently not referenced by the file system in any
way, so
// we'll be in a valid state even if the transaction
fails.
- off_t targetBlock = fVolume->ToBlock(run) + i;
+ off_t targetBlock = fVolume->ToBlock(run) +
blocksWritten;
ssize_t written = write_pos(fVolume->Device(),
targetBlock << blockShift, sourceData,
@@ -2753,7 +2743,7 @@ Inode::_WriteBufferedRuns(Transaction& transaction,
BlockRunBuffer& buffer,
if (written != blocksRead << blockShift)
return B_IO_ERROR;
- i += blocksRead;
+ blocksWritten += blocksRead;
}
}
return B_OK;
@@ -2781,7 +2771,7 @@ Inode::_GetNextBlockRun(off_t& position, block_run& run)
const
bool
-Inode::_BlockRunInRange(block_run run, off_t beginBlock, off_t endBlock) const
+Inode::_IsBlockRunInRange(block_run run, off_t beginBlock, off_t endBlock)
const
{
off_t runStart = GetVolume()->ToBlock(run);
return runStart >= beginBlock && runStart + run.Length() <= endBlock;
@@ -2789,10 +2779,10 @@ Inode::_BlockRunInRange(block_run run, off_t
beginBlock, off_t endBlock) const
bool
-Inode::_BlockRunOutsideRange(block_run run, off_t beginBlock, off_t endBlock)
+Inode::_IsBlockRunOutsideRange(block_run run, off_t beginBlock, off_t endBlock)
const
{
- // note that this is not the opposite of _BlockRunInRange, as we only
+ // note that this is not the opposite of _IsBlockRunInRange, as we only
// return true if the block run is completely outside the range.
off_t runStart = GetVolume()->ToBlock(run);
return runStart + run.Length() <= beginBlock || runStart >= endBlock;
@@ -2805,10 +2795,30 @@ Inode::_BlockRunOutsideRange(block_run run, off_t
beginBlock, off_t endBlock)
status_t
Inode::MoveStream(off_t beginBlock, off_t endBlock)
{
+ // We move the stream by rebuilding it from scratch. block_runs which
+ // can be reused are simply added to the result stream (newStream) as
+ // they are. Runs which can't, are added to a buffer, and the stream
+ // data is copied to newly allocated block_runs when necessary.
+ //
+ // This is complicated by the fact that the length of block_runs in the
+ // double indirect range needs to be a multiple of a certain length.
+ // Two problems result from this:
+ //
+ // 1. We can't reuse a block_run from the original stream if it has
+ // the wrong length, even though it's inside the allowed range.
+ //
+ // 2. The number of blocks in the BlockRunBuffer might not be a
+ // multiple of this base length. When that happens, we can't write
+ // all the blocks to disk, so we can't reuse the block_run from
+ // the original stream even if it has a valid size and location.
+ //
+ // Note that this only applies when we're adding block_runs to the
+ // double indirect range of newStream.
+
status_t status;
data_stream data = {};
- BlockRunBuffer buffer(fVolume);
+ BlockRunBuffer buffer(fVolume, IsDirectory());
status = buffer.AllocateBuffers();
if (status != B_OK)
return status;
@@ -2831,15 +2841,15 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
if (status != B_OK)
return status;
- if (_BlockRunInRange(run, beginBlock, endBlock)) {
+ if (_IsBlockRunInRange(run, beginBlock, endBlock)) {
status = _WriteBufferedRuns(transaction, buffer, &data,
Size(),
false, beginBlock, endBlock);
if (status != B_OK)
return status;
- // if we're writing to the double indirect range, we
might not have
- // cleared the buffer
-
+ // If we're writing to the double indirect range, we
might not have
+ // cleared the buffer. Before we add this block_run to
the stream,
+ // make sure that this is not the case.
if (buffer.IsEmpty()) {
int32 rest;
status = _AddBlockRun(transaction, &data, run,
Size(), &rest,
@@ -2847,6 +2857,8 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
if (status != B_OK)
return status;
+ // if there's no rest, we've managed to add
this run to the
+ // output stream, and can move on to the next
if (rest == 0)
continue;
@@ -2864,7 +2876,7 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
}
buffer.PutRun(run);
- if (_BlockRunOutsideRange(run, beginBlock, endBlock)) {
+ if (_IsBlockRunOutsideRange(run, beginBlock, endBlock)) {
// The block run was outside of the allowed range,
which means
// that we can free it now without running the risk of
// reallocating it and overwriting its content.
@@ -2905,7 +2917,7 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
but not including \endBlock.
*/
status_t
-Inode::StreamInRange(bool& inRange, off_t beginBlock, off_t endBlock) const
+Inode::StreamInRange(off_t beginBlock, off_t endBlock, bool& inRange) const
{
inRange = false;
// now we can simply return if we find a run which is not in
the range
@@ -2914,12 +2926,12 @@ Inode::StreamInRange(bool& inRange, off_t beginBlock,
off_t endBlock) const
const data_stream* data = &fNode.data;
if (!data->indirect.IsZero()
- && !_BlockRunInRange(data->indirect, beginBlock, endBlock)) {
+ && !_IsBlockRunInRange(data->indirect, beginBlock, endBlock)) {
return B_OK;
}
if (!data->double_indirect.IsZero()) {
- if (!_BlockRunInRange(data->double_indirect, beginBlock,
endBlock))
+ if (!_IsBlockRunInRange(data->double_indirect, beginBlock,
endBlock))
return B_OK;
int32 runsPerBlock = fVolume->BlockSize() / sizeof(block_run);
@@ -2938,7 +2950,7 @@ Inode::StreamInRange(bool& inRange, off_t beginBlock,
off_t endBlock) const
break;
}
- if (!_BlockRunInRange(runArray[i], beginBlock,
endBlock))
+ if (!_IsBlockRunInRange(runArray[i],
beginBlock, endBlock))
return B_OK;
}
@@ -2958,7 +2970,7 @@ Inode::StreamInRange(bool& inRange, off_t beginBlock,
off_t endBlock) const
if (status != B_OK)
return status;
- if (!_BlockRunInRange(run, beginBlock, endBlock))
+ if (!_IsBlockRunInRange(run, beginBlock, endBlock))
return B_OK;
}
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.h
b/src/add-ons/kernel/file_systems/bfs/Inode.h
index 46148c4..2d6b122 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.h
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.h
@@ -153,9 +153,8 @@ public:
bool NeedsTrimming() const;
status_t MoveStream(off_t
beginBlock, off_t endBlock);
- status_t StreamInRange(bool&
inRange, off_t beginBlock,
- off_t
endBlock) const;
-
+ status_t StreamInRange(off_t
beginBlock, off_t endBlock,
+ bool&
inRange) const;
status_t Free(Transaction&
transaction);
status_t Sync();
@@ -270,9 +269,9 @@ private:
status_t _GetNextBlockRun(off_t&
position,
block_run& run) const;
- bool
_BlockRunInRange(block_run run,
+ bool
_IsBlockRunInRange(block_run run,
off_t
beginBlock, off_t endBlock) const;
- bool
_BlockRunOutsideRange(block_run run,
+ bool
_IsBlockRunOutsideRange(block_run run,
off_t
beginBlock, off_t endBlock) const;
private:
############################################################################
Commit: 5b66a74c5e609b7de6fcb9cb2f45a1be425c607d
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Mon Jul 2 17:28:49 2012 UTC
Explain why we can't use Replace in Index::UpdateInode (duplicates)
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/Index.cpp
b/src/add-ons/kernel/file_systems/bfs/Index.cpp
index 2ee601b..c66a879 100644
--- a/src/add-ons/kernel/file_systems/bfs/Index.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Index.cpp
@@ -389,7 +389,9 @@ status_t
Index::UpdateInode(Transaction& transaction, const uint8* key, uint16 length,
off_t oldInodeID, off_t newInodeID)
{
- // remove node and insert it with the new id
+ // remove node and insert it with the new id (we can't use
+ // BPlusTree::Replace, as it doesn't handle trees where duplicates
+ // are allowed)
BPlusTree* tree = Node()->Tree();
status_t status = tree->Remove(transaction, key, length, oldInodeID);
############################################################################
Commit: 5b70193647d92c567f117f8dc2e3cf313857aedb
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Mon Jul 2 19:33:11 2012 UTC
Don't use 'data' as a data_stream name unless it refers to the Inode stream
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
index 356239c..07e35a7 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
@@ -1845,7 +1845,7 @@ Inode::_AllocateBlockArray(Transaction& transaction,
block_run& run,
}
-/*! Adds \a run to \a data, allocating indirection blocks if necessary.
+/*! Adds \a run to \a dataStream, allocating indirection blocks if necessary.
If the block run cannot be added as is, due to constraints on block run
size in the double indirect range, \a rest is set to the number of
blocks
that need to be shaved off the run and the function returns.
@@ -1854,74 +1854,75 @@ Inode::_AllocateBlockArray(Transaction& transaction,
block_run& run,
size is set to the target file size.
*/
status_t
-Inode::_AddBlockRun(Transaction& transaction, data_stream* data, block_run run,
- off_t targetSize, int32* rest, off_t beginBlock, off_t endBlock)
+Inode::_AddBlockRun(Transaction& transaction, data_stream* dataStream,
+ block_run run, off_t targetSize, int32* rest, off_t beginBlock,
+ off_t endBlock)
{
status_t status;
if (rest)
*rest = 0;
- bool cutSize = targetSize < data->Size()
+ bool cutSize = targetSize < dataStream->Size()
+ (run.Length() << fVolume->BlockShift());
// if adding this block_run means overshooting the target
stream size,
- // we need to set data->size to targetSize.
+ // we need to set dataStream->size to targetSize.
// Direct block range
- if (data->Size() <= data->MaxDirectRange()) {
+ if (dataStream->Size() <= dataStream->MaxDirectRange()) {
// let's try to put them into the direct block range
int32 free = 0;
for (; free < NUM_DIRECT_BLOCKS; free++) {
- if (data->direct[free].IsZero())
+ if (dataStream->direct[free].IsZero())
break;
}
if (free < NUM_DIRECT_BLOCKS) {
// can we merge the last allocated run with the new one?
int32 last = free - 1;
- if (free > 0 && data->direct[last].MergeableWith(run)) {
- data->direct[last].length =
HOST_ENDIAN_TO_BFS_INT16(
- data->direct[last].Length() +
run.Length());
+ if (free > 0 &&
dataStream->direct[last].MergeableWith(run)) {
+ dataStream->direct[last].length =
HOST_ENDIAN_TO_BFS_INT16(
+ dataStream->direct[last].Length() +
run.Length());
} else
- data->direct[free] = run;
+ dataStream->direct[free] = run;
- data->max_direct_range = HOST_ENDIAN_TO_BFS_INT64(
- data->MaxDirectRange()
+ dataStream->max_direct_range = HOST_ENDIAN_TO_BFS_INT64(
+ dataStream->MaxDirectRange()
+ run.Length() * fVolume->BlockSize());
- data->size = cutSize ?
HOST_ENDIAN_TO_BFS_INT64(targetSize)
- : data->max_direct_range;
+ dataStream->size = cutSize ?
HOST_ENDIAN_TO_BFS_INT64(targetSize)
+ : dataStream->max_direct_range;
return B_OK;
}
}
// Indirect block range
- if (data->Size() <= data->MaxIndirectRange()
- || !data->MaxIndirectRange()) {
+ if (dataStream->Size() <= dataStream->MaxIndirectRange()
+ || !dataStream->MaxIndirectRange()) {
CachedBlock cached(fVolume);
block_run* runs = NULL;
uint32 free = 0;
off_t block;
// if there is no indirect block yet, create one
- if (data->indirect.IsZero()) {
- status = _AllocateBlockArray(transaction,
data->indirect,
+ if (dataStream->indirect.IsZero()) {
+ status = _AllocateBlockArray(transaction,
dataStream->indirect,
NUM_ARRAY_BLOCKS, true, beginBlock, endBlock);
if (status != B_OK)
return status;
- data->max_indirect_range = data->max_direct_range;
+ dataStream->max_indirect_range =
dataStream->max_direct_range;
// insert the block_run in the first block
- runs = (block_run*)cached.SetTo(data->indirect);
+ runs = (block_run*)cached.SetTo(dataStream->indirect);
} else {
uint32 numberOfRuns = fVolume->BlockSize() /
sizeof(block_run);
- block = fVolume->ToBlock(data->indirect);
+ block = fVolume->ToBlock(dataStream->indirect);
// search first empty entry
int32 i = 0;
- for (; i < data->indirect.Length(); i++) {
+ for (; i < dataStream->indirect.Length(); i++) {
if ((runs = (block_run*)cached.SetTo(block +
i)) == NULL)
return B_IO_ERROR;
@@ -1932,7 +1933,7 @@ Inode::_AddBlockRun(Transaction& transaction,
data_stream* data, block_run run,
if (free < numberOfRuns)
break;
}
- if (i == data->indirect.Length())
+ if (i == dataStream->indirect.Length())
runs = NULL;
}
@@ -1949,26 +1950,26 @@ Inode::_AddBlockRun(Transaction& transaction,
data_stream* data, block_run run,
} else
runs[free] = run;
- data->max_indirect_range = HOST_ENDIAN_TO_BFS_INT64(
- data->MaxIndirectRange()
+ dataStream->max_indirect_range =
HOST_ENDIAN_TO_BFS_INT64(
+ dataStream->MaxIndirectRange()
+ (run.Length() << fVolume->BlockShift()));
- data->size = cutSize ?
HOST_ENDIAN_TO_BFS_INT64(targetSize)
- : data->max_indirect_range;
+ dataStream->size = cutSize ?
HOST_ENDIAN_TO_BFS_INT64(targetSize)
+ : dataStream->max_indirect_range;
return B_OK;
}
}
// Double indirect block range
- if (data->Size() <= data->MaxDoubleIndirectRange()
- || !data->max_double_indirect_range) {
+ if (dataStream->Size() <= dataStream->MaxDoubleIndirectRange()
+ || !dataStream->max_double_indirect_range) {
// We make sure here that we have this minimum allocated, so if
// the allocation succeeds, we don't run into an endless loop.
uint16 doubleIndirectBlockLength;
- if (!data->max_double_indirect_range)
+ if (!dataStream->max_double_indirect_range)
doubleIndirectBlockLength =
_DoubleIndirectBlockLength();
else
- doubleIndirectBlockLength =
data->double_indirect.Length();
+ doubleIndirectBlockLength =
dataStream->double_indirect.Length();
if ((run.Length() % doubleIndirectBlockLength) != 0) {
// The number of allocated blocks isn't a multiple of
@@ -1985,14 +1986,15 @@ Inode::_AddBlockRun(Transaction& transaction,
data_stream* data, block_run run,
}
// if there is no double indirect block yet, create one
- if (data->double_indirect.IsZero()) {
+ if (dataStream->double_indirect.IsZero()) {
status = _AllocateBlockArray(transaction,
- data->double_indirect,
_DoubleIndirectBlockLength(),
+ dataStream->double_indirect,
_DoubleIndirectBlockLength(),
beginBlock, endBlock);
if (status != B_OK)
return status;
- data->max_double_indirect_range =
data->max_indirect_range;
+ dataStream->max_double_indirect_range =
+ dataStream->max_indirect_range;
}
// calculate the index where to insert the new blocks
@@ -2000,11 +2002,11 @@ Inode::_AddBlockRun(Transaction& transaction,
data_stream* data, block_run run,
int32 runsPerBlock;
int32 directSize;
int32 indirectSize;
- get_double_indirect_sizes(data->double_indirect.Length(),
+ get_double_indirect_sizes(dataStream->double_indirect.Length(),
fVolume->BlockSize(), runsPerBlock, directSize,
indirectSize);
- off_t start = data->MaxDoubleIndirectRange()
- - data->MaxIndirectRange();
+ off_t start = dataStream->MaxDoubleIndirectRange()
+ - dataStream->MaxIndirectRange();
int32 indirectIndex = start / indirectSize;
int32 index = (start % indirectSize) / directSize;
int32 runsPerArray = runsPerBlock * doubleIndirectBlockLength;
@@ -2025,7 +2027,7 @@ Inode::_AddBlockRun(Transaction& transaction,
data_stream* data, block_run run,
return EFBIG;
array =
(block_run*)cached.SetTo(fVolume->ToBlock(
- data->double_indirect) + block);
+ dataStream->double_indirect) + block);
if (array == NULL)
return B_IO_ERROR;
}
@@ -2037,7 +2039,8 @@ Inode::_AddBlockRun(Transaction& transaction,
data_stream* data, block_run run,
status =
_AllocateBlockArray(transaction,
array[indirectIndex %
runsPerBlock],
- data->double_indirect.Length(),
beginBlock, endBlock);
+
dataStream->double_indirect.Length(), beginBlock,
+ endBlock);
if (status != B_OK)
return status;
}
@@ -2070,11 +2073,11 @@ Inode::_AddBlockRun(Transaction& transaction,
data_stream* data, block_run run,
}
}
- data->max_double_indirect_range = HOST_ENDIAN_TO_BFS_INT64(
- data->MaxDoubleIndirectRange()
+ dataStream->max_double_indirect_range =
HOST_ENDIAN_TO_BFS_INT64(
+ dataStream->MaxDoubleIndirectRange()
+ (runLength << fVolume->BlockShift()));
- data->size = cutSize ? HOST_ENDIAN_TO_BFS_INT64(targetSize)
- : data->max_double_indirect_range;
+ dataStream->size = cutSize ?
HOST_ENDIAN_TO_BFS_INT64(targetSize)
+ : dataStream->max_double_indirect_range;
return B_OK;
}
@@ -2648,13 +2651,13 @@ Inode::Sync()
}
-/*! Write the block runs in \a buffer to \a data.
+/*! Write the block runs in \a buffer to \a dataStream.
If \a flush is set, we always empty the buffer even if it means
allocating a block run which is larger than the availible blocks.
*/
status_t
Inode::_WriteBufferedRuns(Transaction& transaction, BlockRunBuffer& buffer,
- data_stream* data, off_t targetSize, bool flush, off_t beginBlock,
+ data_stream* dataStream, off_t targetSize, bool flush, off_t beginBlock,
off_t endBlock)
{
status_t status;
@@ -2662,8 +2665,8 @@ Inode::_WriteBufferedRuns(Transaction& transaction,
BlockRunBuffer& buffer,
uint32 blockShift = fVolume->BlockShift();
- bool inDoubleIndirect = data->MaxIndirectRange() != 0
- && data->Size() > data->MaxIndirectRange();
+ bool inDoubleIndirect = dataStream->MaxIndirectRange() != 0
+ && dataStream->Size() > dataStream->MaxIndirectRange();
// we might be about to add a block_run to the double indirect
range
// even if this is false, we'll discover that case when we try
@@ -2704,7 +2707,7 @@ Inode::_WriteBufferedRuns(Transaction& transaction,
BlockRunBuffer& buffer,
// add the run to the data stream
int32 rest;
- status = _AddBlockRun(transaction, data, run, targetSize,
+ status = _AddBlockRun(transaction, dataStream, run, targetSize,
&rest, beginBlock, endBlock);
if (status != B_OK)
return status;
@@ -2816,7 +2819,7 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
// double indirect range of newStream.
status_t status;
- data_stream data = {};
+ data_stream newStream = {};
BlockRunBuffer buffer(fVolume, IsDirectory());
status = buffer.AllocateBuffers();
@@ -2842,8 +2845,8 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
return status;
if (_IsBlockRunInRange(run, beginBlock, endBlock)) {
- status = _WriteBufferedRuns(transaction, buffer, &data,
Size(),
- false, beginBlock, endBlock);
+ status = _WriteBufferedRuns(transaction, buffer,
&newStream,
+ Size(), false, beginBlock, endBlock);
if (status != B_OK)
return status;
@@ -2852,8 +2855,8 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
// make sure that this is not the case.
if (buffer.IsEmpty()) {
int32 rest;
- status = _AddBlockRun(transaction, &data, run,
Size(), &rest,
- beginBlock, endBlock);
+ status = _AddBlockRun(transaction, &newStream,
run, Size(),
+ &rest, beginBlock, endBlock);
if (status != B_OK)
return status;
@@ -2869,8 +2872,8 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
// We can't reuse this block_run, add it to the block_run
buffer.
if (buffer.IsFull()) {
- status = _WriteBufferedRuns(transaction, buffer, &data,
Size(),
- false, beginBlock, endBlock);
+ status = _WriteBufferedRuns(transaction, buffer,
&newStream,
+ Size(), false, beginBlock, endBlock);
if (status != B_OK)
return status;
}
@@ -2892,12 +2895,12 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
}
// write all blocks remaining in the buffer to disk
- status = _WriteBufferedRuns(transaction, buffer, &data, Size(), true,
+ status = _WriteBufferedRuns(transaction, buffer, &newStream, Size(),
true,
beginBlock, endBlock);
if (status != B_OK)
return status;
- ASSERT(data.Size() == Size());
+ ASSERT(newStream.Size() == Size());
// free the blocks we didn't end up reusing
block_run toFree;
@@ -2907,7 +2910,7 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
return status;
}
- fNode.data = data;
+ fNode.data = newStream;
return transaction.Done();
}
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.h
b/src/add-ons/kernel/file_systems/bfs/Inode.h
index 2d6b122..d20db02 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.h
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.h
@@ -257,15 +257,16 @@ private:
status_t
_ShrinkStream(Transaction& transaction,
off_t
size);
status_t
_AddBlockRun(Transaction& transaction,
-
data_stream* data, block_run run,
+
data_stream* dataStream, block_run run,
off_t
targetSize, int32* rest = NULL,
off_t
beginBlock = 0, off_t endBlock = 0);
// moving the data stream
status_t
_WriteBufferedRuns(Transaction& transaction,
-
BlockRunBuffer& buffer, data_stream* data,
- off_t
targetSize, bool flush = false,
- off_t
beginBlock = 0, off_t endBlock = 0);
+
BlockRunBuffer& buffer,
+
data_stream* dataStream, off_t targetSize,
+ bool
flush = false, off_t beginBlock = 0,
+ off_t
endBlock = 0);
status_t _GetNextBlockRun(off_t&
position,
block_run& run) const;
############################################################################
Commit: debc1307496dc87e1ec14c6bb0630e6d58f852b2
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Tue Jul 3 12:12:05 2012 UTC
Logic error in _WriteBufferedRuns() that was hidden by the big buffer
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
index 07e35a7..77db9bf 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
@@ -2730,7 +2730,7 @@ Inode::_WriteBufferedRuns(Transaction& transaction,
BlockRunBuffer& buffer,
off_t blocksRead;
const uint8* sourceData
- = buffer.GetBlocks(blocksToWrite, blocksRead);
+ = buffer.GetBlocks(blocksToWrite -
blocksWritten, blocksRead);
if (sourceData == NULL)
return B_IO_ERROR;
Other related posts:
