Move armv6 sysmaps to MD PCPU region
ClosedPublic
Actions

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

Details

Reviewers

andrew
mmel
skra

Commits

rS312610: Like r310481 for i386, move the objects used to create temporary

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 - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jah updated this revision to Diff 23978.Jan 14 2017, 4:18 AM

jah retitled this revision from to Move armv6 sysmaps to MD PCPU region.

jah updated this object.

jah edited the test plan for this revision. (Show Details)

Herald added subscribers: andrew, imp. · View Herald TranscriptJan 14 2017, 4:18 AM

jah added reviewers: skra, mmel, andrew.Jan 14 2017, 4:21 AM

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.