Re: [RFC][PATH 4/4] ovl: relax lock_rename when moving files between work and upper dir

[Al Viro <viro@ZenIV.linux.org.uk>]

On Fri, Nov 11, 2016 at 06:17:07PM +0200, Amir Goldstein wrote:

> I'm afraid so.
> It seems to me that most of the time, lock_rename() is being used in
> overlayfs for no better alternative to lock 2 directories (work+upper).
> 
> My suggestion is a small modification to the overlayfs locking scheme.
> ---Instead of this:
> assert(lock_rename(workdir, upperdir) != NULL));
>    copy_up(src, tmp);
>    vfs_rename(tmp, dst);
> unlock_rename(workdir, upperdir);
> 
> +++Use this:
> assert(lock_rename(workdir, upperdir) != NULL));
> mutex_unlock(s_vfs_rename_mutex);
>    copy_up(src, tmp);
> inode_unlock(upperdir);
> inode_unlock(workdir);
> assert(lock_rename(workdir, upperdir) != NULL));
>   vfs_rename(tmp, dst);
> unlock_rename(workdir, upperdir);
> 
> Miklos,
> 
> Do you see any problem with the proposed scheme?
> Anything that can go wrong while releasing the workdir lock before vfs_rename()?

Huh???

->rename() definitely counts upon parents being locked; please, read the
damn Documentation/filesystems/locking, it's there for a reason.

The real question is why the fsck do you need to lock the workdir for the
duration of copying at all.  O_TMPFILE open + writes there doesn't need
lock_rename() *or* parents being locked.  You need the parent locked when
you link the sucker in place, but that's it.  IDGI...