Re: locking in psmouse

[Pavel Machek <pavel@suse.cz>]

Hi!

> > Anyway, locking still seems to be needed: 
> > 
> >         while (psmouse->cmdcnt && timeout--) {
> > 
> >                 if (psmouse->cmdcnt == 1 && command == PSMOUSE_CMD_RESET_BAT &&
> >                                 timeout > 100000) /* do not run in a endless loop */
> >                         timeout = 100000; /* 1 sec */
> > 
> >                 if (psmouse->cmdcnt == 1 && command == PSMOUSE_CMD_GETID &&
> >                     psmouse->cmdbuf[1] != 0xab && psmouse->cmdbuf[1] != 0xac) {
> >                         psmouse->cmdcnt = 0;
> >                         break;
> >                 }
> > 
> >                 spin_unlock_irq(&psmouse_lock);
> >                 udelay(1);
> >                 spin_lock_irq(&psmouse_lock);
> >         }
> > 
> > racing with
> > 
> >         if (psmouse->cmdcnt) {
> >                 psmouse->cmdbuf[--psmouse->cmdcnt] = data;
> >                 goto out;
> >         }
> > 
> > now... if each runs on different CPU, it can be possible that
> > psmouse->cmdcnt is seen as 1 but data are not yet in
> > psmouse->cmdbuf... Locking seems neccessary here.
> 
> I see.. but this particular case can be resolved but rearranging the code to
> write command response first and then decrementing the counter... and putting
> a barrier? Or just make cmdcnt atomic... spin_lock_irq feels heavier than
> absolutely necessary.

cmdcnt would have to be atomic_t, ack too, state too, and
you'd have to be very carefull with memory barriers...
I guess spinlock is better solution.
				Pavel
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms