Differential D20944

powerpc64/mmu: Make moea64_pvo_enter() return if an entry already exists
ClosedPublic
Actions

Authored by jhibbits on Jul 13 2019, 3:16 AM.

Details

Reviewers

luporl
nwhitehorn
bdragon

Commits

rS350206: powerpc64/mmu: Make moea64_pvo_enter() return if an entry already exists

Summary

Instead of searching for a PVO entry before adding, take advantage of
the fact that RB_INSERT() returns NULL if it inserts, and the existing entry if
an entry exists, without inserting a new entry. This saves an extra tree
traversal in the cases where the PVO does not exist.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 25329
Build 23989: arc lint + arc unit

Event Timeline

jhibbits created this revision.Jul 13 2019, 3:16 AM

Herald added a subscriber: imp. · View Herald TranscriptJul 13 2019, 3:16 AM

Harbormaster completed remote builds in B25329: Diff 59719.Jul 13 2019, 3:16 AM

LGTM.

This revision is now accepted and ready to land.Jul 15 2019, 4:57 PM

Does this break PTE spills? This code path historically was involved in re-adding PVOs to the hashed page table on faults if they got spilled.

In D20944#454504, @nwhitehorn wrote:

Does this break PTE spills? This code path historically was involved in re-adding PVOs to the hashed page table on faults if they got spilled.

It shouldn't. I keep a "second chance" call in the case that a spill needs to take place. Is there an easy way to test that it doesn't break spills? I've been running this for about a month now with no problems.

Just make the page table size really small. There's a spill counter in the statistics; when it gets small enough, you should see it moving. If spill-handling doesn't work, and you have spills, everything breaks fast.

In D20944#454542, @nwhitehorn wrote:

Just make the page table size really small. There's a spill counter in the statistics; when it gets small enough, you should see it moving. If spill-handling doesn't work, and you have spills, everything breaks fast.

Heh, that's a reason to not completely remove the statistics counters ;)

I just tested with a *very* small page table (0x8000 entries on a 256GB machine), and hit the 'moea64_pte_insert: overflow' panic, which you had mentioned a few years back is indicative of the PTEGs being filled with wired entries. Checking the statistics, I do see 0x38 overflows, and other statistics look accurate, too. I don't see anything bad in this patch, because the only thing it does is optimistically try to insert the PVO, and go through the normal logic if it already exists.

OK, sounds good, thanks for testing!

Closed by commit rS350206: powerpc64/mmu: Make moea64_pvo_enter() return if an entry already exists (authored by jhibbits). · Explain WhyJul 22 2019, 3:12 AM

This revision was automatically updated to reflect the committed changes.

jhibbits added a commit: rS350206: powerpc64/mmu: Make moea64_pvo_enter() return if an entry already exists.