Page MenuHomeFreeBSD

clock: Add a long ticks variable
ClosedPublic

Authored by markj on Wed, Jan 8, 11:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jan 20, 6:44 AM
Unknown Object (File)
Thu, Jan 16, 6:27 AM
Unknown Object (File)
Wed, Jan 15, 3:51 AM
Unknown Object (File)
Mon, Jan 13, 8:14 PM
Unknown Object (File)
Sun, Jan 12, 8:29 PM
Unknown Object (File)
Sun, Jan 12, 3:16 AM
Unknown Object (File)
Sat, Jan 11, 2:42 PM
Unknown Object (File)
Sat, Jan 11, 2:36 PM
Subscribers

Details

Summary

For compatibility with Linux, it's useful to have a 64-bit tick counter.
Get us most of the way there by defining a ticksl variable that gives
the desired behaviour on 64-bit systems.

Follow a suggestion from kib to avoid having to maintain two separate
counters and to avoid converting existing code to use ticksl: use
assembler directives to make ticks and ticksl overlap such that loading
ticks gives the bottom 32 bits.

XXX-MJ tested on x86 only for now, I will add other platforms if there
are no objections to this.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj requested review of this revision.Wed, Jan 8, 11:06 PM

Update tc_ticktock() as well. It's not necessary, but this way
we avoid some silent truncation that might confuse readers.

Rename "i" to something a bit better and reduce its scope.

Reorder asm directives a bit.

sys/amd64/amd64/support.S
49 ↗(On Diff #148952)

it is confusing, but .long is same as .int

52 ↗(On Diff #148952)

I think you would want to move this fragment into a dedicated source, to avoid copying it to each arch locore.

I am not sure what guarantees are provided by e.g. machine/endian.h, but ideally it would allow to handle both big- and little-endian in single source.

markj marked 2 inline comments as done.

Add a header to define ticks. I am happy to put it in an existing
header but I'm not sure which one would be suitable.

Use the .quad directive when sizeof(ticksl) == 8.

Restore the initialization of ticksl on 64-bit platforms. It's still useful for
testing the case where some upper bits are non-zero.

sys/sys/ticks.h
33 ↗(On Diff #148958)

this line is invariant for all configurations, it can be moved into DEFINE_TICKS

42 ↗(On Diff #148958)

Just my personal opinion there:

  • I hate multiline C preprocessor macros in asm, the native .macro is much more pleasent in .s
  • I would go with kern/subr_tick.s instead of including into locore.S
sys/sys/ticks.h
18 ↗(On Diff #148958)

I also think that the .p2align 3 directive is better to be added before ticksl.

sys/sys/kernel.h
74–75

I would perhaps add in the last sentence a reminder that even using ticksl doesn't mean that rollover can be ignored, as long as we support 32-bit platforms. This is implicit from the rest of the text, but I feel its current form could induce the reader to think that using ticksl is always safe. Perhaps it's because of the juxtaposition of Either can be used; 32-bit values (...) but rollover must be handled.

markj marked 4 inline comments as done.

Apply reviewer suggestions, move symbol definitions to sys/kern/subr_ticks.s.

Make definitions compatible with arm assembler syntax.

Simplify the comment describing ticks/ticksl, be less prescriptive.

This revision is now accepted and ready to land.Thu, Jan 9, 5:35 PM
sys/kern/subr_param.c
200

Arguably this is still going to catch ticks (non-l) wrapping on 64-bit platform too.

sys/sys/kernel.h
72–73

"either value can be used, but rollover must be handled" gives the impression that rollover must be handled with either value

markj marked 2 inline comments as done.

Adjust comments.

This revision now requires review to proceed.Thu, Jan 9, 6:49 PM
This revision is now accepted and ready to land.Thu, Jan 9, 6:51 PM
This revision was automatically updated to reflect the committed changes.