svn commit: r366997 - head/sys/kern

[Konstantin Belousov]

On Sat, Oct 24, 2020 at 01:30:37PM +0000, Mateusz Guzik wrote:
> Author: mjg
> Date: Sat Oct 24 13:30:37 2020
> New Revision: 366997
> URL: https://svnweb.freebsd.org/changeset/base/366997
> 
> Log:
>   vfs: fix a race where reclaim vholds freed vnodes
A description of the race in the commit message would be respectful to
other readers of the code, so that we do not need to reverse-eng the
change to understand what and why was fixed.

>   
>   Reported by:	pho
>   Tested by:	pho (previous version)
>   Fixes:	r366974 ("vfs: stop taking the interlock in vnode reclaim")
> 
> Modified:
>   head/sys/kern/vfs_subr.c
> 
> Modified: head/sys/kern/vfs_subr.c
> ==============================================================================
> --- head/sys/kern/vfs_subr.c	Sat Oct 24 13:16:10 2020	(r366996)
> +++ head/sys/kern/vfs_subr.c	Sat Oct 24 13:30:37 2020	(r366997)
> @@ -109,6 +109,7 @@ static void	syncer_shutdown(void *arg, int howto);
>  static int	vtryrecycle(struct vnode *vp);
>  static void	v_init_counters(struct vnode *);
>  static void	vgonel(struct vnode *);
> +static bool	vhold_recycle(struct vnode *);
>  static void	vfs_knllock(void *arg);
>  static void	vfs_knlunlock(void *arg);
>  static void	vfs_knl_assert_locked(void *arg);
> @@ -1126,7 +1127,8 @@ restart:
>  			goto next_iter;
>  		}
>  
> -		vhold(vp);
> +		if (!vhold_recycle(vp))
> +			goto next_iter;
>  		TAILQ_REMOVE(&vnode_list, mvp, v_vnodelist);
>  		TAILQ_INSERT_AFTER(&vnode_list, vp, mvp, v_vnodelist);
>  		mtx_unlock(&vnode_list_mtx);
> @@ -1231,7 +1233,8 @@ restart:
>  		if (__predict_false(vp->v_type == VBAD || vp->v_type == VNON)) {
>  			continue;
>  		}
> -		vhold(vp);
> +		if (!vhold_recycle(vp))
> +			continue;
>  		count--;
>  		mtx_unlock(&vnode_list_mtx);
>  		vtryrecycle(vp);
> @@ -3248,13 +3251,11 @@ vholdnz(struct vnode *vp)
>   * However, while this is more performant, it hinders debugging by eliminating
>   * the previously mentioned invariant.
>   */
> -bool
> -vhold_smr(struct vnode *vp)
> +static bool __always_inline
> +_vhold_cond(struct vnode *vp)
>  {
>  	int count;
>  
> -	VFS_SMR_ASSERT_ENTERED();
> -
>  	count = atomic_load_int(&vp->v_holdcnt);
>  	for (;;) {
>  		if (count & VHOLD_NO_SMR) {
> @@ -3270,6 +3271,28 @@ vhold_smr(struct vnode *vp)
>  			return (true);
>  		}
>  	}
> +}
> +
> +bool
> +vhold_smr(struct vnode *vp)
> +{
> +
> +	VFS_SMR_ASSERT_ENTERED();
> +	return (_vhold_cond(vp));
> +}
> +
> +/*
> + * Special case for vnode recycling.
> + *
> + * Vnodes are present on the global list until UMA takes them out.
> + * Attempts to recycle only need the relevant lock and have no use for SMR.
> + */
> +static bool
> +vhold_recycle(struct vnode *vp)
> +{
> +
> +	mtx_assert(&vnode_list_mtx, MA_OWNED);
> +	return (_vhold_cond(vp));
>  }
>  
>  static void __noinline