lkml.org 
[lkml]   [2006]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: A missing i_mutex in rename? (Linux kernel 2.6.latest)
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-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-04-20 13:02    [W:0.048 / U:0.784 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site