On Sat, May 02, 2020 at 10:49:32AM +0200, Hannes Reinecke wrote: > On 5/1/20 7:45 PM, Christoph Hellwig wrote: > > On Fri, May 01, 2020 at 11:01:29PM +0800, Ming Lei wrote: > > > > We cannot increase MAX_QUEUE arbitrarily as this is a compile time variable, > > > > which seems to relate to a hardware setting. > > > > > > > > But I can see to update the reserved command functionality for allowing to > > > > fetch commands from the normal I/O tag pool; in the case of LUN reset it > > > > shouldn't make much of a difference as the all I/O is quiesced anyway. > > > > > > It isn't related with reset. > > > > > > This patch reduces active IO queue depth by 1 anytime no matter there is reset > > > or not, and this way may cause performance regression. > > > > But isn't it the right thing to do? How else do we guarantee that > > there always is a tag available for the LU reset? > > > Precisely. One could argue that this is an issue with the current driver, > too; if all tags have timed-out there is no way how we can send a LUN reset > even now. Hence we need to reserve a tag for us to reliably send a LUN > reset. > And this was precisely the problem what sparked off this entire patchset; > some drivers require a valid tag to send internal, non SCSI commands to the > hardware. Could you explain a bit how you conclude that csio_scsi reset hander has to use one unique tag? At least we don't allocate request from block layer for ioctl(SG_SCSI_RESET), see scsi_ioctl_reset(). Also this patch doesn't use the reserved rq->tag too. > And with the current design it requires some really ugly hacks to make this > to work. You also don't explain how csio_eh_lun_reset_handler() is broken and where the ugly hack is in csio scsi too, and how this patch fixes the issue, could you document the exact reason in the commit log? Thanks, Ming
