Re: [PATCH/RFC 1/2] 3ware 9000 SATA-RAID driver v2.26.00.009

[Jeff Garzik <jgarzik@pobox.com>]

Adam Radford wrote:
> Jeff,
> 
> Thanks for the useful feedback.
> 
> - The card can take up to 256 io's at a time, but 2 are marked off
>   for internal use.  There are no 'per device' limits.  The firmware queue
>   depth is 256 total for all devices.

Ok.

I would therefore recommend not setting device queue depth in 
->slave_configure(), but rather setting the Scsi_Host_Template's 
->can_queue to 256.

This more accurately reflects the hardware.


> - We are not calling do_gettimeofday() on every command sent to hardware.  It is
>   only sent when firmware requests an AEN_SYNC_TIME, which happens at startup, and
>   probably once a day thereafter.  This is so we can have scheduled events in the
>   firmware, such as rebuild events, scrub events, etc, _without_ an annoying daemon
>   running.  There is clock drift so we periodically sync the time.

I see the AEN_SYNC_TIME usage of do_gettimeofday(), but there are other 
uses of this function as well.


> - The ioctl routine _does_ copy down data buffers.  This isn't in the main IO path.

Still, you shouldn't need to copy all those buffers, just make sure they 
are present and mapped, then pass them to hardware.


> - twa_poll_status_gone() is called to prevent return from a function until the
>   hardware is ready.  How else do you propose delaying when the scsi midlayer called
>   your entrypoint with spin_lock_irqsave(host->host_lock, flags) ?

It's a problem with your model.  The ->queuecommand should 
asynchronously submit commands to hardware, up to the hardware limit.

twa_poll_status_done() AFAICS is being used during initialization and 
such, where ideally you should have code that issues a SCSI command, 
then calls scsi_wait_req or similar to wait for the command's completion.


> I will fix some of the issues you mention and re-send.  Thanks!

Thanks!

	Jeff