Re: git: b58ca5df0bb7 - main - vfs: remove the now unused insmntque1

On Thu, Jan 27, 2022 at 12:12:28AM +0000, Mateusz Guzik wrote:
> The branch main has been updated by mjg:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=b58ca5df0bb79f2d1c443617e0899602721ccffa
> 
> commit b58ca5df0bb79f2d1c443617e0899602721ccffa
> Author:     Mateusz Guzik <mjg@FreeBSD.org>
> AuthorDate: 2022-01-27 00:00:24 +0000
> Commit:     Mateusz Guzik <mjg@FreeBSD.org>
> CommitDate: 2022-01-27 00:00:24 +0000
> 
>     vfs: remove the now unused insmntque1
>     
>     Bump __FreeBSD_version to 1400052.
> ---
>  sys/kern/vfs_subr.c | 12 +-----------
>  sys/sys/param.h     |  2 +-
>  sys/sys/vnode.h     |  2 --
>  3 files changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
> index 3218a3f7b6a0..a29f64fddd34 100644
> --- a/sys/kern/vfs_subr.c
> +++ b/sys/kern/vfs_subr.c
> @@ -1948,8 +1948,7 @@ insmntque_stddtr(struct vnode *vp, void *dtr_arg)
>   * Insert into list of vnodes for the new mount point, if available.
>   */
>  int
> -insmntque1(struct vnode *vp, struct mount *mp,
> -	void (*dtr)(struct vnode *, void *), void *dtr_arg)
> +insmntque(struct vnode *vp, struct mount *mp)
>  {
>  
>  	KASSERT(vp->v_mount == NULL,
> @@ -1974,8 +1973,6 @@ insmntque1(struct vnode *vp, struct mount *mp,
>  	    (vp->v_vflag & VV_FORCEINSMQ) == 0) {
>  		VI_UNLOCK(vp);
>  		MNT_IUNLOCK(mp);
> -		if (dtr != NULL)
> -			dtr(vp, dtr_arg);
And how is it supposed to work?

For insmntque(), the default destructor resets vnode vop vector to
deadfs_vops, before calling vgone(). Now consider any original user of
insmntque(), like msdosfs:

	nvp->v_data = ldep;
	...
	error = insmntque(nvp, mntp);
	if (error != 0) {
		free(ldep, M_MSDOSFSNODE);

Isn't ldep is now freed twice?

>  		return (EBUSY);
>  	}
>  	vp->v_mount = mp;
> @@ -1989,13 +1986,6 @@ insmntque1(struct vnode *vp, struct mount *mp,
>  	return (0);
>  }
>  
> -int
> -insmntque(struct vnode *vp, struct mount *mp)
> -{
> -
> -	return (insmntque1(vp, mp, insmntque_stddtr, NULL));
> -}
> -
>  /*
>   * Flush out and invalidate all buffers associated with a bufobj
>   * Called with the underlying object locked.
> diff --git a/sys/sys/param.h b/sys/sys/param.h
> index 948a34da94ec..6bf688105329 100644
> --- a/sys/sys/param.h
> +++ b/sys/sys/param.h
> @@ -76,7 +76,7 @@
>   * cannot include sys/param.h and should only be updated here.
>   */
>  #undef __FreeBSD_version
> -#define __FreeBSD_version 1400051
> +#define __FreeBSD_version 1400052
>  
>  /*
>   * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,
> diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h
> index a1dbdcff4cb5..390cb8791f54 100644
> --- a/sys/sys/vnode.h
> +++ b/sys/sys/vnode.h
> @@ -689,8 +689,6 @@ int	getnewvnode(const char *tag, struct mount *mp, struct vop_vector *vops,
>  	    struct vnode **vpp);
>  void	getnewvnode_reserve(void);
>  void	getnewvnode_drop_reserve(void);
> -int	insmntque1(struct vnode *vp, struct mount *mp,
> -	    void (*dtr)(struct vnode *, void *), void *dtr_arg);
>  int	insmntque(struct vnode *vp, struct mount *mp);
>  u_quad_t init_va_filerev(void);
>  int	speedup_syncer(void);