Re: [PATCH 1/2] Block I/O while SG reset operation in progress - midlayer portion

[Mike Anderson <andmike@us.ibm.com>]

James Smart <James.Smart@Emulex.Com> wrote:
> Well for a couple of reasons. I didn't view it as an actual recovery
> function as it's execution doesn't attempt to change the state of the
> shost, starget or sdev. It's not part of an escalation policy, etc.
> It's essentially performing the same effect as a reset by some other
> initiator in a multi-initiator environment, just with i/o failing faster
> than it may normally take us to detect it. As for side effects on the
> teardown, I don't see anything different than what exists today.
> At worst, it should only add a short delay until the ioflow resumes.

I agree it is unsafe today as we do not restrict the scsi_reset_provider
during scsi_remove_host.

In scsi_remove_host there was code added to detect recovery running during
removal and try to reduce the chance of eh_* routines being called post
scsi_remove_host returning.

> 
> I also looked at what it would take to implement a full state change
> or piggy back on SHOST_RECOVERY. It's a significant amount of code,
> and the addition of a new state brings it's own set of additional
> issues to get everything to play right. In the end, I saw very little
> change in result, but with lots of code changes.
> 

Yes, there is some code overhead, but some of it is needed. In looking at
your patch you add to the scsi_host_in_recovery case, but do not have a
wake_up for event waiters on host_wait.

> My main concern was - what is the interface definition we're telling
> the LLDD's for the eh routines ?  Does the LLDD expect to always be
> entered in them while in the error thread ? Does the LLDD expect that
> an eh routine will never be invoked will another call to that routine
> is outstanding ?  Obviously, none of these are true with the existing
> implementation.

Well the scsi_mid_low_api.txt indicates that the eh_*_reset_handler
routines will be called with "No other commands will be queued on current
host during eh". As you have indicated this is not true today. I did
review a few reset functions: One checked to see if a reset action was
pending and returned an error, the others appeared to not care about
current state of a reset action.


-andmike
--
Michael Anderson
andmike@us.ibm.com