Re: Sleeping on "isp_mboxwaiting" with the followingnon-sleepablelocks held:

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_at_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_at_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.
>