Page MenuHomeFreeBSD

powerpc: Optimize copyinstr() to avoid repeatedly mapping user strings
ClosedPublic

Authored by jhibbits on Jun 4 2020, 6:20 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 3, 8:25 AM
Unknown Object (File)
Tue, Oct 1, 9:58 PM
Unknown Object (File)
Tue, Oct 1, 5:46 PM
Unknown Object (File)
Mon, Sep 30, 8:13 PM
Unknown Object (File)
Sat, Sep 21, 6:15 AM
Unknown Object (File)
Thu, Sep 5, 3:30 PM
Unknown Object (File)
Thu, Sep 5, 1:34 PM
Unknown Object (File)
Sep 1 2024, 9:03 AM
Subscribers
None

Details

Reviewers
nwhitehorn
bdragon
Group Reviewers
PowerPC
Summary

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.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 31510
Build 29114: arc lint + arc unit

Event Timeline

jhibbits created this revision.
sys/powerpc/powerpc/copyinout.c
269

nit: gets set to NULL again in done.

292

should this get moved to beginning of done: just in case the *done = t line faults?

sys/powerpc/powerpc/copyinout.c
269

Good point, will remove before commit.

292

This matches the fu* functions already, and any fault in the target address is a programming error. The API contract requires that done be either NULL or a valid address. No such requirement is made on the user side.

tl;dr; doesn't really matter one way or the other.

Address bdragon's feedback.

Now that the dependencies of this have landed:

RB800 ok
X5000 ok
AIM32 ok
AIM64 HPT ok
AIM64 Radix ok
QEMU BOOKE64 untested (I don't have a root disk at the moment)

Is there a good way to test the segment-crossing bits?

This revision is now accepted and ready to land.Jun 11 2020, 3:21 AM