Re: [PATCH] scsi: megaraid_sas - intercepts cmd timeout and throttle io

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

On Thu, 2007-05-10 at 07:01 -0700, Sumant Patro wrote:
> eh_timed_out call back (megasas_reset_timer) is used to throttle io to the adapter
> when it is called the first time for a scmd.
> The MEGASAS_FW_BUSY flag is set and can_queue reduced to 16. The can_queue is restored
> from completion routine in following two conditions : 5 seconds has elapsed and the # of
> outstanding cmds in FW is < 17.
> Also removed reserved fields from struct megasas_instance.

And the rest of the comments?

This is the cut out of all the ones I made, I think:

> > +static enum
> > +scsi_eh_timer_return megasas_reset_timer(struct scsi_cmnd *scmd)
> > +{
> > +     struct megasas_cmd *cmd = (struct megasas_cmd *)scmd->SCp.ptr;
> > +     struct megasas_instance *instance;
> > +     unsigned long flags;
> > +
> > +     if (cmd) {
> 
> I don't believe we can ever get a command timeout with no command, can
> we?
> 
> > +             if (time_after(jiffies, scmd->jiffies_at_alloc + 170 *
> HZ))
> > +                     return EH_NOT_HANDLED;
> 
> This 170s is a bit arbitrary ... surely you want it to be related to a
> multiple of scmd->timeout_per_command?


> > +             if (!(instance->flag & MEGASAS_FW_BUSY)) {
> > +                     /* FW is busy, throttle IO */
> > +                     spin_lock_irqsave(&instance->throttle_io_lock,
> flags);
> > +
> > +                     instance->host->can_queue = 16;
> 
> can_queue is protected by the host lock ... I think you need to dump
> the
> throttle_io_lock and simply use the host lock for all of this.


> > -     if (cmd->scmd) {
> > +     if (cmd->scmd)
> >               cmd->scmd->SCp.ptr = (char *)0;
> 
> That's NULL, ordinarily ...

James