[ntp:hackers] ntpd shm changes

Håkan Johansson f96hajo at chalmers.se
Fri Mar 18 23:31:40 UTC 2011


On Fri, 18 Mar 2011, Dave Hart wrote:

> [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.  [1]
>
> 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.

Yes, please.  Not having a fixed layout has hit us in the wild before:

http://lists.berlios.de/pipermail/gpsd-users/2009-November/004058.html

When anyhow at it, I'd advocate using a fixed endianess as well.  Network 
order, for htonl and ntohl.  Perhaps never useful on a single machine, but 
who knows?

> 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
> is cleared.

As ntpshm stands right now, both valid and count are needed. Note how gpsd 
also clears valid before the first count update, avoids the race pointed 
out by tz.  (Missing in gpsd is memory barriers for SMP.)

http://lists.berlios.de/pipermail/gpsd-dev/2009-November/007006.html

> 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?

Probably to avoid using the same sample twice?  But this can be checked by 
either count, or the time-stamp itself.  I also cannot see anything 
improved by having the reader clear valid.

> 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
> optimizations.
>
> 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?

One could use only count, if one in addition says that an odd count means 
'being updated' and an even count 'is valid'.  To allow for a previous 
crash during update, the increment before the update would ensure the 
count to be odd.

count++;
count |= 1;
... do updating ...
count++;

--

Also nice would be if the consumer (ntpd) would write some information of 
its current status (stratum, active source, ... - basically what is in a 
NTP network packet) to also allow the interface to be used for alternative 
transmission methods, such as PPS-like over a serial line.

Cheers,
Håkan

>
> Thanks for your time,
> Dave Hart
>
> [1]
> http://lists.ntp.org/pipermail/questions/2011-March/thread.html#28910
> or
> http://groups.google.com/group/comp.protocols.time.ntp/browse_thread/thread/72b460de0c825044?hl=en#
> _______________________________________________
> hackers mailing list
> hackers at lists.ntp.org
> http://lists.ntp.org/listinfo/hackers
>


More information about the hackers mailing list