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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/