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 Skipped
Unit
Tests Skipped
Build Status
Buildable 61591
Build 58475: arc lint + arc unit

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
50

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

53

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
34

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

43

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
19

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
201–202

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.