Re: [PATCH] LinuxPPS - definitive version

[David Woodhouse]

On Tue, 2007-07-24 at 16:20 +0200, Rodolfo Giometti wrote:
> On Tue, Jul 24, 2007 at 02:49:02PM +0100, David Woodhouse wrote:
> 
> > Also 's/unknow /unknown /' (2 instances)
> 
> ?? I didn't find them:
> 
>    $ grep 'unknow ' Documentation/pps/pps.txt

Elsewhere in the patch.

> > In order for your handling of 'pps_source[source].info' to be safe with
> > respect to pps_unregister_source(), you have to guarantee that
> > pps_event() has finished -- and can't be in progress on another CPU --
> > by the time your client's call to pps_unregister_source() completes. At
> > first glance I think your existing clients have that right (you have
> > del_timer_sync() before pps_unregister_source() in ktimer.c, for
> > example). But you should make sure it's clearly documented for new
> > clients.
> 
> This can be done only with locks, but it's not necessary since even if
> a pps_unregister_source() runs while pps_event() executes on another
> CPU the latter will write always on a valid area (even if it could be
> a dummy one) and the data are not corrupted (note also that the data
> will be, in any case, discarted since we are executing a
> pps_unregister_source()).

Read Documentation/memory-barriers.txt

There is a tiny but possibly non-zero chance that one CPU could be in
pps_event() and might not yet have 'seen' the change to the .info field.
Releasing the pps_mutex provides a write-barrier on the CPU which runs
pps_unregister_source(), but there's no corresponding read-barrier on
the CPU running pps_event(). You have to be careful about when
pps_event() is run. It _MUST_ not touch the old info structure after
pps_unregister_source() has completed.

At the moment, I think it's OK because you won't be calling pps_event()
at the wrong times. But you do need to make sure that requirement is
documented. And I think you can remove the whole dummy_info thing
because it's not necessary.

-- 
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/