Re: Sleeping on "isp_mboxwaiting" with the following non-sleepablelocks held:

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.