Page MenuHomeFreeBSD

powerpc: unconditionally mark SLB zones UMA_ZONE_CONTIG
ClosedPublic

Authored by rlibby on Feb 17 2020, 6:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 13 2024, 12:07 AM
Unknown Object (File)
Feb 7 2024, 5:41 PM
Unknown Object (File)
Jan 13 2024, 3:22 AM
Unknown Object (File)
Dec 23 2023, 2:37 AM
Unknown Object (File)
Dec 22 2023, 11:49 PM
Unknown Object (File)
Dec 9 2023, 12:30 PM
Unknown Object (File)
Dec 9 2023, 11:54 AM
Unknown Object (File)
Nov 19 2023, 1:26 PM

Details

Summary

PR: 244118
Reported by: Francis Little
Tested by: Francis Little, Mark Millard

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Can you explain why this makes a difference, at least for slbt_zone? I think I am missing something.

Can you explain why this makes a difference, at least for slbt_zone? I think I am missing something.

I can report observed behavior both ways:

Without removing the conditional structure the old PowerMac G5 "Quads"
do not boot. With the removal they do. The booting has been broken since
head -r357549 when the conditional allocf_flags code structure was put in.

See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=244118 .

I'm now running with the patch from that bugzilla and have progressed to
-r357979 with it.

Can you explain why this makes a difference, at least for slbt_zone? I think I am missing something.

To be honest, I'm not sure that it does matter for slbt_zone, and my reasoning was just that we already use the contig allocator for the platform_real_maxaddr() != VM_MAX_ADDRESS case, so perhaps it matters? I'm not sure if that is purposeful or if it may be partially an accident from wanting to use the maxaddr constraint for that zone.

The slbt_zone has slbtnode objects, which seem to contain embedded slb objects (slb_entries). I think but I'm not sure that this object is where the constraints are coming from.

In practice this doesn't have an effect because the zone already met the minimum efficiency and I think because the object size made it so that the struct slbs wouldn't break across pages already.

If there's someone who knows this area of code better and can describe the actual constraints, we can try to tune it more finely. I think if it's overconstrained, then it's at least safe.

For slb_cache_zone, I think the relevant code is moea64_activate().

Can you explain why this makes a difference, at least for slbt_zone? I think I am missing something.

I can report observed behavior both ways:

Without removing the conditional structure the old PowerMac G5 "Quads"
do not boot. With the removal they do. The booting has been broken since
head -r357549 when the conditional allocf_flags code structure was put in.

See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=244118 .

I'm now running with the patch from that bugzilla and have progressed to
-r357979 with it.

So, my understanding is that the change for slb_cache_zone is necessary (demonstrated by testing), and whether the change is necessary (in principle) for slbt_zone is not clear.

The issue in general is whether it is okay for objects from these zones to be backed by physical memory that is not contiguous, which UMA multipage slabs may cause unless the UMA_ZONE_CONTIG flag is specified or uma_zone_set_allocf is used.

Note that this is not a revert of the change in r357548, this is adding the UMA_ZONE_CONTIG flag unconditionally.

The issue in general is whether it is okay for objects from these zones to be backed by physical memory that is not contiguous, which UMA multipage slabs may cause unless the UMA_ZONE_CONTIG flag is specified or uma_zone_set_allocf is used.

Note that this is not a revert of the change in r357548, this is adding the UMA_ZONE_CONTIG flag unconditionally.

Ah, that clarifies things. I was looking for a problem in r357548.

This revision is now accepted and ready to land.Feb 17 2020, 9:07 PM

. . .> Ah, that clarifies things. I was looking for a problem in r357548.

FYI: My testing included a -r357548 kernel on the G5 Quad and it worked fine.
It was specifically -r357549 where things quit working.