Neil,
Half way through the review I noticed that your individual patches are
not tagged v2
This makes a bit of a mess when trying to search the inbox for
previous versions.
Please do git format-patch -v3 for the next posting.
On Fri, Jul 11, 2025 at 1:21 AM NeilBrown <neil@xxxxxxxxxx> wrote:
>
> Drop the locks immediately after rename, and use a separate lock for
> cleanup.
>
> This makes way for future changes where locks are taken on individual
> dentries rather than the whole directory.
>
> Note that ovl_cleanup_whiteouts() operates on "upper", a child of
> "upperdir" and does not require upperdir or workdir to be locked.
>
> Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
You may keep my RVB, but...
> Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
> ---
> fs/overlayfs/dir.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index fa438e13e8b1..b3d858654f23 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -353,7 +353,6 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
> {
> struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
> struct dentry *workdir = ovl_workdir(dentry);
> - struct inode *wdir = workdir->d_inode;
> struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
> struct path upperpath;
> struct dentry *upper;
> @@ -400,10 +399,10 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
> err = ovl_do_rename(ofs, workdir, opaquedir, upperdir, upper, RENAME_EXCHANGE);
> if (err)
> goto out_cleanup;
> + unlock_rename(workdir, upperdir);
If you look out_cleanup now, it basically does unlock_rename(workdir, upperdir);
and then out_cleanup_unlocked:
so I think this would look a bit nicer to bring unlock further closer
to do_rename ?
err = ovl_do_rename(ofs, workdir, opaquedir, upperdir, upper,
RENAME_EXCHANGE);
+ unlock_rename(workdir, upperdir);
if (err)
- goto out_cleanup;
+ goto out_cleanup_unlocked;
and leave newline after goto please :)
Thanks,
Amir.
