Use of barriers in pvclock ABI
In Xen, we guarantee that the pv clock record will only ever be updated by the current cpu, so there will never be any cross-cpu barrier or synchronization issues to consider, nor any chance a vcpu will see its own clock record in a partially updated state.
I notice the current kvm implementation is the same.
However, the pvclock_clocksource_read() implementation is over-engineered, because it checks for an odd version and uses very strong rmb() barriers (which generates either an "lfence" or "lock add $0, (%esp)").
If we're happy to guarantee as an ABI issue that the record will never be updated cross-cpu, then we can make the barriers simply barrier() and just check for (src->version != dst->version).
Is that OK with you, or do you want to leave open the possibility of doing cross-cpu time updates?
J
Hi,
Quoted 11 lines
However, the
pvclock_clocksource_read()implementation is over-engineered, because it checks for an odd version and uses very strongrmb()barriers (which generates either an "lfence" or "lock add $0, (%esp)").If we're happy to guarantee as an ABI issue that the record will never be updated cross-cpu, then we can make the barriers simply
barrier()and just check for (src->version!=dst->version).Is that OK with you, or do you want to leave open the possibility of doing cross-cpu time updates?
Due to the TSC being involved here I don't expect cross-cpu time updates will ever happen. IMHO it is fine to change that.
cheers,
Gerd
Signature · 1 line
http://kraxel.fedorapeople.org/xenner/
Quoted 12 lines · Gerd Hoffmann
Hi,
Quoted 9 lines
However, the
pvclock_clocksource_read()implementation is over-engineered, because it checks for an odd version and uses very strongrmb()barriers (which generates either an "lfence" or "lock add $0, (%esp)").If we're happy to guarantee as an ABI issue that the record will never be updated cross-cpu, then we can make the barriers simply
barrier()and just check for (src->version!=dst->version).
Quoted 8 lines
Quoted 3 lines
Is that OK with you, or do you want to leave open the possibility of doing cross-cpu time updates?
Due to the TSC being involved here I don't expect cross-cpu time updates will ever happen. IMHO it is fine to change that.
I agree. And if we ever feel the need, we can allocate a feature bit for it.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 11 lines · Gerd Hoffmann
Hi,
Quoted 9 lines
However, the
pvclock_clocksource_read()implementation is over-engineered, because it checks for an odd version and uses very strongrmb()barriers (which generates either an "lfence" or "lock add $0, (%esp)").If we're happy to guarantee as an ABI issue that the record will never be updated cross-cpu, then we can make the barriers simply
barrier()and just check for (src->version!=dst->version).
Quoted 5 lines
Quoted 2 lines
Is that OK with you, or do you want to leave open the possibility of doing cross-cpu time updates?
Due to the TSC being involved here I don't expect cross-cpu time updates will ever happen. IMHO it is fine to change that.
Okay for guest vcpu, but what about physical cpus?
IIRC, the checks are there, and so strict, to account for the possiblity of the vcpu to be migrated to another cpu in the middle of the
clock reading.
Quoted 7 lines
cheers,
Gerd
Signature · 5 lines
Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act."
Quoted 21 lines · Glauber Costa
Quoted 12 lines · Gerd Hoffmann
Hi,
Quoted 9 lines
However, the
pvclock_clocksource_read()implementation is over-engineered, because it checks for an odd version and uses very strongrmb()barriers (which generates either an "lfence" or "lock add $0, (%esp)").If we're happy to guarantee as an ABI issue that the record will never be updated cross-cpu, then we can make the barriers simply
barrier()and just check for (src->version!=dst->version).Quoted 6 lines
Quoted 3 lines
Is that OK with you, or do you want to leave open the possibility of doing cross-cpu time updates?
Due to the TSC being involved here I don't expect cross-cpu time updates will ever happen. IMHO it is fine to change that.
Quoted 7 lines
Okay for guest vcpu, but what about physical cpus?
IIRC, the checks are there, and so strict, to account for the possiblity of the vcpu to be migrated to another cpu in the middle of the
Migration implies an smp barrier (but not a compiler barrier, of course). As to the the version number oddness check, I don't see how it can be necessary.
Signature · 1 line
error compiling committee.c: too many arguments to function
Quoted 10 lines · Glauber Costa
Quoted 2 lines · Gerd Hoffmann
Due to the TSC being involved here I don't expect cross-cpu time updates will ever happen. IMHO it is fine to change that.
Okay for guest vcpu, but what about physical cpus?
IIRC, the checks are there, and so strict, to account for the possiblity of the vcpu to be migrated to another cpu in the middle of the
clock reading.
This is about the check in pvclock_get_time_values() that it got a consistent snapshot. Dropping that is fine.
pvclock_clocksource_read() will still notice when being migrated to another pcpu in the middle of the clock reading.
cheers,
Gerd
Signature · 1 line
http://kraxel.fedorapeople.org/xenner/
Quoted 7 lines · Glauber Costa
Okay for guest vcpu, but what about physical cpus?
IIRC, the checks are there, and so strict, to account for the possiblity of the vcpu to be migrated to another cpu in the middle of the
clock reading.
That's fine. As part of rescheduling a vcpu on a new pcpu, the clock record will be updated with the new cpu's parameters, but that update will be complete by the time the vcpu gets rescheduled. The version check and loop still needs to be there, but it will never see an inconsistent (partially updated) clock record.
J