Re: [PATCH 09/11] scsi: don't set tagging state from scsi_adjust_queue_depth

[Christoph Hellwig <hch@lst.de>]

> > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> > index a759cb2..596c2f4 100644
> > --- a/drivers/scsi/aacraid/linit.c
> > +++ b/drivers/scsi/aacraid/linit.c
> > @@ -462,9 +462,8 @@ static int aac_slave_configure(struct scsi_device *sdev)
> >  			depth = 256;
> >  		else if (depth < 2)
> >  			depth = 2;
> > -		scsi_adjust_queue_depth(sdev, MSG_ORDERED_TAG, depth);
> > -	} else
> > -		scsi_adjust_queue_depth(sdev, 0, 1);
> > +		scsi_adjust_queue_depth(sdev, depth);
> > +	}
> >  
> >  	return 0;
> >  }
> Why did you omit the 'else' branch?
> cmd_per_lun is set to 256 for aacraid AFAICS ...

Thanks, fixed.

> Hmm. This is actually wrong; it should be set to '1'.
> '2' is a left-over from the (long since removed) internal queueing
> within the aic7xxx driver.

Please send a separate patch for that.  Interestingly your patch that
sets it to 1 for aic79xx sais that's only temporarily.  If you still
have the hardware (either one should be fine) that driver could use
some major cleanup in how it deals with queuing and tags.

> > diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
> > index d8dcf36..aa0f403 100644
> > --- a/drivers/scsi/u14-34f.c
> > +++ b/drivers/scsi/u14-34f.c
> > @@ -696,25 +696,25 @@ static int u14_34f_slave_configure(struct scsi_device *dev) {
> >     if (TLDEV(dev->type) && dev->tagged_supported)
> >  
> >        if (tag_mode == TAG_SIMPLE) {
> > -         scsi_adjust_queue_depth(dev, MSG_SIMPLE_TAG, tqd);
> > +         scsi_adjust_queue_depth(dev, tqd);
> >           tag_suffix = ", simple tags";
> >           }
> >        else if (tag_mode == TAG_ORDERED) {
> > -         scsi_adjust_queue_depth(dev, MSG_ORDERED_TAG, tqd);
> > +         scsi_adjust_queue_depth(dev, tqd);
> >           tag_suffix = ", ordered tags";
> >           }
> >        else {
> > -         scsi_adjust_queue_depth(dev, 0, tqd);
> > +         scsi_adjust_queue_depth(dev, tqd);
> >           tag_suffix = ", no tags";
> >           }
> >  
> >     else if (TLDEV(dev->type) && linked_comm) {
> > -      scsi_adjust_queue_depth(dev, 0, tqd);
> > +      scsi_adjust_queue_depth(dev, tqd);
> >        tag_suffix = ", untagged";
> >        }
> >  
> >     else {
> > -      scsi_adjust_queue_depth(dev, 0, utqd);
> > +      scsi_adjust_queue_depth(dev, utqd);
> >        tag_suffix = "";
> >        }
> >  
> See my comment on the previous patches. As we're not using
> ordered tags that branch is never used.

The tag_mode is a module parameter.  I've kept it for compatibility
(or lazyness..), but there's no effect left, similar to the queue_type
sysfs file that still accept the "ordered" value.