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

On Thu, Jan 27, 2022 at 04:00:27AM +0200, Konstantin Belousov wrote:
> 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?
It is not double-free of ldep, but leaked nvp.  Still I do not understand
how it is supposed to work.

> 
> >  		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);