Re: [PATCH 16/19] VFS: add lookup_and_lock_rename()

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

On Thu, Feb 06, 2025 at 04:42:53PM +1100, NeilBrown wrote:
> @@ -3451,8 +3451,14 @@ static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2)
>  {
>  	struct dentry *p = p1, *q = p2, *r;
>  
> -	while ((r = p->d_parent) != p2 && r != p)
> +	/* Ensure d_update_wait() tests are safe - one barrier for all */
> +	smp_mb();
> +
> +	d_update_wait(p, I_MUTEX_NORMAL);
> +	while ((r = p->d_parent) != p2 && r != p) {
>  		p = r;
> +		d_update_wait(p, I_MUTEX_NORMAL);
> +	}
>  	if (r == p2) {
>  		// p is a child of p2 and an ancestor of p1 or p1 itself
>  		inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
> @@ -3461,8 +3467,11 @@ static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2)
>  	}
>  	// p is the root of connected component that contains p1
>  	// p2 does not occur on the path from p to p1
> -	while ((r = q->d_parent) != p1 && r != p && r != q)
> +	d_update_wait(q, I_MUTEX_NORMAL);
> +	while ((r = q->d_parent) != p1 && r != p && r != q) {
>  		q = r;
> +		d_update_wait(q, I_MUTEX_NORMAL);
> +	}

That makes no sense whatsoever.  What are you waiting on here and _why_
are you waiting on those sucker?  Especially since there's nothing
to prevent the condition for which you wait from arising immediately
afterwards.