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

On 1/27/22, Konstantin Belousov <kostikbel@gmail.com> wrote:
> 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.
>

Huh, now that's weird. I could have sworn insmntque was calling
insmntque1 with dtr == NULL.

I'll be reverting this shortly.


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


-- 
Mateusz Guzik <mjguzik gmail.com>