Re: [PATCH] QStor SATA/RAID driver for 2.6.9-rc3

[James Bottomley <James.Bottomley@SteelEye.com>]

On Tue, 2004-10-12 at 12:51, Mark Lord wrote:
> That's how it works already, thanks, except that it
> does have a few calls to in_interrupt() rather than
> simply passing itself a flag parameter to convey the
> same information -- I'll fix that now.
> 
> Except that it uses schedule_work() rather than a tasklet.
> The bottom half is only there for abnormal conditions
> like major chip errors and hotplug events.
> 
> So the only new suggestion here is to use a tasklet for
> the bottom-half processing rather than schedule_work()?
> 
> I thought work queues were the preferred mechanism for
> infrequent uses such as this these days?  A tasklet is no
> problem here though, so long as worker threads (schedule_work)
> do not also rely on tasklets.

Really, no, you don't want to be doing what you are doing in
qs_process_sff_entry()

At certain points you decide you have too much work, disable interrupts
on the card and reschedule the routine in a work queue.

Please, please don't do this.  It's a sure fire recipe for a hang. 
Suppose a prior task in the workqueue needs to page something in from
swap and you're the swap device for instance....

What you need to do is to gather as much information as will reset the
interrupt and then process the data as a tasklet.  For your hotplug
events, they should be fire and forget as schedule_work().

*never* disable interrupts and have re-enabling them contingent on a
workqueue thread.

James