Re: svs_pmap_sync() svs_pdir_switch() overhead

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 24/05/2020 à 23:49, Andrew Doran a écrit :
> On Sun, May 24, 2020 at 09:42:33PM +0000, Andrew Doran wrote:
> > On Sat, May 23, 2020 at 09:04:26AM +0200, Maxime Villard wrote:
> > > Le 20/05/2020 ? 11:12, Manuel Bouyer a ?crit?:
> > > > On Tue, May 19, 2020 at 10:51:52PM +0000, Andrew Doran wrote:
> > > > > Both of these show up prominently in profiling for me.  This change largely
> > > > > cures it:
> > > > >
> > > > > 	http://www.netbsd.org/~ad/2020/svs.diff
> > > > >
> > > > > Comments?
> > >
> > > I didn't know kcpuset_isotherset() existed, that is indeed better.
> > >
> > > Not sure the second part is correct though. Suppose:
> > >
> > > cpu0 is executing svs_pdir_switch(), and cpu1 is modifying the PTEs at the
> > > same time. cpu0 returns to userland before cpu1 finished. [XXX]. cpu1
> > > finishes, and calls svs_pmap_sync().
> > >
> > > In the [XXX] window, the PTEs could be used by userland. If you copied
> > > them using memcpy(), some parts of the bytes could contain stale values.
> >
> > You mean if memcpy() was moving single bytes at a time?  That won't happen,
> > see:
> >
> > https://nxr.netbsd.org/xref/src/common/lib/libc/arch/x86_64/string/bcopy.S
> >
> > Cheers,
> > Andrew
>
> Ah I see the compiler is up to its tricks here, so the assumption is not
> quite so safe, and despite usually being very clever it's memset is daft.
> The removal of bcopy(), bcmp() and bzero() was a mistake.  Anyway I'll do
> it differently.

Overall I think this reasoning is bad. If you want to have stuff copied in
an atomic-safe manner, then use an atomic function; don't rely on some wild
assumptions that just won't hold after the next compiler update.

Besides, bcopy and the like do not guarantee that they do operations on
quads, so using bcopy wouldn't have been more correct than memcpy.