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.
