Date: Thu, 23 Oct 2003 07:54:51 -0700 (PDT) From: Matthew Jacob <mjacob@feral.com> To: Poul-Henning Kamp <phk@phk.freebsd.dk> Cc: 'Kris Kennaway' <kris@obsecurity.org> Subject: Re: Sleeping on "isp_mboxwaiting" with the followingnon-sleepablelocks held: Message-ID: <20031023075058.I86925@beppo> In-Reply-To: <831.1066893153@critter.freebsd.dk> References: <831.1066893153@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
So you're theory here is that that all code that may be necessary to start I/O but could take a while should be done out of band. That's a reasonable response. The only problem is that you sometimes cannot easily tell if things like timeout driven recovery/restart can be used. The basic difference here I would guess might be that if I needed to do, say, FC loop bringup or chip reprogramming after a reset in the isp driver, I should try and do it off of the worker thread so I neither slept nor polled from isp_start and always returned right away. I'll start thinking about that. -matt On Thu, 23 Oct 2003, Poul-Henning Kamp wrote: > In message <20031022055417.L93661@beppo>, Matthew Jacob writes: > > >[1]: by 'sleep', I mean if I do *my* locking right, I should be able to > >yield the processor and wait for an event (an interrupt in this case). > > Not so when your device driver is entered through the devsw->strategy() > function, since that [cw]ould deadlock the entire disk-I/O system until > you return back up. > > Ideally, devsw->strategy() should just queue the request and return > immediately. In a world where context switches were free, scheduling > a task_queue to run foostart() (if necessary) would be the way to > do things. > > Most drivers call their own foostart() from strategy(), and as long > as foostart() does not go on long-term vacation, this is also OK, > poking a few registers, doing a bit of BUSDMA work is acceptable. > > But sleeping is not OK, mostly because a lot of sleeps may not > properly terminate in case of a memory shortage, and the way we > clear up a memory shortage is to page something out, and to page > something out we need to issue disk I/O, but somebody is holding > that hostage by sleeping in a driver... > > I will conceede that there are a certain small class of legitimate > sleeps that could be performed, unfortunately we can not automatically > tell an OK sleep from a not OK sleep, and therefore the decision > was to ban all sleeps, until such time as we had a case of something > that could not be (sensibly) done without the ability to sleep. > > I realize that in this case, you're sitting below CAM and scsi_??.c > and have very little say in what happens between devsw->strategy() > and your driver. > > As I read the original report, this is a case of error-handling. > Considering how long time error handling often takes and how > imperfect results one gets a lot of the time, error handling > should never strategy() sleep or take a long time, since that > will eliminates the chances that the system can flush dirty data > to other devices as part of a shutdown or panic. > > At some point soon, I plan to start measuring how much time > drivers spend in their strategy() routine and any offenders > will be put on notice because this is a brilliant way to hose > our overall system performance. I'm not going to set any > specific performance goal at this time, but I think we can > agree that more than one millisecond is way over the line. > > -- > Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 > phk@FreeBSD.ORG | TCP/IP since RFC 956 > FreeBSD committer | BSD since 4.3-tahoe > Never attribute to malice what can adequately be explained by incompetence. >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031023075058.I86925>