[openib-general] SDP_CONN_LOCK

[Libor Michalek]

On Fri, Feb 18, 2005 at 12:13:30AM +0200, Michael S. Tsirkin wrote:
> Quoting r. Libor Michalek <libor at topspin.com>:
> > On Thu, Feb 17, 2005 at 11:29:37PM +0200, Michael S. Tsirkin wrote:
> > > Quoting r. Libor Michalek <libor at topspin.com>:
> > > > On Thu, Feb 17, 2005 at 05:35:17PM +0200, Michael S. Tsirkin wrote:
> > > > > Hi, Libor!
> > > > > Could you please explain what are the SDP_CONN_LOCK
> > > > > and friends doing in SDP?
> > > > > 
> > > > > It seems they just implement exclusive access to socket,
> > > > > but if so, why is a simple semaphore or mutex not used?
> > > > 
> > > >   They do implement exclusive access to the socket, but they implement
> > > > exclusive access from both process and irq context, which is why a
> > > > semaphore was not used. In interrupt context SDP_CONN_LOCK_BH is used
> > > > to lock the connection, look in sdp_cq_event_handler() for it's use,
> > > > and in process context SDP_CONN_LOCK is used.
> > > 
> > > I dont really understand how it works.
> > > When an interrupt arrives while users != 0, it seems you are
> > > calling scheduler().
> > > What is sdp_conn_internal_lock doing? I understand it is to be called
> > > from interrupt context, but how can it call scheduler() then?
> > 
> >   SDP_CONN_LOCK and SDP_CONN_UNLOCK are never called from interrupt
> > context, only from process context. The spinlock conn->lock.slock
> > is called in interrupt context and process context before the 'users'
> > variable is changed or read. If users != 0, then the connection is in
> > use by process context. If the connection is in use then another lock
> > request in process context will sleep until users == 0, if the connection
> > is in use then a lock request in interrupt context will mark the
> > connection as having had an event and return. When a process context
> > lock holder releases the lock, it checks if there has been an interrupt
> > event, which it then processes by draining and arming the CQ, and it
> > wakes any process context waiting for the lock.
> 
> Cant all this be done by a semaphore in process context, and 
> down_trylock in interrupt context?
> The "connection has an event" bit could be protected by a spinlock.

  The CQ draining happens inside of the spinlock. If the CQ drain
happened in process context inside of a semaphore, then there is
a race condition for the interrupt between checking the semaphore
and setting the spinlock protected "connection event bit". The
CQ drain and the "event bit" have to happen under the same lock
to avoid the race. 

  If you move the CQ drain and the "event bit" into the same spinlock
where do you put the semaphore? In process context the spin would
need to be inside the semaphore lock, but in interrupt context the
semaphore down_trylock and up are inside the spinlock. Now there would
be a race between spin_unlock() followed by up() in process context
and spin_lock() followed by down_trylock() in interrupt context.

   spin_unlock()
                  spin_lock()
                  down_trylock()
   up()


-Libor