tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Adding pulse support to gpio(4), gpioctl(8)

Marc Balmer <> wrote:
> > 
> >> +  mutex_enter(&sc->sc_mtx);
> >> +  if (sc->sc_opened)
> >> +          return EBUSY;
> > 
> > This leaks the lock.
> Ooops... But it is not needed here anymore, see below...

Are underlying drivers MP-safe?

> >>    int                      sc_opened;
> >> +  kmutex_t                 sc_mtx;
> > 
> > Preferred suffix is _lock, rather than _mtx, so please use sc_lock.
> I have seen both forms in the tree.

The ones which use _mtx should rather be fixed, instead of sprinkling them
even more.  Also, for condvars we use _cv suffix.

> I reworked the patch again.  This version changes the behaviour as well:
>  It is now possible that multiple thready/processes have a gpio device
> open, but only one thread can be in ioctl().  Two new functions are
> added, gpio_lock() and gpio_nolock() which can be used by child drivers
> as well.

Locking everything with a single fat lock is inefficient.  As initial step,
while learning, that is suitable.  Otherwise locking should be fine grained.


Home | Main Index | Thread Index | Old Index