[haiku-commits] BRANCH ahenriksson-github.production - src/add-ons/kernel/file_systems/bfs
- From: ahenriksson-github.production <community@xxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Mon, 23 Jul 2012 18:49:37 +0200 (CEST)
added 6 changesets to branch 'refs/remotes/ahenriksson-github/production'
old head: 59ba972f9d6dab2f6728df339b9f56acd3c10d22
new head: 8b54ad6ef747ef971a51e95c855fb6ec6e89e061
----------------------------------------------------------------------------
268574c: Calculate device size in Volume for use by resize code
Volume::fDeviceSize, while not really needed as we're never interested
in the cached device size, is added for orthogonality with
fDeviceBlockSize (which actually isn't used either).
91a341e: Get device size from Volume
68dfc43: Incorrect checking of already set double indirect blocks
Wrong variable usage in inner loop caused some double indirect stream
runs to be checked twice when block size was smaller than
DOUBLE_INDIRECT_ARRAY_SIZE, while some were incorrectly marked as
unallocated in the bitmap.
ff678f7: Double indirect blocks were allocated out of range
4cc09a5: Free indirect blocks when moving a file stream
8b54ad6: Syntax mistake when locking
Not sure why the compiler considered this valid in the first place, to
be honest.
[ ahenriksson <sausageboy@xxxxxxxxx> ]
----------------------------------------------------------------------------
7 files changed, 101 insertions(+), 77 deletions(-)
.../kernel/file_systems/bfs/CheckVisitor.cpp | 2 +-
src/add-ons/kernel/file_systems/bfs/Inode.cpp | 54 +++++++++-
src/add-ons/kernel/file_systems/bfs/Inode.h | 2 +
.../kernel/file_systems/bfs/ResizeVisitor.cpp | 28 +----
.../kernel/file_systems/bfs/ResizeVisitor.h | 2 -
src/add-ons/kernel/file_systems/bfs/Volume.cpp | 86 +++++++---------
src/add-ons/kernel/file_systems/bfs/Volume.h | 4 +
############################################################################
Commit: 268574cec5bf6e5fce5090d2cd39e80152297865
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Fri Jul 20 11:56:31 2012 UTC
Calculate device size in Volume for use by resize code
Volume::fDeviceSize, while not really needed as we're never interested
in the cached device size, is added for orthogonality with
fDeviceBlockSize (which actually isn't used either).
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/Volume.cpp
b/src/add-ons/kernel/file_systems/bfs/Volume.cpp
index a61faa7..a212e8b 100644
--- a/src/add-ons/kernel/file_systems/bfs/Volume.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Volume.cpp
@@ -46,8 +46,6 @@ public:
int Mode() const { return fMode; }
bool IsReadOnly() const { return
_IsReadOnly(fMode); }
- status_t GetSize(off_t* _size, uint32*
_blockSize = NULL);
-
private:
static bool _IsReadOnly(int mode)
{ return (mode &
O_RWMASK) == O_RDONLY;}
@@ -156,38 +154,6 @@ DeviceOpener::Keep()
}
-/*! Returns the size of the device in bytes. It uses B_GET_GEOMETRY
- to compute the size, or fstat() if that failed.
-*/
-status_t
-DeviceOpener::GetSize(off_t* _size, uint32* _blockSize)
-{
- device_geometry geometry;
- if (ioctl(fDevice, B_GET_GEOMETRY, &geometry) < 0) {
- // maybe it's just a file
- struct stat stat;
- if (fstat(fDevice, &stat) < 0)
- return B_ERROR;
-
- if (_size)
- *_size = stat.st_size;
- if (_blockSize) // that shouldn't cause us any problems
- *_blockSize = 512;
-
- return B_OK;
- }
-
- if (_size) {
- *_size = 1LL * geometry.head_count * geometry.cylinder_count
- * geometry.sectors_per_track *
geometry.bytes_per_sector;
- }
- if (_blockSize)
- *_blockSize = geometry.bytes_per_sector;
-
- return B_OK;
-}
-
-
// #pragma mark -
@@ -358,10 +324,11 @@ Volume::Mount(const char* deviceName, uint32 flags)
fAllocationGroupShift = fSuperBlock.AllocationGroupShift();
// check if the device size is large enough to hold the file system
- off_t diskSize;
- if (opener.GetSize(&diskSize, &fDeviceBlockSize) != B_OK)
- RETURN_ERROR(B_ERROR);
- if (diskSize < (NumBlocks() << BlockShift()))
+ status_t status = UpdateDeviceSize();
+ if (status != B_OK)
+ RETURN_ERROR(status);
+
+ if (fDeviceSize < (NumBlocks() << BlockShift()))
RETURN_ERROR(B_BAD_VALUE);
// set the current log pointers, so that journaling will work correctly
@@ -375,7 +342,7 @@ Volume::Mount(const char* deviceName, uint32 flags)
if (fJournal == NULL)
return B_NO_MEMORY;
- status_t status = fJournal->InitCheck();
+ status = fJournal->InitCheck();
if (status < B_OK) {
FATAL(("could not initialize journal: %s!\n",
strerror(status)));
return status;
@@ -470,6 +437,34 @@ Volume::Sync()
}
+/*! Reads the size and block size of the device in bytes. It uses
+ B_GET_GEOMETRY to compute the size, or fstat() if that failed.
+*/
+status_t
+Volume::UpdateDeviceSize()
+{
+ device_geometry geometry;
+ if (ioctl(fDevice, B_GET_GEOMETRY, &geometry) < 0) {
+ // maybe it's just a file
+ struct stat stat;
+ if (fstat(fDevice, &stat) < 0)
+ return B_ERROR;
+
+ fDeviceSize = stat.st_size;
+ fDeviceBlockSize = 512;
+ // that shouldn't cause us any problems
+
+ return B_OK;
+ }
+
+ fDeviceSize = 1LL * geometry.head_count * geometry.cylinder_count
+ * geometry.sectors_per_track * geometry.bytes_per_sector;
+ fDeviceBlockSize = geometry.bytes_per_sector;
+
+ return B_OK;
+}
+
+
status_t
Volume::ValidateBlockRun(block_run run)
{
@@ -747,12 +742,11 @@ Volume::Initialize(int fd, const char* name, uint32
blockSize,
fDevice = opener.Device();
- uint32 deviceBlockSize;
- off_t deviceSize;
- if (opener.GetSize(&deviceSize, &deviceBlockSize) < B_OK)
- return B_ERROR;
+ status_t status = UpdateDeviceSize();
+ if (status != B_OK)
+ return status;
- off_t numBlocks = deviceSize / blockSize;
+ off_t numBlocks = fDeviceSize / blockSize;
// create valid super block
@@ -763,7 +757,7 @@ Volume::Initialize(int fd, const char* name, uint32
blockSize,
fBlockShift = fSuperBlock.BlockShift();
fAllocationGroupShift = fSuperBlock.AllocationGroupShift();
- off_t logSize = CalculateLogSize(numBlocks, deviceSize);
+ off_t logSize = CalculateLogSize(numBlocks, fDeviceSize);
// since the allocator has not been initialized yet, we
// cannot use BlockAllocator::BitmapSize() here
@@ -796,7 +790,7 @@ Volume::Initialize(int fd, const char* name, uint32
blockSize,
RETURN_ERROR(B_ERROR);
off_t id;
- status_t status = Inode::Create(transaction, NULL, NULL,
+ status = Inode::Create(transaction, NULL, NULL,
S_DIRECTORY | 0755, 0, 0, NULL, &id, &fRootNode);
if (status < B_OK)
RETURN_ERROR(status);
diff --git a/src/add-ons/kernel/file_systems/bfs/Volume.h
b/src/add-ons/kernel/file_systems/bfs/Volume.h
index 04cef3e..94972f6 100644
--- a/src/add-ons/kernel/file_systems/bfs/Volume.h
+++ b/src/add-ons/kernel/file_systems/bfs/Volume.h
@@ -71,7 +71,10 @@ public:
{ return
(NumBlocks() + fBlockSize * 8 - 1)
/
(fBlockSize * 8); }
+ status_t UpdateDeviceSize();
+ off_t DeviceSize() const { return
fDeviceSize; }
uint32 DeviceBlockSize() const {
return fDeviceBlockSize; }
+
uint32 BlockSize() const { return
fBlockSize; }
uint32 BlockShift() const { return
fBlockShift; }
uint32 InodeSize() const
@@ -161,6 +164,7 @@ protected:
int fDevice;
disk_super_block fSuperBlock;
+ off_t fDeviceSize;
uint32 fDeviceBlockSize;
uint32 fBlockSize;
uint32 fBlockShift;
############################################################################
Commit: 91a341e38fa50939a45b8ecd1720a549b5a78714
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Fri Jul 20 12:07:05 2012 UTC
Get device size from Volume
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.cpp
b/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.cpp
index 60e7221..8cfef39 100644
--- a/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.cpp
@@ -67,14 +67,13 @@ ResizeVisitor::StartResize()
return;
}
- off_t diskSize;
- status_t status = _TemporaryGetDiskSize(diskSize);
+ status_t status = GetVolume()->UpdateDeviceSize();
if (status != B_OK) {
_SetError(status);
return;
}
- if (diskSize < (fNumBlocks << blockShift)) {
+ if (GetVolume()->DeviceSize() < (fNumBlocks << blockShift)) {
_SetError(B_BAD_VALUE, BFS_DISK_TOO_SMALL);
return;
}
@@ -646,24 +645,3 @@ ResizeVisitor::_SetError(status_t status, uint32
failurePoint)
Control().status = status;
Control().failure_point = failurePoint;
}
-
-
-status_t
-ResizeVisitor::_TemporaryGetDiskSize(off_t& size)
-{
- device_geometry geometry;
- if (ioctl(GetVolume()->Device(), B_GET_GEOMETRY, &geometry) < 0) {
- // maybe it's just a file
- struct stat stat;
- if (fstat(GetVolume()->Device(), &stat) < 0)
- return B_ERROR;
-
- size = stat.st_size;
- return B_OK;
- }
-
- size = 1LL * geometry.head_count * geometry.cylinder_count
- * geometry.sectors_per_track * geometry.bytes_per_sector;
-
- return B_OK;
-}
diff --git a/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.h
b/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.h
index 7a0ea7b..246205f 100644
--- a/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.h
+++ b/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.h
@@ -61,8 +61,6 @@ private:
void _SetError(status_t
status,
uint32
failurePoint = BFS_OTHER_ERROR);
- status_t
_TemporaryGetDiskSize(off_t& size);
-
private:
resize_control fControl;
bool fShrinking;
############################################################################
Commit: 68dfc437c86deca7866d85b80ad932a74a16491d
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Mon Jul 23 12:35:39 2012 UTC
Incorrect checking of already set double indirect blocks
Wrong variable usage in inner loop caused some double indirect stream
runs to be checked twice when block size was smaller than
DOUBLE_INDIRECT_ARRAY_SIZE, while some were incorrectly marked as
unallocated in the bitmap.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/CheckVisitor.cpp
b/src/add-ons/kernel/file_systems/bfs/CheckVisitor.cpp
index 8d835b7..fa2edb5 100644
--- a/src/add-ons/kernel/file_systems/bfs/CheckVisitor.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/CheckVisitor.cpp
@@ -574,7 +574,7 @@ CheckVisitor::_CheckInodeBlocks(Inode* inode, const char*
name)
Control().stats.double_indirect_block_runs++;
Control().stats.blocks_in_double_indirect
+= runs[index %
runsPerBlock].Length();
- } while ((++index % runsPerArray) != 0);
+ } while ((++index % runsPerBlock) != 0);
}
Control().stats.double_indirect_array_blocks++;
############################################################################
Commit: ff678f7b3d570458dc6348b57ad7498afa110016
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Mon Jul 23 14:20:16 2012 UTC
Double indirect blocks were allocated out of range
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
index e21f207..98642bf 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
@@ -2001,7 +2001,7 @@ Inode::_AddBlockRun(Transaction& transaction,
data_stream* dataStream,
if (dataStream->double_indirect.IsZero()) {
status = _AllocateBlockArray(transaction,
dataStream->double_indirect,
_DoubleIndirectBlockLength(),
- beginBlock, endBlock);
+ false, beginBlock, endBlock);
if (status != B_OK)
return status;
@@ -2051,8 +2051,8 @@ Inode::_AddBlockRun(Transaction& transaction,
data_stream* dataStream,
status =
_AllocateBlockArray(transaction,
array[indirectIndex %
runsPerBlock],
-
dataStream->double_indirect.Length(), beginBlock,
- endBlock);
+
dataStream->double_indirect.Length(), false,
+ beginBlock, endBlock);
if (status != B_OK)
return status;
}
############################################################################
Commit: 4cc09a58f2d1364b6e442c17a935a525d82bf1b5
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Mon Jul 23 14:35:51 2012 UTC
Free indirect blocks when moving a file stream
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
index 98642bf..167821d 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
@@ -2765,6 +2765,52 @@ Inode::_WriteBufferedRuns(Transaction& transaction,
BlockRunBuffer& buffer,
}
+status_t
+Inode::_FreeIndirectBlocks(Transaction& transaction, data_stream *dataStream)
+{
+ if (dataStream->indirect.IsZero())
+ return B_OK;
+
+ // free single indirect run
+ status_t status = fVolume->Free(transaction, dataStream->indirect);
+ if (status != B_OK)
+ return status;
+
+ if (dataStream->double_indirect.IsZero())
+ return B_OK;
+
+ int32 runsPerBlock = fVolume->BlockSize() / sizeof(block_run);
+ bool endOfStream = false;
+ CachedBlock cached(fVolume);
+
+ for (uint16 blockIndex = 0;
+ blockIndex < dataStream->double_indirect.Length();
blockIndex++) {
+ // get the double indirect array runs in this block
+ off_t block = fVolume->ToBlock(dataStream->double_indirect)
+ + blockIndex;
+ const block_run* runArray = (const
block_run*)cached.SetTo(block);
+
+ for (int32 i = 0; i < runsPerBlock; i++) {
+ if (runArray[i].IsZero()) {
+ endOfStream = true;
+ break;
+ }
+
+ // free double indirect array run
+ status = fVolume->Free(transaction, runArray[i]);
+ if (status != B_OK)
+ return status;
+ }
+
+ if (endOfStream)
+ break;
+ }
+
+ // free double indirect run
+ return fVolume->Free(transaction, dataStream->double_indirect);
+}
+
+
/*! Get the block run that covers the file position \a position.
\a position is updated to point at the first position covered by
the following run.
@@ -2921,6 +2967,8 @@ Inode::MoveStream(off_t beginBlock, off_t endBlock)
if (status != B_OK)
return status;
}
+
+ _FreeIndirectBlocks(transaction, &fNode.data);
fNode.data = newStream;
status = WriteBack(transaction);
diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.h
b/src/add-ons/kernel/file_systems/bfs/Inode.h
index c76a6d8..15580f8 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.h
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.h
@@ -268,6 +268,8 @@ private:
data_stream* dataStream, off_t targetSize,
bool
flush = false, off_t beginBlock = 0,
off_t
endBlock = 0);
+ status_t
_FreeIndirectBlocks(Transaction& transaction,
+
data_stream *dataStream);
status_t _GetNextBlockRun(off_t&
position,
block_run& run) const;
############################################################################
Commit: 8b54ad6ef747ef971a51e95c855fb6ec6e89e061
Author: ahenriksson <sausageboy@xxxxxxxxx>
Date: Mon Jul 23 15:44:39 2012 UTC
Syntax mistake when locking
Not sure why the compiler considered this valid in the first place, to
be honest.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.cpp
b/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.cpp
index 8cfef39..4156dfc 100644
--- a/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/ResizeVisitor.cpp
@@ -534,7 +534,7 @@ ResizeVisitor::_UpdateChildren(Transaction& transaction,
Inode* inode,
status_t
ResizeVisitor::_UpdateSuperBlock(Inode* inode, off_t newInodeID)
{
- MutexLocker(GetVolume()->Lock());
+ MutexLocker locker(GetVolume()->Lock());
disk_super_block& superBlock = GetVolume()->SuperBlock();
if (inode->BlockRun() == superBlock.root_dir) {
Other related posts:
