Commit b8da46d3 authored by Nicolas Pitre's avatar Nicolas Pitre Committed by Linus Torvalds

clarify a usage constraint for cnt32_to_63()

The cnt32_to_63 algorithm relies on proper counter data evaluation
ordering to work properly. This was missing from the provided
documentation.

Let's augment the documentation with the missing usage constraint and
fix the only instance that got it wrong.
Signed-off-by: default avatarNicolas Pitre <nico@fluxnic.net>
Acked-by: default avatarDavid Howells <dhowells@redhat.com>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent de5e9d58
...@@ -40,21 +40,17 @@ unsigned long long sched_clock(void) ...@@ -40,21 +40,17 @@ unsigned long long sched_clock(void)
unsigned long long ll; unsigned long long ll;
unsigned l[2]; unsigned l[2];
} tsc64, result; } tsc64, result;
unsigned long tsc, tmp; unsigned long tmp;
unsigned product[3]; /* 96-bit intermediate value */ unsigned product[3]; /* 96-bit intermediate value */
/* cnt32_to_63() is not safe with preemption */ /* cnt32_to_63() is not safe with preemption */
preempt_disable(); preempt_disable();
/* read the TSC value /* expand the tsc to 64-bits.
*/
tsc = get_cycles();
/* expand to 64-bits.
* - sched_clock() must be called once a minute or better or the * - sched_clock() must be called once a minute or better or the
* following will go horribly wrong - see cnt32_to_63() * following will go horribly wrong - see cnt32_to_63()
*/ */
tsc64.ll = cnt32_to_63(tsc) & 0x7fffffffffffffffULL; tsc64.ll = cnt32_to_63(get_cycles()) & 0x7fffffffffffffffULL;
preempt_enable(); preempt_enable();
......
...@@ -61,13 +61,31 @@ union cnt32_to_63 { ...@@ -61,13 +61,31 @@ union cnt32_to_63 {
* *
* 2) this code must not be preempted for a duration longer than the * 2) this code must not be preempted for a duration longer than the
* 32-bit counter half period minus the longest period between two * 32-bit counter half period minus the longest period between two
* calls to this code. * calls to this code;
* *
* Those requirements ensure proper update to the state bit in memory. * Those requirements ensure proper update to the state bit in memory.
* This is usually not a problem in practice, but if it is then a kernel * This is usually not a problem in practice, but if it is then a kernel
* timer should be scheduled to manage for this code to be executed often * timer should be scheduled to manage for this code to be executed often
* enough. * enough.
* *
* And finally:
*
* 3) the cnt_lo argument must be seen as a globally incrementing value,
* meaning that it should be a direct reference to the counter data which
* can be evaluated according to a specific ordering within the macro,
* and not the result of a previous evaluation stored in a variable.
*
* For example, this is wrong:
*
* u32 partial = get_hw_count();
* u64 full = cnt32_to_63(partial);
* return full;
*
* This is fine:
*
* u64 full = cnt32_to_63(get_hw_count());
* return full;
*
* Note that the top bit (bit 63) in the returned value should be considered * Note that the top bit (bit 63) in the returned value should be considered
* as garbage. It is not cleared here because callers are likely to use a * as garbage. It is not cleared here because callers are likely to use a
* multiplier on the returned value which can get rid of the top bit * multiplier on the returned value which can get rid of the top bit
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment