Re: git: ef6fcc5e2b07 - main - nfsd: Add VNET_SYSUNINIT() macros for vnet cleanup

On Tue, Feb 21, 2023 at 1:13 PM Gleb Smirnoff <glebius@freebsd.org> wrote:
>
> CAUTION: This email originated from outside of the University of Guelph. Do not click links or open attachments unless you recognize the sender and know the content is safe. If in doubt, forward suspicious emails to IThelp@uoguelph.ca
>
>
> On Mon, Feb 20, 2023 at 09:12:41PM +0000, Rick Macklem wrote:
> R>     nfsd: Add VNET_SYSUNINIT() macros for vnet cleanup
> R>
> R>     Commit ed03776ca7f4 enabled the vnet front end macros.
> R>     As such, for kernels built with the VIMAGE option will malloc
> R>     data and initialize locks on a per-vnet basis, typically
> R>     via a VNET_SYSINIT().
> R>
> R>     This patch adds VNET_SYSUNINIT() macros to do the frees
> R>     of the per-vnet malloc'd data and destroys of per-vnet
> R>     locks.  It also removes the mtx_lock/mtx_unlock calls
> R>     from nfsrvd_cleancache(), since they are not needed.
>
> In the netinet/netinet6/TCP/UDP we came to a style where we avoid
> using IS_DEFAULT_VNET(). Instead doing global and per-vnet init
> in the same function:
>
> static globalfoo;
> VNET_DEFINE_STATIC(foobar);
>
> static void
> foo_vnet_init()
> {
>         initialize(V_foobar);
>         if (IS_DEFAULT_VNET(curvnet))
>                 initialize(globalfoo);
>
> }
> VNET_SYSINIT(foo_vnet_init, ....)
>
> We can do a separate init of global state and separate of per-VNET:
>
> static globalfoo;
> static void
> foo_init()
> {
>         initialize(globalfoo);
> }
> SYSINIT(foo_init, ....)
>
> VNET_DEFINE_STATIC(foobar);
> static void
> foo_vnet_init()
> {
>         initialize(V_foobar);
> }
>
> This allows to:
>
> * guarantee that global state is initialized earlier than per-vnet
> * separate all global vars from all vnet vars, and keep them together, easier to maintain
> * makes it easier to write VNET_SYSUNINIT() that is complement to VNET_SYSINIT()
> * makes it easier to write SYSUNINIT(), if module is unloadable
> * sometimes global SYSINIT cab be avoided, if a static initializer is enough
>
> What do you guys think?
>
I did that for one of the ones related to nfsd (sys/fs/nfsserver/nfs_fha_new.c).
It so happens it doesn't work for this case.  If you look at the patch, you'll
see that the variable is vnet'd. It happens that it is only malloc'd for prisons
other than prison0. (For prison0, it points to a global structure that is not
malloc'd and is shared with the NFS client, which is not vnet'd.

rick

> --
> Gleb Smirnoff
>