On Fri, Apr 22, 2011 at 07:38:08PM +0200, Oliver Hartkopp wrote: > On 04.03.2011 11:47, Kurt Van Dijck wrote: > > Oliver, > > > > while sitting on the train back home, and looking in your isotp code, > > I suspected a possible race condition between waiting for the socket > > to become tx.state == ISOTP_IDLE and actually tx.state = ISOTP_SENDING. > > I think I solved this by using a semaphore. > > The patch is not tested, and may be incomplete. I just send it already > > so people can start thinking about the problem. > > Thanks. > > I tried to apply and compile your idea. Removing the entire wait_queue stuff > and fixing some minor things (ret/err variables), the waiting for the > transmission completion in isotp_release() turned out to be somehow tricky ... > > Then i asked myself what the problem of using the existing waitqueues would be > - as you remarked?!? > > I found this on the web: > > --- > http://www.linuxgrill.com/anonymous/fire/netfilter/kernel-hacking-HOWTO-5.html > > Wait Queues <include/linux/wait.h> <SLEEPS> > > A wait queue is not really a form of locking, but it's used to guarantee that > if you are checking for a condition, and going to sleep if it's not true, that > there is no gap between the check and the sleeping. You declared one > wait_queue_head_t, and then processes which want to wait for that condition > declare a wait_queue_t referring to themselves, and place that in the queue. > --- > > Looks perfectly for my needs - and is widely used, even in networking code ...
I think this means : a wait-queue will not put you asleep while your condition is true. My original problem scenario deals with what happens after you woke up. At the time the state turns ISOTP_IDLE & wake_up_xx(), more than 1 waiter may be waked. When the first waked sleeps (for some reason) before setting ISOTP_SENDING, a second waiter successfully contests so->tx.state, and falls out the wait_event_xxx(). At that point, both are waked, and they both think they can use the socket. That is the point that a locking is necessary. I solved that with a semaphore (=locking+sleeping). Maybe the problem is solved by setting ISOTP_SENDING before a possible sleep. I'm not sure that is a legal solution on SMP. I'm not expert enough on that matter :-). I just trust the semaphore ... Anyhow, the return code of wait_event_interruptible should be checked. Ctrl-C during the wait_event_interruptible will now succeed and start sending, while the state was _not_ ISOTP_IDLE. > > Regards, > Oliver > Kurt _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
