Fixed two bugs in the block allocator:

- the maximum number of blocks a block_run can cover is 65535 not 65536
- allocations over a block boundary in the block bitmap always failed due
  to a incorrectly handled status

Both bugs could prevent BFS to create the VM swap file which led BeOS to
stop booting under certain circumstances.


git-svn-id: file:///srv/svn/repos/haiku/trunk/current@989 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/add-ons/kernel/file_systems/bfs/BlockAllocator.cpp

@@ -162,8 +162,8 @@ AllocationGroup::AllocationGroup()
 void 
 AllocationGroup::AddFreeRange(int32 start, int32 blocks)
 {
-	D(if (blocks > 512)
+	//D(if (blocks > 512)
-		PRINT(("range of %ld blocks starting at %ld\n",blocks,start)));
+	//	PRINT(("range of %ld blocks starting at %ld\n",blocks,start)));
 
 	if (fFirstFree == -1)
 		fFirstFree = start;
@@ -205,7 +205,7 @@ BlockAllocator::Initialize()
 	fGroups = new AllocationGroup[fNumGroups];
 	if (fGroups == NULL)
 		return B_NO_MEMORY;
-	
+
 	thread_id id = spawn_kernel_thread((thread_func)BlockAllocator::initialize,"bfs block allocator",B_LOW_PRIORITY,(void *)this);
 	if (id < B_OK)
 		return initialize(this);
@@ -279,6 +279,9 @@ BlockAllocator::initialize(BlockAllocator *allocator)
 status_t
 BlockAllocator::AllocateBlocks(Transaction *transaction,int32 group,uint16 start,uint16 maximum,uint16 minimum, block_run &run)
 {
+	if (maximum == 0)
+		return B_BAD_VALUE;
+
 	AllocationBlock cached(fVolume);
 	Locker lock(fLock);
 
@@ -319,7 +322,7 @@ BlockAllocator::AllocateBlocks(Transaction *transaction,int32 group,uint16 start
 		int32 range = 0, rangeStart = 0,rangeBlock = 0;
 
 		for (;block < fBlocksPerGroup;block++) {
-			if (cached.SetTo(fGroups[group],block) < B_OK)
+			if (cached.SetTo(fGroups[group], block) < B_OK)
 				RETURN_ERROR(B_ERROR);
 
 			// find a block large enough to hold the allocation
@@ -358,15 +361,15 @@ BlockAllocator::AllocateBlocks(Transaction *transaction,int32 group,uint16 start
 
 				if (block != rangeBlock) {
 					// allocate the part that's in the current block
-					cached.Allocate(0,(rangeStart + numBlocks) % cached.NumBlockBits());
+					cached.Allocate(0, (rangeStart + numBlocks) % cached.NumBlockBits());
 					if (cached.WriteBack(transaction) < B_OK)
 						RETURN_ERROR(B_ERROR);
 
 					// set the blocks in the previous block
-					if (cached.SetTo(fGroups[group],block - 1) < B_OK)
+					if (cached.SetTo(fGroups[group], block - 1) < B_OK)
-						cached.Allocate(rangeStart);
-					else
 						RETURN_ERROR(B_ERROR);
+
+					cached.Allocate(rangeStart);
 				} else {
 					// just allocate the bits in the current block
 					cached.Allocate(rangeStart,numBlocks);
@@ -420,12 +423,24 @@ BlockAllocator::Allocate(Transaction *transaction,const Inode *inode, off_t numB
 	if (numBlocks > fGroups[0].fNumBits)
 		numBlocks = fGroups[0].fNumBits;
 
+	// since block_run.length is uint16, the largest number of blocks that
+	// can be covered by a block_run is 65535
+	// ToDo: if we drop compatibility, couldn't we do this any better?
+	// There are basically two possibilities:
+	// a) since a length of zero doesn't have any sense, take that for 65536 -
+	//    but that could cause many problems (bugs) in other areas
+	// b) reduce the maximum amount of blocks per block_run, so that the remaining
+	//    number of free blocks can be used in a useful manner (like 4 blocks) -
+	//    but that would also reduce the maximum file size
+	if (numBlocks == 65536)
+		numBlocks = 65535;
+
 	// apply some allocation policies here (AllocateBlocks() will break them
 	// if necessary)
 	uint16 group = inode->BlockRun().allocation_group;
 	uint16 start = 0;
 
-	// are there already allocated blocks? (then just allocate near the last)
+	// are there already allocated blocks? (then just try to allocate near the last one)
 	if (inode->Size() > 0) {
 		data_stream *data = &inode->Node()->data;
 		// we currently don't care for when the data stream is
@@ -449,7 +464,7 @@ BlockAllocator::Allocate(Transaction *transaction,const Inode *inode, off_t numB
 		group = inode->BlockRun().allocation_group + 1;
 	}
 
-	return AllocateBlocks(transaction,group,start,numBlocks,minimum,run);
+	return AllocateBlocks(transaction, group, start, numBlocks, minimum, run);
 }
 
 

src/add-ons/kernel/file_systems/bfs/Inode.h

@@ -84,7 +84,7 @@ class CachedBlock {
 				release_block(fVolume->Device(),fBlockNumber);
 		}
 
-		uint8 *SetTo(off_t block,bool empty = false)
+		uint8 *SetTo(off_t block, bool empty = false)
 		{
 			Unset();
 			fBlockNumber = block;
@@ -92,7 +92,7 @@ class CachedBlock {
 								  : (uint8 *)get_block(fVolume->Device(),block,fVolume->BlockSize());
 		}
 
-		uint8 *SetTo(block_run run,bool empty = false)
+		uint8 *SetTo(block_run run, bool empty = false)
 		{
 			return SetTo(fVolume->ToBlock(run),empty);
 		}

src/add-ons/kernel/file_systems/bfs/ToDo

@@ -1,4 +1,4 @@
-BFS - ToDo, August 12th, 2002
+BFS - ToDo, September 7th, 2002
 -----
 
 BlockAllocator
@@ -11,9 +11,8 @@ BlockAllocator
 
 DataStream
 
- - growing/shrinking the stream size is not implemented for the double indirect range
  - only files are trimmed back (in bfs_close()), but every inode has a preallocated stream...
- - merging of block_runs doesn't work between range/block boundaries
+ - Inode::GrowStream(): merging of block_runs doesn't work between range/block boundaries
 
 
 Queries