> Date: Thu, 7 Dec 2017 22:26:54 -0500 (EST) > From: Mouse <mo...@rodents-montreal.org> > > DIAGNOSTIC and DEBUG are both on already. None of the LOCK* options > are, though - see Paul Goyette's response, below.
My usual debug-all-the-things kernel configuration is: makeoptions DBG=-"-g" options DEBUG options DIAGNOSTIC options LOCKDEBUG > ftp.rodents-montreal.org:/mouse/misc/lpt/ holds the code. base/ has > the code I started with (which is stock 5.2, for these files); new/ has > my current version. lptreg.h is identical; lptvar.h and lpt.c have > changes. diffs is output from "diff -u -r base new". (All these are > relative to sys/dev/ic/.) Where do you set LPT_RF_WAITING? Some other general comments: Are you sure that's the code you're using? lpt_raw_read seems to refer to a variable s in splx(s) without defining it -- seems to have just `spllpt()' where it should have `s = spllpt()'. Of course, all that is unnecessary with a lock at the right IPL, but it makes me suspicious that I'm not looking at the code that reproduces your bug. It's unclear to me why you have splhigh in lptopen. splhigh should be reserved for very special purposes that are clearly justified. If you need to defer the first tick until after the rest of the lpt is initialized, just use callout_setfunc first, and then after it's initialized at the end of lptopen do callout_schedule. For lptclose, note that callout_stop does not wait for the callout to stop if it was in the middle of executing, and does not destroy it. You must use callout_halt to wait, and then use callout_destroy to destroy it. In this case, everything will run giant-locked, but if you're writing new code you should avoid relying on that, and definitely avoid volatile voodoo. For instance, in lpt_raw_read_tick, I advise that you unconditionally acquire the lock and use it for all access to the new fields you added: rawbuf, rflags, rbfill, rbptr, laststat. (If not, then whoever wants to remove the giant lock will have to figure out the right places to put in memory ordering barriers, and that's not a fun surprise to give someone!) Can you write down all the states you expect this thing to be in, and the state transitions that each action -- lpt_raw_read_tick, lpt_raw_read -- can take, and in what states lpt_raw_read must wait for a tick to transition to another state before it can complete? The condition variable represents exactly that set of state transitions of interest under that mutex, and it's helpful to identify what it is in order to assess whether all paths that effect those state transitions also notify the condition variable. > ...I used IPL_VM, because splvm() is the same as spllpt(), and because > the manpage says that results in a spin mutex, which I thought was what > I needed. Is there some way to tell what IPL the hardware interrupt > for the device comes in on? You should set the IPL to the same one that the interrupt handler runs at. In this case, it's a callout, so it's IPL_SOFTCLOCK, as the callout(9) man page says. (Under the hood, in this case, there actually is _no_ splsoftclock() issued by mutex_enter, but that's not important to you -- what is important is that you pass IPL_SOFTCLOCK to mutex_init so that mutex_enter works in thread or interrupt context to exclude one another.) > Or, in my case, hang? I don't know who holds it; I'm not sure how to > find out. I should probably go peek under the hood. With LOCKDEBUG, if you know the lock address you can use `show lock' in ddb.
