Fixed another bad bug caused by calling get_vnode() on a node currently

being constructed: bfs_read_node() created an Inode object independent from
the one set in new_vnode().
As a work-around (the new VFS layer will introduce some better methods here)
we are now using the bfs_inode.etc field as a pointer to our object - just
like BFS has thought to use this field. If bfs_read_vnode() has to wait for
the inode (because it is being constructed), it will use the "etc" pointer
and don't create it's own Inode object.
Almost all changes made change the API to take this case into account:
- new Inode constructor to be able to be created from a CachedBlock
- CachedBlock now has a Keep() method which suppresses the release_block()
  call when the object is destructed.
- a CachedBlock can now be constructed from the contents of another one (by
  calling the source's Keep() method).
- Inode::InitCheck() no longer checks the integrity of the bfs_inode - this
  is now done by bfs_inode::InitCheck() which is optionally called by the
  former (default).
- moved the inline CachedBlock methods out of the class definition (was
  too crowded to be readable).
- new Inode::Initialize() which is called by all Inode constructors
- an Inode object now sets a better name for its read/write lock.


git-svn-id: file:///srv/svn/repos/haiku/trunk/current@3364 a95241bf-73f2-0310-859d-f6bbb57e9c96
This commit is contained in:
Axel Dörfler 2003-05-28 01:27:27 +00:00
parent 1d9fd63d3a
commit ae58716024
4 changed files with 241 additions and 120 deletions

View File

@ -13,6 +13,7 @@
#include "Index.h"
#include <string.h>
#include <stdio.h>
class InodeAllocator {
@ -81,6 +82,9 @@ InodeAllocator::New(block_run *parentRun, mode_t mode, block_run &run, Inode **_
node->flags = INODE_IN_USE | INODE_NOT_READY;
// INODE_NOT_READY prevents the inode from being opened - it is
// cleared in InodeAllocator::Keep()
node->etc = (uint32)fInode;
// this is temporarily set along INODE_NOT_READY and lets bfs_read_vnode()
// find the associated Inode object
node->create_time = (bigtime_t)time(NULL) << INODE_TIME_SHIFT;
node->last_modified_time = node->create_time | (volume->GetUniqueID() & INODE_TIME_MASK);
@ -134,16 +138,55 @@ InodeAllocator::Keep()
// #pragma mark -
status_t
bfs_inode::InitCheck(Volume *volume)
{
if (flags & INODE_NOT_READY) {
// the other fields may not yet contain valid values
return B_BUSY;
}
if (magic1 != INODE_MAGIC1
|| !(flags & INODE_IN_USE)
|| inode_num.length != 1
// matches inode size?
|| (uint32)inode_size != volume->InodeSize()
// parent resides on disk?
|| parent.allocation_group > int32(volume->AllocationGroups())
|| parent.allocation_group < 0
|| parent.start > (1L << volume->AllocationGroupShift())
|| parent.length != 1
// attributes, too?
|| attributes.allocation_group > int32(volume->AllocationGroups())
|| attributes.allocation_group < 0
|| attributes.start > (1L << volume->AllocationGroupShift()))
RETURN_ERROR(B_BAD_DATA);
// ToDo: Add some tests to check the integrity of the other stuff here,
// especially for the data_stream!
return B_OK;
}
// #pragma mark -
Inode::Inode(Volume *volume, vnode_id id, bool empty, uint8 reenter)
: CachedBlock(volume, volume->VnodeToBlock(id), empty),
fTree(NULL),
fLock("bfs inode")
fLock()
{
Node()->flags &= INODE_PERMANENT_FLAGS;
Initialize();
}
// these two will help to maintain the indices
fOldSize = Size();
fOldLastModified = LastModified();
Inode::Inode(CachedBlock *cached)
: CachedBlock(cached),
fTree(NULL),
fLock()
{
Initialize();
}
@ -153,37 +196,39 @@ Inode::~Inode()
}
void
Inode::Initialize()
{
char lockName[32];
sprintf(lockName, "bfs inode %ld.%d", BlockRun().allocation_group, BlockRun().start);
fLock.Initialize(lockName);
Node()->flags &= INODE_PERMANENT_FLAGS;
// these two will help to maintain the indices
fOldSize = Size();
fOldLastModified = LastModified();
}
status_t
Inode::InitCheck()
Inode::InitCheck(bool checkNode)
{
if (!Node())
RETURN_ERROR(B_IO_ERROR);
// test inode magic and flags
if (Node()->magic1 != INODE_MAGIC1
|| !(Node()->flags & INODE_IN_USE)
|| Node()->inode_num.length != 1
// matches inode size?
|| (uint32)Node()->inode_size != fVolume->InodeSize()
// parent resides on disk?
|| Node()->parent.allocation_group > int32(fVolume->AllocationGroups())
|| Node()->parent.allocation_group < 0
|| Node()->parent.start > (1L << fVolume->AllocationGroupShift())
|| Node()->parent.length != 1
// attributes, too?
|| Node()->attributes.allocation_group > int32(fVolume->AllocationGroups())
|| Node()->attributes.allocation_group < 0
|| Node()->attributes.start > (1L << fVolume->AllocationGroupShift())) {
FATAL(("inode at block %Ld corrupt!\n", fBlockNumber));
RETURN_ERROR(B_BAD_DATA);
if (checkNode) {
status_t status = Node()->InitCheck(fVolume);
if (status == B_BUSY)
return B_BUSY;
if (status < B_OK) {
FATAL(("inode at block %Ld corrupt!\n", fBlockNumber));
RETURN_ERROR(B_BAD_DATA);
}
}
if (Flags() & INODE_NOT_READY)
return B_BUSY;
// ToDo: Add some tests to check the integrity of the other stuff here,
// especially for the data_stream!
// it's more important to know that the inode is corrupt
// so we check for the lock not until here
return fLock.InitCheck();

View File

@ -55,60 +55,17 @@ enum inode_type {
class CachedBlock {
public:
CachedBlock(Volume *volume)
:
fVolume(volume),
fBlock(NULL)
{
}
CachedBlock(Volume *volume);
CachedBlock(Volume *volume, off_t block, bool empty = false);
CachedBlock(Volume *volume, block_run run, bool empty = false);
CachedBlock(CachedBlock *cached);
~CachedBlock();
CachedBlock(Volume *volume, off_t block, bool empty = false)
:
fVolume(volume),
fBlock(NULL)
{
SetTo(block, empty);
}
CachedBlock(Volume *volume, block_run run, bool empty = false)
:
fVolume(volume),
fBlock(NULL)
{
SetTo(volume->ToBlock(run), empty);
}
~CachedBlock()
{
Unset();
}
void Unset()
{
if (fBlock != NULL)
release_block(fVolume->Device(), fBlockNumber);
}
uint8 *SetTo(off_t block, bool empty = false)
{
Unset();
fBlockNumber = block;
return fBlock = empty ? (uint8 *)get_empty_block(fVolume->Device(), block, BlockSize())
: (uint8 *)get_block(fVolume->Device(), block, BlockSize());
}
uint8 *SetTo(block_run run, bool empty = false)
{
return SetTo(fVolume->ToBlock(run), empty);
}
status_t WriteBack(Transaction *transaction)
{
if (transaction == NULL || fBlock == NULL)
RETURN_ERROR(B_BAD_VALUE);
return transaction->WriteBlocks(fBlockNumber, fBlock);
}
inline void Keep();
inline void Unset();
inline uint8 *SetTo(off_t block, bool empty = false);
inline uint8 *SetTo(block_run run, bool empty = false);
inline status_t WriteBack(Transaction *transaction);
uint8 *Block() const { return fBlock; }
off_t BlockNumber() const { return fBlockNumber; }
@ -126,10 +83,12 @@ class CachedBlock {
uint8 *fBlock;
};
//--------------------------------------
class Inode : public CachedBlock {
public:
Inode(Volume *volume,vnode_id id,bool empty = false,uint8 reenter = 0);
Inode(Volume *volume, vnode_id id, bool empty = false, uint8 reenter = 0);
Inode(CachedBlock *cached);
~Inode();
bfs_inode *Node() const { return (bfs_inode *)fBlock; }
@ -162,7 +121,7 @@ class Inode : public CachedBlock {
block_run &Attributes() const { return Node()->attributes; }
Volume *GetVolume() const { return fVolume; }
status_t InitCheck();
status_t InitCheck(bool checkNode = true);
status_t CheckPermissions(int accessMode) const;
@ -225,6 +184,8 @@ class Inode : public CachedBlock {
friend AttributeIterator;
friend InodeAllocator;
void Initialize();
status_t RemoveSmallData(small_data *item, int32 index);
void AddIterator(AttributeIterator *iterator);
@ -321,6 +282,99 @@ class AttributeIterator {
};
//--------------------------------------
// inlines
inline
CachedBlock::CachedBlock(Volume *volume)
:
fVolume(volume),
fBlock(NULL)
{
}
inline
CachedBlock::CachedBlock(Volume *volume, off_t block, bool empty = false)
:
fVolume(volume),
fBlock(NULL)
{
SetTo(block, empty);
}
inline
CachedBlock::CachedBlock(Volume *volume, block_run run, bool empty = false)
:
fVolume(volume),
fBlock(NULL)
{
SetTo(volume->ToBlock(run), empty);
}
inline
CachedBlock::CachedBlock(CachedBlock *cached)
:
fVolume(cached->fVolume),
fBlock(cached->fBlock),
fBlockNumber(cached->BlockNumber())
{
cached->Keep();
}
inline
CachedBlock::~CachedBlock()
{
Unset();
}
inline void
CachedBlock::Keep()
{
fBlock = NULL;
}
inline void
CachedBlock::Unset()
{
if (fBlock != NULL)
release_block(fVolume->Device(), fBlockNumber);
}
inline uint8 *
CachedBlock::SetTo(off_t block, bool empty = false)
{
Unset();
fBlockNumber = block;
return fBlock = empty ? (uint8 *)get_empty_block(fVolume->Device(), block, BlockSize())
: (uint8 *)get_block(fVolume->Device(), block, BlockSize());
}
inline uint8 *
CachedBlock::SetTo(block_run run, bool empty = false)
{
return SetTo(fVolume->ToBlock(run), empty);
}
inline status_t
CachedBlock::WriteBack(Transaction *transaction)
{
if (transaction == NULL || fBlock == NULL)
RETURN_ERROR(B_BAD_VALUE);
return transaction->WriteBlocks(fBlockNumber, fBlock);
}
/** Converts the "omode", the open flags given to bfs_open(), into
* access modes, e.g. since O_RDONLY requires read access to the
* file, it will be converted to R_OK.

View File

@ -17,8 +17,7 @@
#endif
struct block_run
{
struct block_run {
int32 allocation_group;
uint16 start;
uint16 length;
@ -45,8 +44,7 @@ typedef block_run inode_addr;
#define BFS_DISK_NAME_LENGTH 32
struct disk_super_block
{
struct disk_super_block {
char name[BFS_DISK_NAME_LENGTH];
int32 magic1;
int32 fs_byte_order;
@ -82,8 +80,7 @@ struct disk_super_block
#define NUM_DIRECT_BLOCKS 12
struct data_stream
{
struct data_stream {
block_run direct[NUM_DIRECT_BLOCKS];
off_t max_direct_range;
block_run indirect;
@ -104,8 +101,7 @@ struct data_stream
struct bfs_inode;
struct small_data
{
struct small_data {
uint32 type;
uint16 name_size;
uint16 data_size;
@ -123,12 +119,14 @@ struct small_data
#define FILE_NAME_NAME 0x13
#define FILE_NAME_NAME_LENGTH 1
//**************************************
class Volume;
#define SHORT_SYMLINK_NAME_LENGTH 144 // length incl. terminating '\0'
struct bfs_inode
{
struct bfs_inode {
int32 magic1;
inode_addr inode_num;
int32 uid;
@ -142,7 +140,7 @@ struct bfs_inode
uint32 type; // attribute type
int32 inode_size;
uint32 etc; // for in-memory structures (unused in OpenBeOS' fs)
uint32 etc; // a pointer to the Inode object during construction
union {
data_stream data;
@ -150,6 +148,9 @@ struct bfs_inode
};
int32 pad[4];
small_data small_data_start[0];
status_t InitCheck(Volume *volume);
// defined in Inode.cpp
};
#define INODE_MAGIC1 0x3bbe0ad9
@ -157,8 +158,7 @@ struct bfs_inode
#define INODE_TIME_MASK 0xffff
#define INODE_FILE_NAME_LENGTH 256
enum inode_flags
{
enum inode_flags {
INODE_IN_USE = 0x00000001, // always set
INODE_ATTR_INODE = 0x00000004,
INODE_LOGGED = 0x00000008, // log changes to the data stream
@ -324,4 +324,5 @@ small_data::IsLast(bfs_inode *inode)
return (uint32)this > (uint32)inode + inode->inode_size - sizeof(small_data) || name_size == 0;
}
#endif /* BFS_H */

View File

@ -343,10 +343,11 @@ bfs_sync(void *_ns)
// #pragma mark -
/** Using vnode id, read in vnode information into fs-specific struct,
* and return it in node. the reenter flag tells you if this function
* is being called via some other fs routine, so that things like
* double-locking can be avoided.
/** Reads in the node from disk and creates an inode object from it.
* Has to be cautious if the node in question is currently under
* construction, in which case it waits for that action to be completed,
* and uses the inode object from the construction instead of creating
* a new one.
*/
static int
@ -360,35 +361,55 @@ bfs_read_vnode(void *_ns, vnode_id id, char reenter, void **_node)
return B_ERROR;
}
Inode *inode = new Inode(volume, id, false, reenter);
if (inode == NULL)
return B_NO_MEMORY;
CachedBlock cached(volume, id);
status_t status;
bfs_inode *node = (bfs_inode *)cached.Block();
Inode *inode = NULL;
int32 tries = 0;
while (true) {
if ((status = inode->InitCheck()) == B_OK) {
// the inode is okay and ready to be used
*_node = (void *)inode;
return B_OK;
}
restartIfBusy:
status_t status = node->InitCheck(volume);
// if "status" is B_BUSY, we wait a bit and try again
if (status == B_BUSY) {
inode = (Inode *)node->etc;
// We have to use the "etc" field to get the inode object
// (the inode is currently being constructed)
// We would need to call something like get_vnode() again here
// to get rid of the "etc" field - which would be nice, especially
// for other file systems which don't have this messy field.
if (status == B_BUSY) {
// let us wait a bit and try again later
if (tries++ < 200) {
// wait for one second at maximum
if (tries++ < 200) {
snooze(5000);
continue;
}
FATAL(("inode is not becoming unbusy (id = %Ld)\n", id));
snooze(5000);
goto restartIfBusy;
}
break;
FATAL(("inode is not becoming unbusy (id = %Ld)\n", id));
return status;
} else if (status < B_OK) {
FATAL(("inode at %Ld is corrupt!\n", id));
return status;
}
delete inode;
RETURN_ERROR(status);
// If the inode is currently being constructed, we already have an inode
// pointer (taken from the node's etc field).
// If not, we create a new one here
if (inode == NULL) {
inode = new Inode(&cached);
if (inode == NULL)
return B_NO_MEMORY;
status = inode->InitCheck(false);
if (status < B_OK)
delete inode;
} else
status = inode->InitCheck(false);
if (status == B_OK)
*_node = inode;
return status;
}