Message-ID: <20070730083259.GD9840@enneenne.com>
Date:	Mon, 30 Jul 2007 10:32:59 +0200
From:	Rodolfo Giometti <giometti@...eenne.com>
To:	Satyam Sharma <satyam@...radead.org>
Cc:	Satyam Sharma <satyam.sharma@...il.com>,
	Chris Friesen <cfriesen@...tel.com>,
	David Woodhouse <dwmw2@...radead.org>,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: LinuxPPS & spinlocks

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:    giometti@...eenne.com
Linux Device Driver                             giometti@...dd.com
Embedded Systems                     		giometti@...ux.it
UNIX programming                     phone:     +39 349 2432127
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists