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).
> > - sys_link() needs to lock the target to be sure it isn't removed and
> > replaced with a directory in the meantime.
>
> Agreed.
"Replaced" part is bogus - we'd done lookup, so we won't get anything new.
> > If you need to lock the old inode inside ntfs for your own consistency
> > purposes, that looks like it should be fine, but the VFS doesn't need to
> > lock it for you.
>
> Great, thanks. That was my own conclusion also but it never hurts to be
> sure. (-:
>
> 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.
-
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
