Am 24.08.11 00:00, schrieb Mindaugas Rasiukevicius:
> Marc Balmer <mbal...@netbsd.org> wrote:
>> Here comes the third iteration of the gpio(4) patch. In addition to
>> whjat I already described, u_int_XXX types have been replaced by
>> uint_XXX and gpio(4) is made MPSAFE. Comments?
>>
>> Furthermore I am thinking if it would be useful if more than one process
>> could open a gpio device, as a long as they use different pins, e.g. one
>> process controlls a stepper motor using some of the pins while another
>> process drives some LEDs.
>
> There is nothing what prevents from multiple threads calling gpioioctl(),
> which is obviously not MP-safe. As soon as you will start fixing this, it
> will bring you back to the point I have already stated - the design needs
> to be MP-safe in general.
Well, you need to open it first, before you can to ioctl, and if only
one process can open it, only one process can ioctl it, right?
>
>> + mutex_enter(&sc->sc_mtx);
>> + if (sc->sc_opened)
>> + return EBUSY;
>
> This leaks the lock.
Yes, this part is utterly wrong.
>
>> int sc_opened;
>> + kmutex_t sc_mtx;
>
> Preferred suffix is _lock, rather than _mtx, so please use sc_lock.
Yes, no problem.
>
>> + sc->sc_pins[pin].pin_ticks_on = tvtohz(&pulse->gp_pulse_on);
>> + sc->sc_pins[pin].pin_ticks_off = tvtohz(&pulse->gp_pulse_off);
>> + if (sc->sc_pins[pin].pin_ticks_on == 0
>> + || sc->sc_pins[pin].pin_ticks_off == 0) {
>> + sc->sc_pins[pin].pin_ticks_on = hz / 2;
>> + sc->sc_pins[pin].pin_ticks_off = hz / 2;
>> + }
>
> Use gpio_pin_t *gpin = &sc->sc_pins[pin]; and gpin variable instead of
> sc->sc_pins[pin] each time. Apart from better readability, GCC will most
> likely generate a better code.
Sure, no problem either.
