Messages in this thread | | | Date | Sat, 6 Feb 1999 21:03:57 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: Linux-2.2.2-pre2.. |
| |
On Sat, 6 Feb 1999, Alexander Viro wrote: > > D'oh. Now, if it would be the only bad thing about rename()... > In essence, we have it serialized by a per-fs lock. It's done in each > fs separately. And it prevents us from moving other generic tests to VFS, > where they belong.
Yeah, that would be good. "rename()" is a real pain. It's _the_ most complex operation, mainly because of renaming subdirectories has such a non-local effect.
> I'ld be more than happy to have this puppet moved to > vfs_rename(), along with is_subdir() stuff and check for source and target > having the same inode. Then foo_rename()s would start to look sane. But it > requires adding new fields to struct super (lock and wait queue) [1]. If > you'll give clearance I'll do it ASAP.
I'll give you clearance, not for 2.2.2, but you wouldn't get it done by then anyway, so 2.2.3 would be your target. HOWEVER, I'd ask you to actually go one step further: the VFS layer should separate the case of renaming a subdirectory from the case of renaming a regular file.
In the end, both cases would call the same fs-specific rename() function, but basically something like this would be what I'd want to see to make each fs not have to worry about locking at all, the way it should be:
- a non-directory "rename()" would _not_ get the per-fs rename lock. They don't need them, the per-fs lock is only used to protect against concurrent renames that change the topology (so that you can sanely avoid loops in the directory structure)
- the directory case is different in other ways too: it needs to do the is_subdir() check (which should also be done on a VFS layer, as it these days is just a matter of following the dentry pointers), and it needs to look up the target dentry with the LOOKUP_SLASHOK flag.
- the directory case would unhash the target directory in the VFS layer as per your race fix, and would re-hash it if the low-level rename returns an error.
- the VFS layer would do the "d_move()" operation (both for directory and non-directories), the low-level FS would only do the actual low-level rename.
If we have to change rename (and all low-level filesystems) to fix the race, let's just fix it once and for all, and separate the two rename cases properly.
Linus
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |