Re: A missing i_mutex in rename? (Linux kernel 2.6.latest)

[Matthew Wilcox <matthew@wil.cx>]

On Wed, Apr 19, 2006 at 11:50:00AM +0100, Anton Altaparmakov wrote:
> Both sys_unlink()/sys_rmdir() and sys_link() all end up taking the i_mutex 
> on all parent directories and source/destination inodes before calling 
> into the file system inode operations.
> 
> sys_rename() OTOH, does not take i_mutex on the old inode.  It only takes 
> i_mutex on the two parent directories and on the target inode if it 
> exists.
> 
> Why is this?  To me it seems that either sys_rename() must take i_mutex on 
> the old inode or sys_unlink()/sys_rmdir(), sys_link(), etc do not need to 
> hold the i_mutex.
> 
> What am I missing?

I believe the current locking scheme to be correct.  Reading
Documentation/filesystems/directory-locking and pondering for a few
minutes leads me to the following conclusions:

 - sys_rmdir() must take the lock on the parent directory and on the
   victim.  If a different process is trying to create a file in the
   victim, sys_rmdir() mustn't race with it.
 - 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.
 - sys_link() needs to lock the target to be sure it isn't removed and
   replaced with a directory in the meantime.
 - sys_rename() does not need to lock the old inode.  Since the parent
   is already locked, the old inode can't be removed/renamed by a racing
   process.  It doesn't matter if something's created or deleted from
   within the old inode (if it's a directory), unlike rmdir().  It
   doesn't need to be protected from a sys_link() race.

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.