On Thu, 20 Apr 2006, Al Viro wrote:
> On Wed, Apr 19, 2006 at 01:51:21PM +0100, Anton Altaparmakov wrote:
> > > - I don't immediately see a race that taking the lock on the victim of
> > > sys_unlink() solves; however, for symmetry with sys_rmdir(), it seems
> > > desirable.
> >
> > I guess the symmetry thing is fair enough.
>
> Not only; it does, among other things, guarantee that fs can assume that
> ->link() won't race with unlink() (and that link count is protected by
> ->i_mutex, while we are at it).
Ok.
> > ntfs_rename() at the moment looks roughly like this:
> >
> > if (target_inode) {
> > if (S_ISDIR(target_inode->i_mode)
> > ntfs_rmdir(target_dir_inode, target_dentry);
> > else
> > ntfs_unlink(target_dir_inode, target_dentry);
> > }
> > mutex_lock(&old_inode->i_mutex);
> > ntfs_link(old_dentry, target_dir_inode, target_dentry);
> > ntfs_unlink(old_dir_inode, old_dentry);
> > mutex_unlock(&old_inode->i_mutex);
>
> Have fun dealing with error handling in the above... Note that failing
> rename() should _NOT_ lead to target disappearing.
Indeed. But if I do not define a rename operation at all then the mv
command looses the target if the move fails, too. So I don't see how the
native rename needs to be any better other than for standards compliance.
Error recovery is actually pretty easy as if the second ntfs_unlink()
fails I simply ntfs_unlink() the link I just created with ntfs_link().
That already works fine. But yes at the moment I do lose the target if
either the ntfs_link() or the second ntfs_unlink() fails.
I am planning to fix this though. It is not too hard because I can more
or less just ntfs_link() it back in (with a few special casings).
Even if I implement are more "native" rename not using
ntfs_unlink()/ntfs_link() I still have to remove the target first
otherwise I cannot insert the new target because you cannot have two
entries in the B+tree that have identical index keys (the index key is the
filename itself so the two would be equal). And I cannot simply replace
the B+tree index entry because NTFS is not case sensitive so the two
names (old and new) may not be in the same place in the B+tree. The only
thing I can do is to remove the old B+tree index entry and add the new one
after that fact. So no matter what I do, I have to be able to recreate
the old index entry and that is just what ntfs_link() does which is why
I chose my "simplified rename" approach in the first place.
Thanks a lot for you comments about the locking!
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
