Page MenuHomeFreeBSD

Address the COW and PV entry allocation problems in pmap_enter() on RISCV
ClosedPublic

Authored by alc on Jul 13 2018, 7:29 PM.

Details

Summary

The title mostly says it all. The exception being that I introduced a missing atomic clear operation of the PGA_WRITEABLE flag when the PV list is emptied.

Test Plan

Get Mark's help.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Here is our current state:

                Uses            Has PV Alloc    Has COW         Needs r324665
                pv_chunk        Problem         Bug             and r325285
                -------------------------------------------------------------
amd64           Yes             Fixed           Fixed           Fixed
arm/pmap-v4.c   No              N/A             No[1]           N/A
arm/pmap-v6.c   Yes             No              No[2]           No[4]
arm64           Yes             Fixed           No[2]           Yes
i386            Yes             No              Fixed           No[4]
mips            Yes             No              Fixed           No[4]
powerpc/booke   No              N/A             No[2]           N/A
powerpc/oea     No              N/A[3]          No[2]           N/A
powerpc/oea64   No              N/A             No[2]           N/A
powerpc/pseries[5]
riscv           Yes             This            This            No[4]
sparc64         No              N/A             No[2]           N/A

[1] SMP is not supported.
[2] Performs "break-before-make".
[3] The comments say that the PV entry is reused, but it is not.  That said,
    the old PV entry is freed before the new one is allocated.  I believe
    that reuse could be beneficial because it would eliminate two O(log n)
    Red-Black tree operations.
[4] Still has pvh_global_lock.
[5] Literally derived from powerpc/oea64.

I had no problems booting in qemu and running some commands with this change.

riscv/riscv/pmap.c
2164 ↗(On Diff #45257)

Any reason not to include the "Temporarily invalidate ..." comment that's in some other pmaps?

This revision is now accepted and ready to land.Jul 14 2018, 5:05 PM
riscv/riscv/pmap.c
2164 ↗(On Diff #45257)

Nope. I will add it.

This revision now requires review to proceed.Jul 14 2018, 5:50 PM
alc marked 2 inline comments as done.Jul 14 2018, 5:51 PM
This revision is now accepted and ready to land.Jul 14 2018, 7:30 PM

I also booted multiuser in QEMU with this change and it works fine. Thank you

This revision was automatically updated to reflect the committed changes.