Date: Wed, 30 Dec 2020 16:58:26 -0600 From: "Brandon Bergren" <bdragon@FreeBSD.org> To: "Piotr Kubaj" <pkubaj@FreeBSD.org>, src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: =?UTF-8?Q?Re:_git:_6260bfb08742_-_main_-_powerpc:_Optimize_copyinstr()_t?= =?UTF-8?Q?o_avoid_repeatedly_mapping_user_strings?= Message-ID: <2c16d4dd-30e4-40ae-91f9-1936434dc922@www.fastmail.com> In-Reply-To: <202012302245.0BUMjrQN032417@gitrepo.freebsd.org> References: <202012302245.0BUMjrQN032417@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Approved-By: bdragon (in IRC) On Wed, Dec 30, 2020, at 4:45 PM, Piotr Kubaj wrote: > The branch main has been updated by pkubaj (ports committer): > > URL: > https://cgit.FreeBSD.org/src/commit/?id=6260bfb0874280d3fb58ac52699e2aee6ecca8a8 > > commit 6260bfb0874280d3fb58ac52699e2aee6ecca8a8 > Author: Justin Hibbits <chmeeedalf@gmail.com> > AuthorDate: 2020-06-04 18:15:15 +0000 > Commit: Piotr Kubaj <pkubaj@FreeBSD.org> > CommitDate: 2020-12-30 22:45:35 +0000 > > powerpc: Optimize copyinstr() to avoid repeatedly mapping user strings > > Currently copyinstr() uses fubyte() to read each byte from userspace. > However, this means that for each byte, it calls pmap_map_user_ptr() to > map the string into memory. This is needlessly wasteful, since the > string will rarely ever cross a segment boundary. Instead, map a > segment at a time, and copy as much from that segment as possible at a > time. > > Measured with the HPT pmap on powerpc64, this saves roughly 8% time on > buildkernel, and 5% on buildworld, in wallclock time. > --- > sys/powerpc/powerpc/copyinout.c | 46 +++++++++++++++++++++++++++++------------ > 1 file changed, 33 insertions(+), 13 deletions(-) > > diff --git a/sys/powerpc/powerpc/copyinout.c b/sys/powerpc/powerpc/copyinout.c > index 76965ad996b8..1528accc0e0e 100644 > --- a/sys/powerpc/powerpc/copyinout.c > +++ b/sys/powerpc/powerpc/copyinout.c > @@ -236,31 +236,51 @@ REMAP(copyin)(const void *udaddr, void *kaddr, size_t len) > int > REMAP(copyinstr)(const void *udaddr, void *kaddr, size_t len, size_t *done) > { > + struct thread *td; > + pmap_t pm; > + jmp_buf env; > const char *up; > - char *kp; > - size_t l; > - int rv, c; > + char *kp, *p; > + size_t i, l, t; > + int rv; > > - kp = kaddr; > - up = udaddr; > + td = curthread; > + pm = &td->td_proc->p_vmspace->vm_pmap; > > + t = 0; > rv = ENAMETOOLONG; > > - for (l = 0; len-- > 0; l++) { > - if ((c = fubyte(up++)) < 0) { > + td->td_pcb->pcb_onfault = &env; > + if (setjmp(env)) { > + rv = EFAULT; > + goto done; > + } > + > + kp = kaddr; > + up = udaddr; > + > + while (len > 0) { > + if (pmap_map_user_ptr(pm, up, (void **)&p, len, &l)) { > rv = EFAULT; > - break; > + goto done; > } > > - if (!(*kp++ = c)) { > - l++; > - rv = 0; > - break; > + for (i = 0; len > 0 && i < l; i++, t++, len--) { > + if ((*kp++ = *p++) == 0) { > + i++, t++; > + rv = 0; > + goto done; > + } > } > + > + up += l; > } > > +done: > + td->td_pcb->pcb_onfault = NULL; > + > if (done != NULL) { > - *done = l; > + *done = t; > } > > return (rv); > -- Brandon Bergren bdragon@FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2c16d4dd-30e4-40ae-91f9-1936434dc922>