On Mon, Jul 30, 2007 at 09:49:20AM +0530, Satyam Sharma wrote: > > Hmm? I still don't see why you can't introduce spin_lock_irqsave/restore() > in pps_event() around the access to pps_source.
In pps_event() is not useful using spin_lock_irqsave/restore() since the only difference between spin_lock_irqsave() and spin_lock() is that the former will turn off interrupts if they are on, otherwise does nothing (if we are already in an interrupt handler). Maybe you meant I should using spin_lock_irqsave/restore() in user context, but doing like this I will disable interrupts and I don't wish doing it since, in this manner, the interrupt handler will be delayed and the (probably) PPS event recording will be wrong. I prefere loosing the event that registering it at delayed time. > > About using both mutex and spinlock I did it since (I think) I should > > protect syscalls from each others and from pps_register/unregister(), > > and pps_event() against pps_register/unregister(). > > Nopes, it's not about protecting code from each other, you're needlessly > complicating things. Locking is pretty simple, really -- any shared data, > that can be concurrently accessed by multiple threads (or from interrupts) > must be protected with a lock. Note that *data* is protected by a lock, > and not "code" that handles it (well, this is the kind of behaviour most > cases need, at least, including yours). Of course, I meant "protecting data". In fact to protect pps_source[] I need spin_lock() to protect user context from interrupt context and mutex to protect user context from itself. > So here we're introducing the lock to protect *pps_source*, and not keep > *threads* of execution from stepping over each other. So, simply, just > ensure you grab the lock whenever you want to start accessing the shared > data, and release it when you're done. I see. But consider pps_register_source(). This function should provide protection of pps_source against both interrupt context (pps_event()) and user context (maybe pps_unregister_source() or one syscalls). Using only mutex is not possible, since we cannot use mutex in interrupt context, and using only spin_locks is not possible since in UP() they became void. Can you please show me how I could write pps_register_source() in order to be correct from your point of view? > The _irqsave/restore() variants are required because (say) one of the > syscalls executing in process context grabs the spinlock. Then, before it > has released it, it gets interrupted and pps_event() begins executing. > Now pps_event() also wants to grab the lock, but the syscall already > has it, so will continue spinning and deadlock! That's the point. I don't wish using _irqsave/restore() since they may delay interrupt handler execution. As above, I prefere loosing the event then registering it at wrong time. > I think you're unnecessarily worrying about contention here -- you can > have multiple locks (one for the list, and separate ones for your sources) > if you're really worrying about contention -- or probably rwlocks. But > really, rwlocks would end up being *slower* than spinlocks, unless the > contention is really heavy and it helps to keep multiple readers in the > critical section. But frankly, with at max a few (I'd expect generally > one) PPS sources ever to be connected / registered with teh system, and > just one-pulse-per-second, I don't see why any contention is ever gonna > happen. Why you wish using one lock per sources? Just one lock for the list/array is not enought? :-o Ciao, Rodolfo -- GNU/Linux Solutions e-mail: [EMAIL PROTECTED] Linux Device Driver [EMAIL PROTECTED] Embedded Systems [EMAIL PROTECTED] UNIX programming phone: +39 349 2432127 - 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/