Hi,
After not working on fatfs for a long time I picked it up again. The
current problem fatfs has is that write support does not yet work.
This is because of a problem in the locking. I hope you all still
know about this issue, if it is not clear I can explain again or point
at the previous discussions about this (but I will explain the problem
again later in this way, using another approach: having a look at the
cause and not at the effect).
The previously proposed solution was adding a flag to some interfaces
like diskfs_cached_lookup to tell this function the node of the
directory is locked. For reading support this works perfectly. When
I was trying the same for write support I ran into a problem. The
diskfs_nput function calls diskfs_drop_node, which writes back the
node to disk and drops it. When the node is written to disk the
directory holding the node must be locked.
The problem here is that diskfs_nput is called from almost anywhere in
both libdiskfs and the filesystem. So it is impossible to make any
assumption if the directory holding the node is locked or not. I
think everyone would agree that adding a flag to inform diskfs_nput is
locked or not is insane. :)
To find another solution, it is important to understand the problem
better. In a filesystem with hardlinks there are two separate
concepts: inodes and directory entries. An inode can be used to
access the data on the disk and is identified with a number. The
directory entry is placed in a directory and gives the inode a name,
there can be multiple dir. ents. pointing at the same inode. For some
operations both need to be locked at the same time.
In fatfs both the dir ent. and the inode are stored in the same
(physical) location. The problem with the current code is that the
same node can be locked twice, once as directory entry and once as
inode. One example of what happens:
libdiskfs (diskfs_S_dir_unlink):
- first the directory is locked. (Because it holds the *directory entry*)
- The node is looked up and locked.
- diskfs_node_update is called.
fatfs: (diskfs_node_update)
- in fatfs diskfs_node_update calls write_node
- write_node locks the directory again (Because it holds the *inode*)
This results in a locked up fatfs. This will happen only when the
inode on disk is updated and the directory is locked because it as the
directory entry.
Hopefully the problem will be clear by this explanation.
Now for a possible fix of the problem is by adding a lock (rwlock) to
the disknode. This node will protect against the changes to all the
dirents of the directory (including resizing the dir. and changing
entries). All files (already) have a reference to the directory
holding that file and can access the dirent that way (and lock it).
This is safe because of that.
So before it accesses any directory entry in the directory (either as
dir entry or for inode related stuff) the disknode will be locked.
Sometimes the directory ("struct node") will be locked by libdiskfs,
but that will be used because fatfs knows more about the specific
locking.
I assume ignoring the lock will be safe because libdiskfs will never
write to the directory itself and the node will continue to exist
because the files in the directory hold a reference.
The only issue that could be a problem is that libdiskfs can access
the directory as file (readonly). Perhaps it is possible to modify
libdiskfs in a way so it does not allow io_read and io_map for
directories when a node is a directory (and make it return EISDIR?).
I will need to think better about this, I'd like to hear opinions of
other people about this.
This will not be the only solution, but this is the best I see at the
moment. And the advantage is that big interface changes to libdiskfs
are not required anymore (and that this *will* work of course :)). I
really hope other people will have better solutions to this problem.
If you agree with this solution, please tell me so I can start
implementing this.
Thanks,
Marco
_______________________________________________
Bug-hurd mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-hurd
