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... -- 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
