Re: [PATCH RFC v3 04/41] csiostor: use reserved command for LUN reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/2/20 4:29 PM, Ming Lei wrote:
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?

The problem is the ioctl path.
When issuing TMF commands from the ioctl path we currently do not have a valid SCSI command to pass to the various SCSI EH functions. This requires the SCSI LLDDs to check for every EH function whether the passed in SCSI command is valid (ie coming from SCSI EH), or a made up one coming from the ioctl path.
And having to code various ways to work around this issue.

With this patchset I'm implementing a standardized way how these functions can be coded. By using a reserved command the EH functions and the driver internal command handling will always have a valid command, so the workarounds can be removed. And this is also the first step to my final SCSI EH rewrite, where I'm planning to move the SCSI EH functions from having the SCSI command as argument to the respective object (ie LU reset will be having a SCSI device as argument, host reset a SCSI host etc.)

Cheers,

Hannes
--
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@xxxxxxx                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux