Date: Sat, 21 May 2022 18:58:39 +0400 From: Sergey Kandaurov <pluknet@gmail.com> To: Mark Johnston <markj@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: fcaa890c4469 - main - mbuf: Only allow extpg mbufs if the system has a direct map Message-ID: <CAE-mSOKV%2BxU%2BuCDGGwGtjecepC_yP9xeTE1kXF4U9a2%2BVKKRqw@mail.gmail.com> In-Reply-To: <202111161852.1AGIqhxc017891@gitrepo.freebsd.org> References: <202111161852.1AGIqhxc017891@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Tue, 16 Nov 2021 at 22:52, Mark Johnston <markj@freebsd.org> wrote: > The branch main has been updated by markj: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=fcaa890c4469118255d463495b4044eef484fa3e > > commit fcaa890c4469118255d463495b4044eef484fa3e > Author: Mark Johnston <markj@FreeBSD.org> > AuthorDate: 2021-11-16 18:31:04 +0000 > Commit: Mark Johnston <markj@FreeBSD.org> > CommitDate: 2021-11-16 18:31:04 +0000 > > mbuf: Only allow extpg mbufs if the system has a direct map > > Some upcoming changes will modify software checksum routines like > in_cksum() to operate using m_apply(), which uses the direct map to > access packet data for unmapped mbufs. This approach of course does > not > work on platforms without a direct map, so we have to disallow the use > of unmapped mbufs on such platforms. > > I believe this is the right tradeoff: we only configure KTLS on amd64 > and arm64 today (and one KTLS consumer, NFS TLS, requires a direct map > already), and the use of unmapped mbufs with plain sendfile is a recent > optimization. If need be, m_apply() could be modified to create > CPU-private mappings of extpg mbuf pages as a fallback. > > So, change mb_use_ext_pgs to be hard-wired to zero on systems without a > direct map. Note that PMAP_HAS_DMAP is not a compile-time constant on > some systems, so the default value of mb_use_ext_pgs has to be > determined during boot. > > Reviewed by: jhb > Discussed with: gallatin > MFC after: 2 weeks > Sponsored by: The FreeBSD Foundation > Differential Revision: https://reviews.freebsd.org/D32940 > --- > sys/kern/kern_mbuf.c | 32 ++++++++++++++++++++++++++++++-- > sys/rpc/rpcsec_tls/rpctls_impl.c | 2 +- > 2 files changed, 31 insertions(+), 3 deletions(-) > > diff --git a/sys/kern/kern_mbuf.c b/sys/kern/kern_mbuf.c > index d1f2fd2bd9e4..78a270189a4b 100644 > --- a/sys/kern/kern_mbuf.c > +++ b/sys/kern/kern_mbuf.c > @@ -116,9 +116,26 @@ int nmbjumbop; /* limits number > of page size jumbo clusters */ > int nmbjumbo9; /* limits number of 9k jumbo clusters */ > int nmbjumbo16; /* limits number of 16k jumbo > clusters */ > > -bool mb_use_ext_pgs = true; /* use M_EXTPG mbufs for sendfile & TLS */ > -SYSCTL_BOOL(_kern_ipc, OID_AUTO, mb_use_ext_pgs, CTLFLAG_RWTUN, > +bool mb_use_ext_pgs = false; /* use M_EXTPG mbufs for sendfile & TLS */ > Hi, Does it mean that mb_use_ext_pgs has to be enabled manually now in head and releng/13.1 ? (it was on by default in releng/13.0) I failed to see how it still can be on by default from this change. What about initializing to true under #if PMAP_HAS_DMAP ? > > + > +static int > +sysctl_mb_use_ext_pgs(SYSCTL_HANDLER_ARGS) > +{ > + int error, extpg; > + > + extpg = mb_use_ext_pgs; > + error = sysctl_handle_int(oidp, &extpg, 0, req); > + if (error == 0 && req->newptr != NULL) { > + if (extpg != 0 && !PMAP_HAS_DMAP) > + error = EOPNOTSUPP; > + else > + mb_use_ext_pgs = extpg != 0; > + } > + return (error); > +} > +SYSCTL_PROC(_kern_ipc, OID_AUTO, mb_use_ext_pgs, CTLTYPE_INT | CTLFLAG_RW, > &mb_use_ext_pgs, 0, > + sysctl_mb_use_ext_pgs, "IU", > "Use unmapped mbufs for sendfile(2) and TLS offload"); > > static quad_t maxmbufmem; /* overall real memory limit for all mbufs > */ > @@ -137,6 +154,7 @@ static void > tunable_mbinit(void *dummy) > { > quad_t realmem; > + int extpg; > > /* > * The default limit for all mbuf related memory is 1/2 of all > @@ -173,6 +191,16 @@ tunable_mbinit(void *dummy) > if (nmbufs < nmbclusters + nmbjumbop + nmbjumbo9 + nmbjumbo16) > nmbufs = lmax(maxmbufmem / MSIZE / 5, > nmbclusters + nmbjumbop + nmbjumbo9 + nmbjumbo16); > + > + /* > + * Unmapped mbufs can only safely be used on platforms with a > direct > + * map. > + */ > + if (PMAP_HAS_DMAP) { > + extpg = mb_use_ext_pgs; > + TUNABLE_INT_FETCH("kern.ipc.mb_use_ext_pgs", &extpg); > + mb_use_ext_pgs = extpg != 0; > + } > } > SYSINIT(tunable_mbinit, SI_SUB_KMEM, SI_ORDER_MIDDLE, tunable_mbinit, > NULL); [..] [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace"></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 16 Nov 2021 at 22:52, Mark Johnston <<a href="mailto:markj@freebsd.org">markj@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The branch main has been updated by markj:<br> <br> URL: <a href="https://cgit.FreeBSD.org/src/commit/?id=fcaa890c4469118255d463495b4044eef484fa3e" rel="noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=fcaa890c4469118255d463495b4044eef484fa3e</a><br>; <br> commit fcaa890c4469118255d463495b4044eef484fa3e<br> Author: Mark Johnston <markj@FreeBSD.org><br> AuthorDate: 2021-11-16 18:31:04 +0000<br> Commit: Mark Johnston <markj@FreeBSD.org><br> CommitDate: 2021-11-16 18:31:04 +0000<br> <br> mbuf: Only allow extpg mbufs if the system has a direct map<br> <br> Some upcoming changes will modify software checksum routines like<br> in_cksum() to operate using m_apply(), which uses the direct map to<br> access packet data for unmapped mbufs. This approach of course does not<br> work on platforms without a direct map, so we have to disallow the use<br> of unmapped mbufs on such platforms.<br> <br> I believe this is the right tradeoff: we only configure KTLS on amd64<br> and arm64 today (and one KTLS consumer, NFS TLS, requires a direct map<br> already), and the use of unmapped mbufs with plain sendfile is a recent<br> optimization. If need be, m_apply() could be modified to create<br> CPU-private mappings of extpg mbuf pages as a fallback.<br> <br> So, change mb_use_ext_pgs to be hard-wired to zero on systems without a<br> direct map. Note that PMAP_HAS_DMAP is not a compile-time constant on<br> some systems, so the default value of mb_use_ext_pgs has to be<br> determined during boot.<br> <br> Reviewed by: jhb<br> Discussed with: gallatin<br> MFC after: 2 weeks<br> Sponsored by: The FreeBSD Foundation<br> Differential Revision: <a href="https://reviews.freebsd.org/D32940" rel="noreferrer" target="_blank">https://reviews.freebsd.org/D32940</a><br>; ---<br> sys/kern/kern_mbuf.c | 32 ++++++++++++++++++++++++++++++--<br> sys/rpc/rpcsec_tls/rpctls_impl.c | 2 +-<br> 2 files changed, 31 insertions(+), 3 deletions(-)<br> <br> diff --git a/sys/kern/kern_mbuf.c b/sys/kern/kern_mbuf.c<br> index d1f2fd2bd9e4..78a270189a4b 100644<br> --- a/sys/kern/kern_mbuf.c<br> +++ b/sys/kern/kern_mbuf.c<br> @@ -116,9 +116,26 @@ int nmbjumbop; /* limits number of page size jumbo clusters */<br> int nmbjumbo9; /* limits number of 9k jumbo clusters */<br> int nmbjumbo16; /* limits number of 16k jumbo clusters */<br> <br> -bool mb_use_ext_pgs = true; /* use M_EXTPG mbufs for sendfile & TLS */<br> -SYSCTL_BOOL(_kern_ipc, OID_AUTO, mb_use_ext_pgs, CTLFLAG_RWTUN,<br> +bool mb_use_ext_pgs = false; /* use M_EXTPG mbufs for sendfile & TLS */<br></blockquote><div><br></div><div><div style="font-family:monospace,monospace" class="gmail_default">Hi,<br></div></div><div><br></div><div>Does it mean that mb_use_ext_pgs has to be enabled manually now</div><div>in head and releng/13.1 ? (it was on by default in releng/13.0)</div><div>I failed to see how it still can be on by default from this change.</div><br></div><div class="gmail_quote">What about initializing to true under #if PMAP_HAS_DMAP ?<br> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +<br> +static int<br> +sysctl_mb_use_ext_pgs(SYSCTL_HANDLER_ARGS)<br> +{<br> + int error, extpg;<br> +<br> + extpg = mb_use_ext_pgs;<br> + error = sysctl_handle_int(oidp, &extpg, 0, req);<br> + if (error == 0 && req->newptr != NULL) {<br> + if (extpg != 0 && !PMAP_HAS_DMAP)<br> + error = EOPNOTSUPP;<br> + else<br> + mb_use_ext_pgs = extpg != 0;<br> + }<br> + return (error);<br> +}<br> +SYSCTL_PROC(_kern_ipc, OID_AUTO, mb_use_ext_pgs, CTLTYPE_INT | CTLFLAG_RW,<br> &mb_use_ext_pgs, 0,<br> + sysctl_mb_use_ext_pgs, "IU",<br> "Use unmapped mbufs for sendfile(2) and TLS offload");<br> <br> static quad_t maxmbufmem; /* overall real memory limit for all mbufs */<br> @@ -137,6 +154,7 @@ static void<br> tunable_mbinit(void *dummy)<br> {<br> quad_t realmem;<br> + int extpg;<br> <br> /*<br> * The default limit for all mbuf related memory is 1/2 of all<br> @@ -173,6 +191,16 @@ tunable_mbinit(void *dummy)<br> if (nmbufs < nmbclusters + nmbjumbop + nmbjumbo9 + nmbjumbo16)<br> nmbufs = lmax(maxmbufmem / MSIZE / 5,<br> nmbclusters + nmbjumbop + nmbjumbo9 + nmbjumbo16);<br> +<br> + /*<br> + * Unmapped mbufs can only safely be used on platforms with a direct<br> + * map.<br> + */<br> + if (PMAP_HAS_DMAP) {<br> + extpg = mb_use_ext_pgs;<br> + TUNABLE_INT_FETCH("kern.ipc.mb_use_ext_pgs", &extpg);<br> + mb_use_ext_pgs = extpg != 0;<br> + }<br> }<br> SYSINIT(tunable_mbinit, SI_SUB_KMEM, SI_ORDER_MIDDLE, tunable_mbinit, NULL);<span class="gmail_default" style="font-family:monospace,monospace"></span></blockquote><div> <span class="gmail_default" style="font-family:monospace,monospace">[..]</span></div><div><br> </div></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAE-mSOKV%2BxU%2BuCDGGwGtjecepC_yP9xeTE1kXF4U9a2%2BVKKRqw>