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
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
