mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 13:12:46 +00:00 
			
		
		
		
	Okay let's just not have this broken locking at all right now.
I think I should just protect access to shared data structures and eventually do read/write atomicity locks at the inode level.
This commit is contained in:
		
							parent
							
								
									e6284a8774
								
							
						
					
					
						commit
						b1ff62f605
					
				
					 7 changed files with 16 additions and 38 deletions
				
			
		|  | @ -2,7 +2,6 @@ | |||
| #include "Task.h" | ||||
| #include "Syscall.h" | ||||
| #include "Console.h" | ||||
| #include <AK/Lock.h> | ||||
| 
 | ||||
| extern "C" void syscall_entry(); | ||||
| extern "C" void syscall_ISR(); | ||||
|  | @ -100,7 +99,6 @@ DWORD handle(DWORD function, DWORD arg1, DWORD arg2, DWORD arg3) | |||
|         return current->sys$gethostname((char*)arg1, (size_t)arg2); | ||||
|     case Syscall::PosixExit: | ||||
|         cli(); | ||||
|         //locker.unlock();
 | ||||
|         current->sys$exit((int)arg1); | ||||
|         ASSERT_NOT_REACHED(); | ||||
|         return 0; | ||||
|  |  | |||
|  | @ -204,6 +204,7 @@ InodeMetadata Ext2FileSystem::inodeMetadata(InodeIdentifier inode) const | |||
|         metadata.majorDevice = (dev & 0xfff00) >> 8; | ||||
|         metadata.minorDevice= (dev & 0xff) | ((dev >> 12) & 0xfff00); | ||||
|     } | ||||
| 
 | ||||
|     return metadata; | ||||
| } | ||||
| 
 | ||||
|  |  | |||
|  | @ -25,8 +25,6 @@ bool additionWouldOverflow(Unix::off_t a, Unix::off_t b) | |||
| 
 | ||||
| int FileHandle::stat(Unix::stat* buffer) | ||||
| { | ||||
|     LOCKER(VirtualFileSystem::lock()); | ||||
| 
 | ||||
|     if (!m_vnode) | ||||
|         return -EBADF; | ||||
| 
 | ||||
|  |  | |||
|  | @ -56,15 +56,15 @@ auto SyntheticFileSystem::createDirectory(String&& name) -> OwnPtr<File> | |||
|     return file; | ||||
| } | ||||
| 
 | ||||
| auto SyntheticFileSystem::createTextFile(String&& name, String&& text) -> OwnPtr<File> | ||||
| auto SyntheticFileSystem::createTextFile(String&& name, ByteBuffer&& contents, Unix::mode_t mode) -> OwnPtr<File> | ||||
| { | ||||
|     auto file = make<File>(); | ||||
|     file->data = text.toByteBuffer(); | ||||
|     file->data = contents; | ||||
|     file->name = move(name); | ||||
|     file->metadata.size = file->data.size(); | ||||
|     file->metadata.uid = 100; | ||||
|     file->metadata.gid = 200; | ||||
|     file->metadata.mode = 0010644; | ||||
|     file->metadata.mode = mode; | ||||
|     file->metadata.mtime = mepoch; | ||||
|     return file; | ||||
| } | ||||
|  |  | |||
|  | @ -39,7 +39,7 @@ protected: | |||
|     }; | ||||
| 
 | ||||
|     OwnPtr<File> createDirectory(String&& name); | ||||
|     OwnPtr<File> createTextFile(String&& name, String&& text); | ||||
|     OwnPtr<File> createTextFile(String&& name, ByteBuffer&&, Unix::mode_t = 0010644); | ||||
|     OwnPtr<File> createGeneratedFile(String&& name, Function<ByteBuffer()>&&, Unix::mode_t = 0100644); | ||||
| 
 | ||||
|     InodeIdentifier addFile(OwnPtr<File>&&, InodeIndex parent = RootInodeIndex); | ||||
|  |  | |||
|  | @ -103,11 +103,9 @@ auto VirtualFileSystem::getOrCreateNode(InodeIdentifier inode) -> RetainPtr<Node | |||
| 
 | ||||
| bool VirtualFileSystem::mount(RetainPtr<FileSystem>&& fileSystem, const String& path) | ||||
| { | ||||
|     kprintf("mount\n"); | ||||
|     LOCKER(VirtualFileSystem::lock()); | ||||
|     ASSERT(fileSystem); | ||||
|     int error; | ||||
|     auto inode = resolvePath(path, error, locker); | ||||
|     auto inode = resolvePath(path, error); | ||||
|     if (!inode.isValid()) { | ||||
|         kprintf("[VFS] mount can't resolve mount point '%s'\n", path.characters()); | ||||
|         return false; | ||||
|  | @ -360,10 +358,8 @@ void VirtualFileSystem::listDirectoryRecursively(const String& path) | |||
| 
 | ||||
| bool VirtualFileSystem::touch(const String& path) | ||||
| { | ||||
|     LOCKER(VirtualFileSystem::lock()); | ||||
| 
 | ||||
|     int error; | ||||
|     auto inode = resolvePath(path, error, locker); | ||||
|     auto inode = resolvePath(path, error); | ||||
|     if (!inode.isValid()) | ||||
|         return false; | ||||
|     return inode.fileSystem()->setModificationTime(inode, ktime(nullptr)); | ||||
|  | @ -371,9 +367,7 @@ bool VirtualFileSystem::touch(const String& path) | |||
| 
 | ||||
| OwnPtr<FileHandle> VirtualFileSystem::open(const String& path, int& error, int options, InodeIdentifier base) | ||||
| { | ||||
|     LOCKER(VirtualFileSystem::lock()); | ||||
| 
 | ||||
|     auto inode = resolvePath(path, error, locker, base, options); | ||||
|     auto inode = resolvePath(path, error, base, options); | ||||
|     if (!inode.isValid()) | ||||
|         return nullptr; | ||||
|     auto vnode = getOrCreateNode(inode); | ||||
|  | @ -384,8 +378,6 @@ OwnPtr<FileHandle> VirtualFileSystem::open(const String& path, int& error, int o | |||
| 
 | ||||
| OwnPtr<FileHandle> VirtualFileSystem::create(const String& path, InodeIdentifier base) | ||||
| { | ||||
|     LOCKER(VirtualFileSystem::lock()); | ||||
| 
 | ||||
|     // FIXME: Do the real thing, not just this fake thing!
 | ||||
|     (void) path; | ||||
|     m_rootNode->fileSystem()->createInode(m_rootNode->fileSystem()->rootInode(), "empty", 0100644, 0); | ||||
|  | @ -394,28 +386,25 @@ OwnPtr<FileHandle> VirtualFileSystem::create(const String& path, InodeIdentifier | |||
| 
 | ||||
| OwnPtr<FileHandle> VirtualFileSystem::mkdir(const String& path, InodeIdentifier base) | ||||
| { | ||||
|     LOCKER(VirtualFileSystem::lock()); | ||||
|     // FIXME: Do the real thing, not just this fake thing!
 | ||||
|     (void) path; | ||||
|     m_rootNode->fileSystem()->makeDirectory(m_rootNode->fileSystem()->rootInode(), "mydir", 0400755); | ||||
|     return nullptr; | ||||
| } | ||||
| 
 | ||||
| InodeIdentifier VirtualFileSystem::resolveSymbolicLink(InodeIdentifier base, InodeIdentifier symlinkInode, int& error, Locker& locker) | ||||
| InodeIdentifier VirtualFileSystem::resolveSymbolicLink(InodeIdentifier base, InodeIdentifier symlinkInode, int& error) | ||||
| { | ||||
|     locker.unlock(); | ||||
|     auto symlinkContents = symlinkInode.readEntireFile(); | ||||
|     locker.lock(); | ||||
|     if (!symlinkContents) | ||||
|         return { }; | ||||
|     auto linkee = String((const char*)symlinkContents.pointer(), symlinkContents.size()); | ||||
| #ifdef VFS_DEBUG | ||||
|     kprintf("linkee (%s)(%u) from %u:%u\n", linkee.characters(), linkee.length(), base.fileSystemID(), base.index()); | ||||
| #endif | ||||
|     return resolvePath(linkee, error, locker, base); | ||||
|     return resolvePath(linkee, error, base); | ||||
| } | ||||
| 
 | ||||
| String VirtualFileSystem::absolutePathInternal(InodeIdentifier inode, Locker& locker) | ||||
| String VirtualFileSystem::absolutePath(InodeIdentifier inode) | ||||
| { | ||||
|     if (!inode.isValid()) | ||||
|         return String(); | ||||
|  | @ -428,7 +417,7 @@ String VirtualFileSystem::absolutePathInternal(InodeIdentifier inode, Locker& lo | |||
|         else | ||||
|             lineage.append(inode); | ||||
|         if (inode.metadata().isDirectory()) { | ||||
|             inode = resolvePath("..", error, locker, inode); | ||||
|             inode = resolvePath("..", error, inode); | ||||
|         } else | ||||
|             inode = inode.fileSystem()->findParentOfInode(inode); | ||||
|         ASSERT(inode.isValid()); | ||||
|  | @ -448,13 +437,7 @@ String VirtualFileSystem::absolutePathInternal(InodeIdentifier inode, Locker& lo | |||
|     return builder.build(); | ||||
| } | ||||
| 
 | ||||
| String VirtualFileSystem::absolutePath(InodeIdentifier inode) | ||||
| { | ||||
|     LOCKER(VirtualFileSystem::lock()); | ||||
|     return absolutePathInternal(inode, locker); | ||||
| } | ||||
| 
 | ||||
| InodeIdentifier VirtualFileSystem::resolvePath(const String& path, int& error, Locker& locker, InodeIdentifier base, int options) | ||||
| InodeIdentifier VirtualFileSystem::resolvePath(const String& path, int& error, InodeIdentifier base, int options) | ||||
| { | ||||
|     if (path.isEmpty()) | ||||
|         return { }; | ||||
|  | @ -521,7 +504,7 @@ InodeIdentifier VirtualFileSystem::resolvePath(const String& path, int& error, L | |||
|                 if (options & O_NOFOLLOW_NOERROR) | ||||
|                     return inode; | ||||
|             } | ||||
|             inode = resolveSymbolicLink(parent, inode, error, locker); | ||||
|             inode = resolveSymbolicLink(parent, inode, error); | ||||
|             if (!inode.isValid()) { | ||||
|                 kprintf("Symbolic link resolution failed :(\n"); | ||||
|                 return { }; | ||||
|  |  | |||
|  | @ -104,11 +104,9 @@ public: | |||
| private: | ||||
|     friend class FileHandle; | ||||
| 
 | ||||
|     String absolutePathInternal(InodeIdentifier, Locker&); | ||||
| 
 | ||||
|     void enumerateDirectoryInode(InodeIdentifier, Function<bool(const FileSystem::DirectoryEntry&)>); | ||||
|     InodeIdentifier resolvePath(const String& path, int& error, Locker&, InodeIdentifier base = InodeIdentifier(), int options = 0); | ||||
|     InodeIdentifier resolveSymbolicLink(InodeIdentifier base, InodeIdentifier symlinkInode, int& error, Locker&); | ||||
|     InodeIdentifier resolvePath(const String& path, int& error, InodeIdentifier base = InodeIdentifier(), int options = 0); | ||||
|     InodeIdentifier resolveSymbolicLink(InodeIdentifier base, InodeIdentifier symlinkInode, int& error); | ||||
| 
 | ||||
|     RetainPtr<Node> allocateNode(); | ||||
|     void freeNode(Node*); | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andreas Kling
						Andreas Kling