Date: Wed, 22 Feb 2023 22:28:53 -0800 From: Gleb Smirnoff <glebius@freebsd.org> To: Rick Macklem <rick.macklem@gmail.com> Cc: Rick Macklem <rmacklem@freebsd.org>, bz@freebsd.org, jamie@freebsd.org, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: ef6fcc5e2b07 - main - nfsd: Add VNET_SYSUNINIT() macros for vnet cleanup Message-ID: <Y/cHpcn/1S3Q0Swa@FreeBSD.org> In-Reply-To: <CAM5tNy6KFzCarKWze4mjbnqsERUoshHFfZeXzZMF3RurVKf7TA@mail.gmail.com> References: <202302202112.31KLCfQB080359@gitrepo.freebsd.org> <Y/Uz5B6ywWjaa1TB@FreeBSD.org> <CAM5tNy6KFzCarKWze4mjbnqsERUoshHFfZeXzZMF3RurVKf7TA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Rick, On Tue, Feb 21, 2023 at 07:29:30PM -0800, Rick Macklem wrote: R> I did that for one of the ones related to nfsd (sys/fs/nfsserver/nfs_fha_new.c). This one actually doesn't look correct to me. What happens here is that the sysctls will affect only the default VNET. I think the VNET-itezation of this file went a bit wrong. You don't need to convert static structures to malloced when you VNET-ize a module. The infrastructure should take care of memory management. The dynamic sysctl context seems to be unneeded from the very beginning. It was always attached to the static softc. Suggested patch: https://reviews.freebsd.org/D38742 It gets rid of one case of IS_DEFAULT_VNET. Tested very little: run two jails and observed difference in sysctl. R> It so happens it doesn't work for this case. If you look at the patch, you'll R> see that the variable is vnet'd. It happens that it is only malloc'd for prisons R> other than prison0. (For prison0, it points to a global structure that is not R> malloc'd and is shared with the NFS client, which is not vnet'd. I think all of this instances can be fixed. For example nfsstatsv1, which is now malloced for non-default vnets and static for the default. Lots of modules still incorrectly update the global one. This patch makes it right: https://reviews.freebsd.org/D38743 Tested very little: run two jails, use some NFS, observe difference in nfsstat. Removes another IS_DEFAULT_VNET. Note that D38743 is still far from perfect. NFS would benefit a lot if used macros for per-VNET per-CPU (raceless!) stats, that are used in TCP/IP. Sorry, they aren't documented :( You can find them in vnet.h starting from VNET_PCPUSTAT_DECLARE() and look for example usage in TCP/IP. Please fill free to commandeer those reviews from me and finalize them if you agree with general idea. -- Gleb Smirnoff
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Y/cHpcn/1S3Q0Swa>