On 30 Dec 2014, at 09:57, matthew green <[email protected]> wrote: > while trying to fix any midi/sequencer issues i've seen in the > last year, noticed that any attempt to call 'midiplay' on more > than one midi device at the same time, while not allowed, would > leave the sequencer device always busy. > > i tracked this down to an ugly race condition inside the > specfs code. the problem is that: > > t1 t2 > opens dev > sd_opencnt++ > tries to open dev > sd_opencnt++ > closes dev > since sd_opencnt is != 1, does not call close > sd_opencnt-- > gets EBUSY > sd_opencnt-- > > in this case, the device close routine is never called, and in > at least the midi case, all future opens return EBUSY due to > there already being an active open. > > i've spent some time reviewing the specfs code, and i think that > the below patch fixes these specific problems. it certainly > fixes the problem i have with trying to open /dev/music > concurrently. it may not be entirely correct or have problems > i've not seen with it, but atf works fine, as does the last > couple of days of random testing. > > the main changes are: > - introduce a "dying" flag to the specnode struture, > and use it appropriately > - add a condvar between open/close and revoke, to > ensure any revoke succeeds properly. > > i'd like to get this into netbsd 7, so any additional testing > against that branch or against -current would be greatly > appreciated. <snip> > @@ -397,7 +405,14 @@ spec_node_revoke(vnode_t *vp) > KASSERT(sn->sn_gone == false); > > mutex_enter(&device_lock); > - KASSERT(sn->sn_opencnt <= sd->sd_opencnt); > + sn->sn_dying = true; > + while (sn->sn_opencnt > sd->sd_opencnt) { > + /* > + * Open is in progress, but not complete, must wait > + * for it to complete. > + */ > + cv_wait(&specopenclose_cv, &device_lock); > + } > if (sn->sn_opencnt != 0) { > sd->sd_opencnt -= (sn->sn_opencnt - 1); > sn->sn_opencnt = 1;
Will this work if we revoke a device whose cdev_open() blocks, a TTY waiting for carrier for example? -- J. Hannken-Illjes - [email protected] - TU Braunschweig (Germany)
