[ntp:hackers] ntpd shm changes
hart at ntp.org
Fri Mar 18 17:34:17 UTC 2011
[cross-posted to hackers at ntp and gpsd-dev@ by Dave Hart]
Bruce Lilly has done some work to develop a POSIX named shared memory
alternative to ntpd's refclock_shm.c / driver 28, which uses SysV shm
"named" using 32 bits, currently including the ascii for NTP (or ntp)
and a unit digit. See "new driver development" thread. 
If ntpd is to grow the flavors of shared memory it understands to
include POSIX named shm, which would require a new driver or an
extension of the current refclock_shm.c, I think we should take the
opportunity presented to clean up the shared memory interface
contract. We could use the existing layout but since it requires new
code in ntpd and producers like gpsd to move to the different shm
namespace, there's no existing code to offended by a reshuffling of
layout and/or protocol.
Today's mode 1 approach is pretty good. As I mentioned on questions@
I would prefer fixed sizes so the interface is resilient in the face
of a 64-bit ntpd and a 32-bit gpsd or vice-versa.
Additionally, I noticed the comments in our driver are one-sided,
describing only instructions for ntpd's interaction with the shared
memory without spelling out the writer's contract. Harlan raised a
concern about writers getting it wrong, and it took me a few guesses
to infer the right steps for a writer before resorting to gpsd source
code to to confirm. In short, writers increment count, then the
payload data, then increment count again, then set valid. Readers
fail if valid is zero, otherwise store count, copy payload, compare
count, fail if they don't match. Whether count matches or not, valid
I'm not sure valid is achieving anything. Readers don't read when
it's zero, but that seems redundant to me with the count checking.
Also I don't like the reader resetting valid to 0 -- that shuts down
the possibility of multiple readers, and for what gain? valid is
cleared even when the reader abandoned the attempt because it was
mid-update, which seems particularly ill-advised to me.
There are missing volatile qualifiers, as far as I see, the shared
memory should be accessed solely through volatile pointers, as it is
being modified by code outside the scope of the compiler's
To facilitate correctness and avoid duplication of code, I'd like to
see public header file lay out the structure and provide inline code
for producer and consumer. That would be eased by having not just one
structure, but a payload structure used by the producer and consumer
and a container structure with mode, count, and if needed, valid along
with an embedded payload struct.
Do you believe we need both valid and count? Can you think of other
changes or extensions that might be an improvement in some way?
Thanks for your time,
More information about the hackers