Re: [PATCH] NFSD: restart ssc_expire_umount walk after dropping nfsd_ssc_lock

[Chuck Lever]

On Fri, May 22, 2026, at 9:41 PM, Michael Bommarito wrote:

> -			/* wakeup ssc_connect waiters */
> -			do_wakeup = true;
> -			continue;
> -		}
> -		break;
> +		/* wakeup ssc_connect waiters */
> +		do_wakeup = true;
> +		/*
> +		 * The list_for_each_entry_safe() saved-next pointer was
> +		 * not pinned across the spin_unlock() above: a concurrent
> +		 * nfsd4_ssc_cancel_dul() can free the next item under the
> +		 * same spinlock while mntput() runs.  Restart the walk
> +		 * from the head so no stale next is dereferenced.
> +		 */

Same observation as Jeff's:

The comment references list_for_each_entry_safe(), but the patched code
uses plain list_for_each_entry(). A reader looking only at the post-patch
source sees no _safe iterator anywhere in the function and has to
reconstruct the bug history to make sense of the comment.

The past-tense framing ("was not pinned") reinforces this: it describes a
removed code state rather than the current one.

Please send a v2 with this comment corrected.


> +		spin_unlock(&nn->nfsd_ssc_lock);
> +		goto restart;
>  	}
>  	if (do_wakeup)
>  		wake_up_all(&nn->nfsd_ssc_waitq);
> -- 
> 2.53.0

-- 
Chuck Lever