On Fri, Jul 11, 2025 at 1:21 AM NeilBrown <neil@xxxxxxxxxx> wrote: > > This is a revised set of patches following helpful feedback. There are > now more patches, but they should be a lot easier to review. I confirm that this set was "reviewable" :) No major comments on my part, mostly petty nits. I would prefer to see parent_lock/unlock helpers in vfs for v3, but if you prefer to keep the prep patches internal to ovl, that's fine too. In that case I'd prefer to use ovl_parent_lock/unlock, but if that's too painful, don't bother. Thanks, Amir. > > These patches are all in a git tree at > https://github.com/neilbrown/linux/commits/pdirops > though there a lot more patches there too - demonstrating what is to come. > 0eaa1c629788 ovl: rename ovl_cleanup_unlocked() to ovl_cleanup() > is the last in the series posted here. > > I welcome further review. > > Original description: > > This series of patches for overlayfs is primarily focussed on preparing > for some proposed changes to directory locking. In the new scheme we > will lock individual dentries in a directory rather than the whole > directory. > > ovl currently will sometimes lock a directory on the upper filesystem > and do a few different things while holding the lock. This is > incompatible with the new scheme. > > This series narrows the region of code protected by the directory lock, > taking it multiple times when necessary. This theoretically open up the > possibilty of other changes happening on the upper filesytem between the > unlock and the lock. To some extent the patches guard against that by > checking the dentries still have the expect parent after retaking the > lock. In general, I think ovl would have trouble if upperfs were being > changed independantly, and I don't think the changes here increase the > problem in any important way. > > I have tested this with fstests, both generic and unionfs tests. I > wouldn't be surprised if I missed something though, so please review > carefully. > > After this series (with any needed changes) lands I will resubmit my > change to vfs_rmdir() behaviour to have it drop the lock on error. ovl > will be much better positioned to handle that change. It will come with > the new "lookup_and_lock" API that I am proposing. > > Thanks, > NeilBrown > > > [PATCH 01/20] ovl: simplify an error path in ovl_copy_up_workdir() > [PATCH 02/20] ovl: change ovl_create_index() to take write and dir > [PATCH 03/20] ovl: Call ovl_create_temp() without lock held. > [PATCH 04/20] ovl: narrow the locked region in ovl_copy_up_workdir() > [PATCH 05/20] ovl: narrow locking in ovl_create_upper() > [PATCH 06/20] ovl: narrow locking in ovl_clear_empty() > [PATCH 07/20] ovl: narrow locking in ovl_create_over_whiteout() > [PATCH 08/20] ovl: narrow locking in ovl_rename() > [PATCH 09/20] ovl: narrow locking in ovl_cleanup_whiteouts() > [PATCH 10/20] ovl: narrow locking in ovl_cleanup_index() > [PATCH 11/20] ovl: narrow locking in ovl_workdir_create() > [PATCH 12/20] ovl: narrow locking in ovl_indexdir_cleanup() > [PATCH 13/20] ovl: narrow locking in ovl_workdir_cleanup_recurse() > [PATCH 14/20] ovl: change ovl_workdir_cleanup() to take dir lock as > [PATCH 15/20] ovl: narrow locking on ovl_remove_and_whiteout() > [PATCH 16/20] ovl: change ovl_cleanup_and_whiteout() to take rename > [PATCH 17/20] ovl: narrow locking in ovl_whiteout() > [PATCH 18/20] ovl: narrow locking in ovl_check_rename_whiteout() > [PATCH 19/20] ovl: change ovl_create_real() to receive dentry parent > [PATCH 20/20] ovl: rename ovl_cleanup_unlocked() to ovl_cleanup()
