Kernel/VFS - Fixed mount reference counting and shutdown cleanup
authorJohn Hodge <[email protected]>
Thu, 23 Aug 2012 05:17:53 +0000 (13:17 +0800)
committerJohn Hodge <[email protected]>
Thu, 23 Aug 2012 05:17:53 +0000 (13:17 +0800)
- VFS SysFS handles will be removed when / is unmounted
- Mountpoint reference counting fixed such that unmount can work

KernelLand/Kernel/debug.c
KernelLand/Kernel/drv/proc.c
KernelLand/Kernel/include/vfs_int.h
KernelLand/Kernel/syscalls.c
KernelLand/Kernel/vfs/dir.c
KernelLand/Kernel/vfs/handle.c
KernelLand/Kernel/vfs/main.c
KernelLand/Kernel/vfs/mount.c
KernelLand/Kernel/vfs/open.c

index dab20ad..31d3b39 100644 (file)
@@ -221,6 +221,7 @@ void Panic(const char *Fmt, ...)
        Debug_Putchar('\n');
 
        Threads_Dump();
+       Heap_Dump();
 
        for(;;) ;
 }
index 1e0c6bd..9c7d909 100644 (file)
@@ -276,14 +276,15 @@ int SysFS_RemoveFile(int ID)
        tSysFS_Ent      *ent, *parent, *prev;
        
        prev = NULL;
-       for( ent = gSysFS_FileList; ent; prev = ent, ent = ent->Next )
+       for( ent = gSysFS_FileList; ent; prev = ent, ent = ent->ListNext )
        {
                // It's a reverse sorted list
-               if(ent->Node.Inode < (Uint64)ID)        return 0;
-               if(ent->Node.Inode == (Uint64)ID)       break;
+               if(ent->Node.Inode <= (Uint64)ID)       break;
+       }
+       if( !ent || ent->Node.Inode != (Uint64)ID) {
+               Log_Notice("SysFS", "ID %i not present", ID);
+               return 0;
        }
-       
-       if(!ent)        return 0;
        
        // Set up for next part
        file = ent;
@@ -297,26 +298,37 @@ int SysFS_RemoveFile(int ID)
        file->Node.Size = 0;
        file->Node.ImplPtr = NULL;
        
-       // Search parent directory
-       for( ent = parent->Node.ImplPtr; ent; prev = ent, ent = ent->Next )
+       // Clean out of parent directory
+       while(parent)
        {
-               if( ent == file )       break;
-       }
-       if(!ent) {
-               Log_Warning("SysFS", "Bookkeeping Error: File in list, but not in directory");
-               return 0;
+               for( ent = parent->Node.ImplPtr; ent; prev = ent, ent = ent->Next )
+               {
+                       if( ent == file )       break;
+               }
+               if(!ent) {
+                       Log_Warning("SysFS", "Bookkeeping Error: File in list, but not in directory");
+                       return 0;
+               }
+               
+               // Remove from parent directory
+               if(prev)
+                       prev->Next = ent->Next;
+               else
+                       parent->Node.ImplPtr = ent->Next;
+
+               // Free if not in use
+               if(file->Node.ReferenceCount == 0) {
+                       free(file);
+               }
+
+               if( parent->Node.ImplPtr )
+                       break;
+
+               // Remove parent from the tree
+               file = parent;
+               parent = parent->Parent;
        }
        
-       // Remove from parent directory
-       if(prev)
-               prev->Next = ent->Next;
-       else
-               parent->Node.ImplPtr = ent->Next;
-       
-       // Free if not in use
-       if(file->Node.ReferenceCount == 0)
-               free(file);
-       
        return 1;
 }
 
index 1f67f4b..232c55e 100644 (file)
@@ -51,6 +51,7 @@ extern tVFS_Mount     *gVFS_Mounts;
 extern tVFS_Driver     *gVFS_Drivers;
 
 // === PROTOTYPES ===
+extern void    VFS_Deinit(void);
 // --- open.c ---
 extern char    *VFS_GetAbsPath(const char *Path);
 extern tVFS_Node       *VFS_ParsePath(const char *Path, char **TruePath, tVFS_Mount **MountPoint);
index d629ae4..0348d57 100644 (file)
@@ -275,21 +275,35 @@ void SyscallHandler(tSyscallRegs *Regs)
                        ret = -1;
                        break;
                }
-               // Sanity check the paths
-               if(!Syscall_ValidString((char*)Regs->Arg1)
-               || !Syscall_ValidString((char*)Regs->Arg2)
-               || !Syscall_ValidString((char*)Regs->Arg3)
-               || !Syscall_ValidString((char*)Regs->Arg4) ) {
-                       err = -EINVAL;
-                       ret = -1;
-                       break;
+               
+               if( !Regs->Arg1 )
+               {
+                       if( !Syscall_ValidString((char*)Regs->Arg2) ) {
+                               err = -EINVAL;
+                               ret = -1;
+                               break;
+                       }
+                       
+                       ret = VFS_Unmount((char*)Regs->Arg2);
+               }
+               else
+               {
+                       // Sanity check the paths
+                       if(!Syscall_ValidString((char*)Regs->Arg1)
+                       || !Syscall_ValidString((char*)Regs->Arg2)
+                       || (Regs->Arg3 && !Syscall_ValidString((char*)Regs->Arg3))
+                       || !Syscall_ValidString((char*)Regs->Arg4) ) {
+                               err = -EINVAL;
+                               ret = -1;
+                               break;
+                       }
+                       ret = VFS_Mount(
+                               (char*)Regs->Arg1,      // Device
+                               (char*)Regs->Arg2,      // Mount point
+                               (char*)Regs->Arg3,      // Filesystem
+                               (char*)Regs->Arg4       // Options
+                               );
                }
-               ret = VFS_Mount(
-                       (char*)Regs->Arg1,      // Device
-                       (char*)Regs->Arg2,      // Mount point
-                       (char*)Regs->Arg3,      // Filesystem
-                       (char*)Regs->Arg4       // Options
-                       );
                break;
                
        // Wait on a set of handles
index fab9d56..cd45121 100644 (file)
@@ -2,6 +2,7 @@
  * Acess2 VFS
  * - Directory Management Functions
  */
+#define SANITY 1
 #define DEBUG  0
 #include <acess.h>
 #include <vfs.h>
@@ -68,26 +69,22 @@ int VFS_MkNod(const char *Path, Uint Flags)
        LOG("parent = %p", parent);
        
        if(!parent) {
-               LEAVE('i', -1);
-               return -1;      // Error Check
+               errno = ENOENT;
+               goto _error;
        }
 
        // Permissions Check
        if( !VFS_CheckACL(parent, VFS_PERM_EXECUTE|VFS_PERM_WRITE) ) {
-               _CloseNode(parent);
-               mountpt->OpenHandleCount --;
-               free(absPath);
-               LEAVE('i', -1);
-               return -1;
+               errno = EACCES;
+               goto _error;
        }
        
        LOG("parent = %p", parent);
        
        if(!parent->Type || !parent->Type->MkNod) {
                Log_Warning("VFS", "VFS_MkNod - Directory has no MkNod method");
-               mountpt->OpenHandleCount --;
-               LEAVE('i', -1);
-               return -1;
+               errno = ENOTDIR;
+               goto _error;
        }
        
        // Create node
@@ -98,12 +95,21 @@ int VFS_MkNod(const char *Path, Uint Flags)
        free(absPath);
        
        // Free Parent
+       ASSERT(mountpt->OpenHandleCount>0);
        mountpt->OpenHandleCount --;
        _CloseNode(parent);
 
        // Return whatever the driver said      
        LEAVE('i', ret==NULL);
        return ret==NULL;
+
+_error:
+       _CloseNode(parent);
+       ASSERT(mountpt->OpenHandleCount>0);
+       mountpt->OpenHandleCount --;
+       free(absPath);
+       LEAVE('i', -1);
+       return -1;
 }
 
 /**
@@ -123,6 +129,7 @@ int VFS_Symlink(const char *Name, const char *Link)
        realLink = VFS_GetAbsPath( Link );
        if(!realLink) {
                Log_Warning("VFS", "Path '%s' is badly formed", Link);
+               errno = EINVAL;
                LEAVE('i', -1);
                return -1;
        }
@@ -133,12 +140,19 @@ int VFS_Symlink(const char *Name, const char *Link)
        if( VFS_MkNod(Name, VFS_FFLAG_SYMLINK) != 0 ) {
                Log_Warning("VFS", "Unable to create link node '%s'", Name);
                free(realLink);
+               // errno is set by VFS_MkNod
                LEAVE('i', -2);
                return -2;      // Make link node
        }
        
        // Write link address
        fp = VFS_Open(Name, VFS_OPENFLAG_WRITE|VFS_OPENFLAG_NOLINK);
+       if( fp == -1 ) {
+               Log_Warning("VFS", "Unable to open newly created symlink '%s'", Name);
+               free(realLink);
+               LEAVE('i', -3);
+               return -3;
+       }
        VFS_Write(fp, strlen(realLink), realLink);
        VFS_Close(fp);
        
index 8cae804..88877ab 100644 (file)
@@ -2,6 +2,7 @@
  * Acess2 VFS
  * - AllocHandle, GetHandle
  */
+#define SANITY 1
 #define DEBUG  0
 #include <acess.h>
 #include <mm_virt.h>
@@ -25,6 +26,19 @@ tVFS_Handle  *gaUserHandles = (void*)MM_PPD_HANDLES;
 tVFS_Handle    *gaKernelHandles = (void*)MM_KERNEL_VFS;
 
 // === CODE ===
+inline void _ReferenceNode(tVFS_Node *Node)
+{
+       if( !MM_GetPhysAddr(Node->Type) ) {
+               Log_Error("VFS", "Node %p's type is invalid (%p bad pointer) - %P corrupted",
+                       Node, Node->Type, MM_GetPhysAddr(&Node->Type));
+               return ;
+       }
+       if( Node->Type && Node->Type->Reference )
+               Node->Type->Reference( Node );
+       else
+               Node->ReferenceCount ++;
+}
+
 /**
  * \fn tVFS_Handle *VFS_GetHandle(int FD)
  * \brief Gets a pointer to the handle information structure
@@ -130,8 +144,8 @@ void VFS_ReferenceUserHandles(void)
                h = &gaUserHandles[i];
                if( !h->Node )
                        continue ;
-               if( h->Node->Type && h->Node->Type->Reference )
-                       h->Node->Type->Reference( h->Node );
+               _ReferenceNode(h->Node);
+               h->Mount->OpenHandleCount ++;
        }
 }
 
@@ -150,8 +164,7 @@ void VFS_CloseAllUserHandles(void)
                h = &gaUserHandles[i];
                if( !h->Node )
                        continue ;
-               if( h->Node->Type && h->Node->Type->Close )
-                       h->Node->Type->Close( h->Node );
+               _CloseNode(h->Node);
        }
 }
 
@@ -196,8 +209,7 @@ void *VFS_SaveHandles(int NumFDs, int *FDs)
                // Reference node
                if( !h->Node )
                        continue ;
-               if( h->Node->Type && h->Node->Type->Reference )
-                       h->Node->Type->Reference( h->Node );
+               _ReferenceNode(h->Node);
                h->Mount->OpenHandleCount ++;
        }       
 
@@ -243,8 +255,7 @@ void VFS_RestoreHandles(int NumFDs, void *Handles)
        
                if( !h->Node )
                        continue ;
-               if( h->Node->Type && h->Node->Type->Reference )
-                       h->Node->Type->Reference( h->Node );
+               _ReferenceNode(h->Node);
                h->Mount->OpenHandleCount ++;
        }
 }
@@ -265,8 +276,10 @@ void VFS_FreeSavedHandles(int NumFDs, void *Handles)
        
                if( !h->Node )
                        continue ;
-               if( h->Node->Type && h->Node->Type->Close )
-                       h->Node->Type->Close( h->Node );
+               _CloseNode(h->Node);
+               
+               ASSERT(h->Mount->OpenHandleCount > 0);
+               LOG("dec. mntpt '%s' to %i", h->Mount->MountPoint, h->Mount->OpenHandleCount-1);
                h->Mount->OpenHandleCount --;
        }
        free( Handles );
index 38f7c80..fb1da89 100644 (file)
@@ -65,6 +65,14 @@ int VFS_Init(void)
        return 0;
 }
 
+void VFS_Deinit(void)
+{
+       SysFS_RemoveFile(giVFS_MountFileID);
+       free(gsVFS_MountFile);
+       SysFS_RemoveFile(giVFS_DriverFileID);
+       free(gsVFS_DriverFile);
+}
+
 /**
  * \fn char *VFS_GetTruePath(const char *Path)
  * \brief Gets the true path (non-symlink) of a file
index 1832ac1..1823857 100644 (file)
@@ -1,6 +1,7 @@
 /* 
  * Acess Micro - VFS Server version 1
  */
+#define SANITY 1
 #define DEBUG  0
 #include <acess.h>
 #include <vfs.h>
@@ -104,6 +105,7 @@ int VFS_Mount(const char *Device, const char *MountPoint, const char *Filesystem
        // Create mount information
        mnt = malloc( sizeof(tVFS_Mount)+deviceLen+1+mountLen+1+argLen+1 );
        if(!mnt) {
+               ASSERT(parent_mnt->OpenHandleCount > 0);
                parent_mnt->OpenHandleCount --;
                return -2;
        }
@@ -148,6 +150,7 @@ int VFS_Mount(const char *Device, const char *MountPoint, const char *Filesystem
        mnt->RootNode = fs->InitDevice(Device, (const char **)args);
        if(!mnt->RootNode) {
                free(mnt);
+               ASSERT(parent_mnt->OpenHandleCount>0);
                parent_mnt->OpenHandleCount --;
                return -2;
        }
@@ -204,7 +207,8 @@ void VFS_int_Unmount(tVFS_Mount *Mount)
                        break;
                }
                if(mpmnt) {
-                       mpmnt->OpenHandleCount -= 1;
+                       ASSERT(mpmnt->OpenHandleCount>0);
+                       mpmnt->OpenHandleCount --;
                }
                else {
                        Log_Notice("VFS", "Mountpoint '%s' has no parent", Mount->MountPoint);
@@ -227,6 +231,8 @@ int VFS_Unmount(const char *Mountpoint)
                        if( mount->OpenHandleCount ) {
                                LOG("Mountpoint busy");
                                RWLock_Release(&glVFS_MountList);
+                               Log_Log("VFS", "Unmount of '%s' deferred, still busy (%i open handles)",
+                                       Mountpoint, mount->OpenHandleCount);
                                return EBUSY;
                        }
                        if(prev)
@@ -258,12 +264,18 @@ int VFS_UnmountAll(void)
        // If we've unmounted the final filesystem, all good
        if( gVFS_Mounts == NULL) {
                RWLock_Release( &glVFS_MountList );
+               
+               // Final unmount means VFS completely deinited
+               VFS_Deinit();
                return -1;
        }
 
        for( mount = gVFS_Mounts; mount; prev = mount, mount = next )
        {
                next = mount->Next;
+               
+               ASSERT(mount->OpenHandleCount >= 0);            
+
                // Can't unmount stuff with open handles
                if( mount->OpenHandleCount > 0 ) {
                        LOG("%p (%s) has open handles (%i of them)",
index 5ea67c1..e83ace8 100644 (file)
@@ -2,6 +2,7 @@
  * Acess2 VFS
  * - Open, Close and ChDir
  */
+#define SANITY 1
 #define DEBUG  0
 #include <acess.h>
 #include "vfs.h"
@@ -21,9 +22,22 @@ extern int   VFS_AllocHandle(int bIsUser, tVFS_Node *Node, int Mode);
 extern tVFS_Node       *VFS_MemFile_Create(const char *Path);
 
 // === PROTOTYPES ===
+void   _ReferenceMount(tVFS_Mount *Mount, const char *DebugTag);
+void   _DereferenceMount(tVFS_Mount *Mount, const char *DebugTag);
  int   VFS_int_CreateHandle( tVFS_Node *Node, tVFS_Mount *Mount, int Mode );
 
 // === CODE ===
+void _ReferenceMount(tVFS_Mount *Mount, const char *DebugTag)
+{
+//     Log_Debug("VFS", "%s: inc. mntpt '%s' to %i", DebugTag, Mount->MountPoint, Mount->OpenHandleCount+1);
+       Mount->OpenHandleCount ++;
+}
+void _DereferenceMount(tVFS_Mount *Mount, const char *DebugTag)
+{
+//     Log_Debug("VFS", "%s: dec. mntpt '%s' to %i", DebugTag, Mount->MountPoint, Mount->OpenHandleCount-1);
+       ASSERT(Mount->OpenHandleCount > 0);
+       Mount->OpenHandleCount --;
+}
 /**
  * \fn char *VFS_GetAbsPath(const char *Path)
  * \brief Create an absolute path from a relative one
@@ -201,7 +215,7 @@ restart_parse:
                        *TruePath = malloc( gVFS_RootMount->MountPointLen+1 );
                        strcpy(*TruePath, gVFS_RootMount->MountPoint);
                }
-               gVFS_RootMount->OpenHandleCount ++;
+               _ReferenceMount(gVFS_RootMount, "ParsePath - Fast Tree Root");
                if(MountPoint)  *MountPoint = gVFS_RootMount;
                LEAVE('p', gVFS_RootMount->RootNode);
                return gVFS_RootMount->RootNode;
@@ -239,13 +253,19 @@ restart_parse:
                                *MountPoint = mnt;
                        RWLock_Release( &glVFS_MountList );
                        LOG("Mount %p root", mnt);
+                       _ReferenceMount(mnt, "ParsePath - Mount Root");
                        LEAVE('p', mnt->RootNode);
                        return mnt->RootNode;
                }
                #endif
                longestMount = mnt;
        }
-       longestMount->OpenHandleCount ++;       // Increment assuimg it worked
+       if(!longestMount) {
+               Log_Panic("VFS", "VFS_ParsePath - No mount for '%s'", Path);
+               return NULL;
+       }
+       
+       _ReferenceMount(longestMount, "ParsePath");
        RWLock_Release( &glVFS_MountList );
        
        // Save to shorter variable
@@ -347,7 +367,7 @@ restart_parse:
 
                        // EVIL: Goto :)
                        LOG("Symlink -> '%s', restart", Path);
-                       mnt->OpenHandleCount --;        // Not in this mountpoint
+                       _DereferenceMount(mnt, "ParsePath - sym");
                        goto restart_parse;
                }
                
@@ -430,7 +450,7 @@ _error:
                *TruePath = NULL;
        }
        // Open failed, so decrement the open handle count
-       mnt->OpenHandleCount --;
+       _DereferenceMount(mnt, "ParsePath - error");
        
        LEAVE('n');
        return NULL;
@@ -459,6 +479,13 @@ int VFS_int_CreateHandle( tVFS_Node *Node, tVFS_Mount *Mount, int Mode )
                errno = EACCES;
                LEAVE_RET('i', -1);
        }
+
+       if( MM_GetPhysAddr(Node->Type) == 0 ) {
+               Log_Error("VFS", "Node %p from mount '%s' (%s) has a bad type (%p)",
+                       Node, Mount->MountPoint, Mount->Filesystem->Name, Node->Type);
+               errno = EINTERNAL;
+               LEAVE_RET('i', -1);
+       }
        
        i = VFS_AllocHandle( !!(Mode & VFS_OPENFLAG_USER), Node, Mode );
        if( i < 0 ) {
@@ -521,28 +548,44 @@ int VFS_OpenEx(const char *Path, Uint Flags, Uint Mode)
                        LEAVE_RET('i', -1);
                }
 
-               // TODO: Check ACLs on the parent
+               // Check ACLs on the parent
                if( !VFS_CheckACL(pnode, VFS_PERM_EXECUTE|VFS_PERM_WRITE) ) {
-                       _CloseNode(pnode);
-                       pmnt->OpenHandleCount --;
-                       free(absPath);
-                       LEAVE('i', -1);
-                       return -1;
+                       errno = EACCES;
+                       goto _pnode_err;
                }
 
                // Check that there's a MkNod method
                if( !pnode->Type || !pnode->Type->MkNod ) {
                        Log_Warning("VFS", "VFS_Open - Directory has no MkNod method");
                        errno = EINVAL;
-                       LEAVE_RET('i', -1);
+                       goto _pnode_err;
                }
                
                node = pnode->Type->MkNod(pnode, file, new_flags);
+               if( !node ) {
+                       LOG("Cannot create node '%s' in '%s'", file, absPath);
+                       errno = ENOENT;
+                       goto _pnode_err;
+               }
+               // Set mountpoint (and increment open handle count)
+               mnt = pmnt;
+               _ReferenceMount(mnt, "Open - create");
                // Fall through on error check
                
                _CloseNode(pnode);
-               pmnt->OpenHandleCount --;
+               _DereferenceMount(pmnt, "Open - create");
+               goto _pnode_ok;
+
+       _pnode_err:
+               if( pnode ) {
+                       _CloseNode(pnode);
+                       _DereferenceMount(pmnt, "Open - create,fail");
+                       free(absPath);
+               }
+               LEAVE('i', -1);
+               return -1;
        }
+       _pnode_ok:
        
        // Free generated path
        free(absPath);
@@ -552,7 +595,7 @@ int VFS_OpenEx(const char *Path, Uint Flags, Uint Mode)
        {
                LOG("Cannot find node");
                errno = ENOENT;
-               LEAVE_RET('i', -1);
+               goto _error;
        }
        
        // Check for symlinks
@@ -561,26 +604,35 @@ int VFS_OpenEx(const char *Path, Uint Flags, Uint Mode)
                char    tmppath[node->Size+1];
                if( node->Size > MAX_PATH_LEN ) {
                        Log_Warning("VFS", "VFS_Open - Symlink is too long (%i)", node->Size);
-                       LEAVE_RET('i', -1);
+                       goto _error;
                }
                if( !node->Type || !node->Type->Read ) {
                        Log_Warning("VFS", "VFS_Open - No read method on symlink");
-                       LEAVE_RET('i', -1);
+                       goto _error;
                }
                // Read symlink's path
                node->Type->Read( node, 0, node->Size, tmppath );
                tmppath[ node->Size ] = '\0';
                _CloseNode( node );
+               _DereferenceMount(mnt, "Open - symlink");
                // Open the target
                node = VFS_ParsePath(tmppath, NULL, &mnt);
                if(!node) {
                        LOG("Cannot find symlink target node (%s)", tmppath);
                        errno = ENOENT;
-                       LEAVE_RET('i', -1);
+                       goto _error;
                }
        }
 
-       LEAVE_RET('x', VFS_int_CreateHandle(node, mnt, Flags));
+        int    ret = VFS_int_CreateHandle(node, mnt, Flags);
+       LEAVE_RET('x', ret);
+_error:
+       if( node )
+       {
+               _DereferenceMount(mnt, "Open - error");
+               _CloseNode(node);
+       }
+       LEAVE_RET('i', -1);
 }
 
 
@@ -624,7 +676,7 @@ int VFS_OpenChild(int FD, const char *Name, Uint Mode)
        }
 
        // Increment open handle count, no problems with the mount going away as `h` is already open on it
-       h->Mount->OpenHandleCount ++;
+       _ReferenceMount(h->Mount, "OpenChild");
 
        LEAVE_RET('x', VFS_int_CreateHandle(node, h->Mount, Mode));
 }
@@ -676,7 +728,12 @@ void VFS_Close(int FD)
                Log_Warning("VFS", "Invalid file handle passed to VFS_Close, 0x%x", FD);
                return;
        }
-       
+
+       if( h->Node == NULL ) {
+               Log_Warning("VFS", "Non-open handle passed to VFS_Close, 0x%x", FD);
+               return ;
+       }       
+
        #if VALIDATE_VFS_FUNCTIPONS
        if(h->Node->Close && !MM_GetPhysAddr(h->Node->Close)) {
                Log_Warning("VFS", "Node %p's ->Close method is invalid (%p)",
@@ -685,10 +742,12 @@ void VFS_Close(int FD)
        }
        #endif
        
+       LOG("Handle %x", FD);
        _CloseNode(h->Node);
 
-       if( h->Mount )
-               h->Mount->OpenHandleCount --;   
+       if( h->Mount ) {
+               _DereferenceMount(h->Mount, "Close");
+       }
 
        h->Node = NULL;
 }

UCC git Repository :: git.ucc.asn.au