Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 08 Oct 2019 12:48:41 +0000
From:      "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net>
To:        "Hans Petter Selasky" <hselasky@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r353274 - in head/sys: net sys
Message-ID:  <E1765A16-B0DB-48D2-A25B-C494FB494B09@lists.zabbadoz.net>
In-Reply-To: <201910071415.x97EFfiN064058@repo.freebsd.org>
References:  <201910071415.x97EFfiN064058@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 7 Oct 2019, at 14:15, Hans Petter Selasky wrote:

> Author: hselasky
> Date: Mon Oct  7 14:15:41 2019
> New Revision: 353274
> URL: https://svnweb.freebsd.org/changeset/base/353274
>
> Log:
>   Factor out VNET shutdown check into an own vnet structure field.
>   Remove the now obsolete vnet_state field. This greatly simplifies 
> the
>   detection of VNET shutdown and avoids code duplication.

I think I tried to say that the vnet_state is extremely helpful for 
debugging as you know where each of the stacks is during 
initialisation/shutdown, especially with loadable  modules and that it 
should stay and I cannot remember that I removed it in the patch that I 
suggested.

I didn’t re-used a field but extended the structure.  What you did 
means you cannot MFC this easily.  Also it means that all previous vnet 
consumers got invalidated and the VNET_MAGIC_N should have been bumped 
and all modules need a re-compile.



>   Discussed with:	bz@
>   MFC after:	1 week
>   Sponsored by:	Mellanox Technologies
>
> Modified:
>   head/sys/net/if.c
>   head/sys/net/vnet.c
>   head/sys/net/vnet.h
>   head/sys/sys/param.h
>
> Modified: head/sys/net/if.c
> ==============================================================================
> --- head/sys/net/if.c	Mon Oct  7 13:40:29 2019	(r353273)
> +++ head/sys/net/if.c	Mon Oct  7 14:15:41 2019	(r353274)
> @@ -1088,10 +1088,9 @@ if_detach_internal(struct ifnet *ifp, int 
> vmove, struc
>   	struct ifnet *iter;
>   	int found = 0;
>  #ifdef VIMAGE
> -	int shutdown;
> +	bool shutdown;
>
> -	shutdown = (ifp->if_vnet->vnet_state > SI_SUB_VNET &&
> -		 ifp->if_vnet->vnet_state < SI_SUB_VNET_DONE) ? 1 : 0;
> +	shutdown = ifp->if_vnet->vnet_shutdown;
>  #endif
>  	IFNET_WLOCK();
>  	CK_STAILQ_FOREACH(iter, &V_ifnet, if_link)
> @@ -1341,7 +1340,6 @@ if_vmove_loan(struct thread *td, struct ifnet 
> *ifp, ch
>  {
>  	struct prison *pr;
>  	struct ifnet *difp;
> -	int shutdown;
>
>  	/* Try to find the prison within our visibility. */
>  	sx_slock(&allprison_lock);
> @@ -1369,9 +1367,7 @@ if_vmove_loan(struct thread *td, struct ifnet 
> *ifp, ch
>  	}
>
>  	/* Make sure the VNET is stable. */
> -	shutdown = (ifp->if_vnet->vnet_state > SI_SUB_VNET &&
> -		 ifp->if_vnet->vnet_state < SI_SUB_VNET_DONE) ? 1 : 0;
> -	if (shutdown) {
> +	if (ifp->if_vnet->vnet_shutdown) {
>  		CURVNET_RESTORE();
>  		prison_free(pr);
>  		return (EBUSY);
> @@ -1394,7 +1390,6 @@ if_vmove_reclaim(struct thread *td, char 
> *ifname, int
>  	struct prison *pr;
>  	struct vnet *vnet_dst;
>  	struct ifnet *ifp;
> - 	int shutdown;
>
>  	/* Try to find the prison within our visibility. */
>  	sx_slock(&allprison_lock);
> @@ -1423,9 +1418,7 @@ if_vmove_reclaim(struct thread *td, char 
> *ifname, int
>  	}
>
>  	/* Make sure the VNET is stable. */
> -	shutdown = (ifp->if_vnet->vnet_state > SI_SUB_VNET &&
> -		 ifp->if_vnet->vnet_state < SI_SUB_VNET_DONE) ? 1 : 0;
> -	if (shutdown) {
> +	if (ifp->if_vnet->vnet_shutdown) {
>  		CURVNET_RESTORE();
>  		prison_free(pr);
>  		return (EBUSY);
> @@ -2996,16 +2989,11 @@ ifioctl(struct socket *so, u_long cmd, caddr_t 
> data, s
>  	struct ifreq *ifr;
>  	int error;
>  	int oif_flags;
> -#ifdef VIMAGE
> -	int shutdown;
> -#endif
>
>  	CURVNET_SET(so->so_vnet);
>  #ifdef VIMAGE
>  	/* Make sure the VNET is stable. */
> -	shutdown = (so->so_vnet->vnet_state > SI_SUB_VNET &&
> -		 so->so_vnet->vnet_state < SI_SUB_VNET_DONE) ? 1 : 0;
> -	if (shutdown) {
> +	if (so->so_vnet->vnet_shutdown) {
>  		CURVNET_RESTORE();
>  		return (EBUSY);
>  	}
>
> Modified: head/sys/net/vnet.c
> ==============================================================================
> --- head/sys/net/vnet.c	Mon Oct  7 13:40:29 2019	(r353273)
> +++ head/sys/net/vnet.c	Mon Oct  7 14:15:41 2019	(r353274)
> @@ -235,7 +235,6 @@ vnet_alloc(void)
>  	SDT_PROBE1(vnet, functions, vnet_alloc, entry, __LINE__);
>  	vnet = malloc(sizeof(struct vnet), M_VNET, M_WAITOK | M_ZERO);
>  	vnet->vnet_magic_n = VNET_MAGIC_N;
> -	vnet->vnet_state = 0;
>  	SDT_PROBE2(vnet, functions, vnet_alloc, alloc, __LINE__, vnet);
>
>  	/*
> @@ -280,6 +279,9 @@ vnet_destroy(struct vnet *vnet)
>  	LIST_REMOVE(vnet, vnet_le);
>  	VNET_LIST_WUNLOCK();
>
> +	/* Signal that VNET is being shutdown. */
> +	vnet->vnet_shutdown = 1;
> +
>  	CURVNET_SET_QUIET(vnet);
>  	vnet_sysuninit();
>  	CURVNET_RESTORE();
> @@ -573,10 +575,8 @@ vnet_sysinit(void)
>  	struct vnet_sysinit *vs;
>
>  	VNET_SYSINIT_RLOCK();
> -	TAILQ_FOREACH(vs, &vnet_constructors, link) {
> -		curvnet->vnet_state = vs->subsystem;
> +	TAILQ_FOREACH(vs, &vnet_constructors, link)
>  		vs->func(vs->arg);
> -	}
>  	VNET_SYSINIT_RUNLOCK();
>  }
>
> @@ -592,10 +592,8 @@ vnet_sysuninit(void)
>
>  	VNET_SYSINIT_RLOCK();
>  	TAILQ_FOREACH_REVERSE(vs, &vnet_destructors, vnet_sysuninit_head,
> -	    link) {
> -		curvnet->vnet_state = vs->subsystem;
> +	    link)
>  		vs->func(vs->arg);
> -	}
>  	VNET_SYSINIT_RUNLOCK();
>  }
>
> @@ -709,7 +707,7 @@ db_vnet_print(struct vnet *vnet)
>  	db_printf(" vnet_data_mem  = %p\n", vnet->vnet_data_mem);
>  	db_printf(" vnet_data_base = %#jx\n",
>  	    (uintmax_t)vnet->vnet_data_base);
> -	db_printf(" vnet_state     = %#08x\n", vnet->vnet_state);
> +	db_printf(" vnet_shutdown  = %#08x\n", vnet->vnet_shutdown);
>  	db_printf("\n");
>  }
>
>
> Modified: head/sys/net/vnet.h
> ==============================================================================
> --- head/sys/net/vnet.h	Mon Oct  7 13:40:29 2019	(r353273)
> +++ head/sys/net/vnet.h	Mon Oct  7 14:15:41 2019	(r353274)
> @@ -72,7 +72,7 @@ struct vnet {
>  	u_int			 vnet_magic_n;
>  	u_int			 vnet_ifcnt;
>  	u_int			 vnet_sockcnt;
> -	u_int			 vnet_state;	/* SI_SUB_* */
> +	u_int			 vnet_shutdown; /* Shutdown in progress. */
>  	void			*vnet_data_mem;
>  	uintptr_t		 vnet_data_base;
>  };
>
> Modified: head/sys/sys/param.h
> ==============================================================================
> --- head/sys/sys/param.h	Mon Oct  7 13:40:29 2019	(r353273)
> +++ head/sys/sys/param.h	Mon Oct  7 14:15:41 2019	(r353274)
> @@ -60,7 +60,7 @@
>   *		in the range 5 to 9.
>   */
>  #undef __FreeBSD_version
> -#define __FreeBSD_version 1300048	/* Master, propagated to newvers */
> +#define __FreeBSD_version 1300049	/* Master, propagated to newvers */
>
>  /*
>   * __FreeBSD_kernel__ indicates that this system uses the kernel of 
> FreeBSD,



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E1765A16-B0DB-48D2-A25B-C494FB494B09>