Page MenuHomeFreeBSD

Move armv6 sysmaps to MD PCPU region
ClosedPublic

Authored by jah on Jan 14 2017, 4:18 AM.

Details

Summary

Similar to r310481 for i386, move the objects used to create temporary
mappings for armv6 pmap zero and copy operations to the MD PCPU region.
Change sysmap initialization to only allocate KVA pages for CPUs that
are actually present.

While here, collapse CMAP3 into CMAP2 (their use was mutually exclusive
anyway) and "recover" some space in PCPU padding that has always been
available due to 64-byte cacheline padding.

Diff Detail

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

Event Timeline

jah retitled this revision from to Move armv6 sysmaps to MD PCPU region.Jan 14 2017, 4:18 AM
jah updated this object.
jah edited the test plan for this revision. (Show Details)
jah updated this revision to Diff 23978.
skra added inline comments.Jan 14 2017, 9:18 AM
sys/arm/arm/pmap-v6.c
1576 ↗(On Diff #23978)

For consistency sake, the variable should be named cmap_pte2p. Or better, cmap2_pte2p in this case. However, the question is, if the variable is still needed. You already did not used it in pte2_clear().

The same comment about consistent naming of variable(s) is valid for other functions too.

1601 ↗(On Diff #23978)

Please, explain why you moved mtx_unlock() above sched_unpin(). IMO, it's not necessary and if mtx_unlock() causes that current thread is rescheduled, it remains pinned to some CPU. Thus, it can be scheduled only to that CPU, just to unpin itself.

You also did this change in other functions bellow.

sys/arm/include/pcpu.h
57 ↗(On Diff #23978)

I suggest to use the following names for new fields:

pc_cmaps_lock ... lock used for cmaps mappings
pc_cmap1_pte2p ... pte2 pointer for cmap1 mapping
pc_cmap2_pte2p ... pte2 pointer for cmap2 mapping
pc_cmap1_addr ... address (KVA) of cmap1 mapping
pc_cmap2_addr ... address (KVA) of cmap2 mapping

We strictly distinguish pt1_entry_t from pt2_entry_t and value from pointer. I.e.:

pt1_entry_t -> pte1
pt1_entry_t* -> pte1p
pt2_entry_t -> pte2
pt2_entry_t* -> pte2p

Using pte1 name for pt2_entry_t should be avoided.

jah added inline comments.Jan 14 2017, 9:43 AM
sys/arm/arm/pmap-v6.c
1576 ↗(On Diff #23978)

I should be using it in pte2_clear() below...I'll change that.

As for the naming, these names were taken from the i386 implementation where we don't have names like pt1_entry_t and pt2_entry_t to distinguish L1 and L2 PTEs. I'm fine with renaming the armv6 version according to your suggestion.

1601 ↗(On Diff #23978)

Good catch, this came from an earlier version of the change in which I referenced the lock directly using PCPU_GET. It's no longer necessary and potentially harmful to performance as you point out.

jah updated this revision to Diff 24005.Jan 14 2017, 8:54 PM

Return sched_unpin() to original order, rename fields

skra edited edge metadata.Jan 15 2017, 8:37 AM
skra accepted this revision.

Thanks.

sys/arm/include/pcpu.h
57 ↗(On Diff #24005)

Style (stray TAB).

This revision is now accepted and ready to land.Jan 15 2017, 8:37 AM
jah marked 3 inline comments as done.Jan 22 2017, 12:46 AM
This revision was automatically updated to reflect the committed changes.