Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
On 03/11/2013 07:04 PM, James Smart wrote:
On 3/11/2013 1:05 PM, Hannes Reinecke wrote:
On 03/07/2013 09:35 PM, Jeremy Linton wrote:
On 3/7/2013 2:20 PM, Mike Christie wrote:
On 03/07/2013 02:13 PM, Jeremy Linton wrote:
For lpfc, you never get to the code. Or rather when I was
testing it, I
couldn't find any way to propagate an error beyond the initial
lpfc_reset_flush_io_context() call in lpfc_device_reset_handler().
That call pretty much always returns success indpependent
of the remote
device because the firmware acks the context clear aborts,
resulting in the
outstanding iocb count being zero (independent of both the mid
layer status
and the actual device state).
Your lpfc patch fixes that right?
Yes. It allows the device reset to fail if the device doesn't
respond to the
task mgmt request, or rejects it, etc.
It doesn't unjam the commands that get aborted by the
flush_io_context() call.
Those have to depend on their timeouts. That is another patch...
It's actually worse than that.
lpfc_terminate_rport_io() calls lpfc_sli_abort_iocb(), which has
this:
if (lpfc_is_link_up(phba))
abtsiocb->iocb.ulpCommand = CMD_ABORT_XRI_CN;
else
abtsiocb->iocb.ulpCommand = CMD_CLOSE_XRI_CN;
/* Setup callback routine and issue the command. */
abtsiocb->iocb_cmpl = lpfc_sli_abort_fcp_cmpl;
ret_val = lpfc_sli_issue_iocb(phba, pring->ringno,
abtsiocb, 0);
if (ret_val == IOCB_ERROR) {
lpfc_sli_release_iocbq(phba, abtsiocb);
errcnt++;
continue;
}
Ie we're calling into firmware and waiting for an async event
telling us that the command has been aborted (ideally).
What I would like is some kind of synchronous call here, which would
guarantee us that we won't run into use-after-free issues.
Also 'lpfc_is_link_up' is clearly deficient here as the link
itself most likely is up, it's the I_T Nexus which is not.
James, is it safe to use 'CMD_CLOSE_XRI_CN' even when the link is up?
No, it's not safe. The ABORT, which sends an ABTS, is mandated so
that the other end and ourselves maintain proper (unique) exchange
id state. CLOSE sends no link traffic - but can only be used if
the login is broken (e.g. there's a different mechanism that
communicated termination of exchange states). I don't believe I
can trust the logic in the OS about frames laying in wait in the
fabric (maybe sent earlier, delayed at a switch, delivered after os
thinks nexus is gone), so driver needs to terminate them properly.
True. Just as I thought.
Which makes me wonder, how _exactly_ is I_T nexus reset supposed
to work? After all, we're trying to tell the target port that we
cannot talk to it anymore, right?
Which has some hurdles, conceptually ...
So from my POV I_T nexus reset can only be implemented on the
_initiator_ side, disregarding any target implementation.
(which would be pointless anyway).
Hmm. Probably have to ask T10 for clarification. Robert, any
insights?
The I_T nexus reset should be a FC transport implicit logout call to
the LLDD. E.g. this becomes a transport-specific action on what it
means to break the I_T nexus, which for FC, is to terminate the
login. This logout call allows the driver to do all the implicit
work to kill exchange contexts and allows it to adjust the state of
the target in it's FC discovery engine. Question is - should the
driver re-login ? Typically, this would be driven by a RSCN, which
I'm guessing for this scenario, would not be occurring. If you knew
it would, you could let the driver respond to the RSCN and re-login
later. If there's no RSCN, then I would assume we put a heartbeat
into the transport to retry login (to a WWPN/WWNN basis - remembered
from the I_T nexus reset) with the LLDD - a new interface as well -
call it "establish I_T_nexus".
Hmm. As I feared, my solution was a bit optimistic.
But good idea, using a 'logout' to trigger I_T nexus removal.
I wonder if we shouldn't attempt to logout for the fast_io_fail
case, too?
And for the timer, yeah, I guess we need something like this.
In lpfc's case - the Logout would allow the driver to take the
CLOSE_XRI case, giving you the speed/asynchronicity you desire.
Reuse of scsi job structures still can't occur until the driver
returns then via the completion routines (as DMA related to them
must be cancelled within the card by the ABORT/CLOSE commands - even
if we know there shouldn't be something to DMA).
The problem here is that the _eh calls are _synchronous_ in nature.
Not that it works perfectly nowadays (cf the discussion about TMF
results) but that's at least the theory.
Anyway, thanks for you insights.
It has been _very_ helpful.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendör
RE: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of James Smart
> Sent: Monday, March 11, 2013 11:04 AM
> To: Hannes Reinecke
> Cc: Jeremy Linton; Mike Christie; linux-scsi@vger.kernel.org; Andrew
> Vasquez; Chad Dupuis; Robert Elliot; Smart, James
> Subject: Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
>
>
> On 3/11/2013 1:05 PM, Hannes Reinecke wrote:
> > On 03/07/2013 09:35 PM, Jeremy Linton wrote:
> >> On 3/7/2013 2:20 PM, Mike Christie wrote:
> >>> On 03/07/2013 02:13 PM, Jeremy Linton wrote:
> >>>> For lpfc, you never get to the code. Or rather when I was
> >>>> testing it, I couldn't find any way to propagate an error beyond
> >>>> the initial
> >>>> lpfc_reset_flush_io_context() call in lpfc_device_reset_handler().
> >>>>
> >>>> That call pretty much always returns success indpependent of
> >>>> the remote device because the firmware acks the context clear
> >>>> aborts, resulting in the outstanding iocb count being zero
> >>>> (independent of both the mid layer status and the actual device
> >>>> state).
> >>>>
> >>>
> >>> Your lpfc patch fixes that right?
> >>
> >> Yes. It allows the device reset to fail if the device doesn't
> >> respond to the task mgmt request, or rejects it, etc.
> >>
> >> It doesn't unjam the commands that get aborted by the
> >> flush_io_context() call.
> >> Those have to depend on their timeouts. That is another patch...
> >>
> >>
> >
> > It's actually worse than that.
> > lpfc_terminate_rport_io() calls lpfc_sli_abort_iocb(), which has this:
> >
> >
> > if (lpfc_is_link_up(phba))
> > abtsiocb->iocb.ulpCommand = CMD_ABORT_XRI_CN;
> > else
> > abtsiocb->iocb.ulpCommand = CMD_CLOSE_XRI_CN;
> >
> > /* Setup callback routine and issue the command. */
> > abtsiocb->iocb_cmpl = lpfc_sli_abort_fcp_cmpl;
> > ret_val = lpfc_sli_issue_iocb(phba, pring->ringno,
> > abtsiocb, 0);
> > if (ret_val == IOCB_ERROR) {
> > lpfc_sli_release_iocbq(phba, abtsiocb);
> > errcnt++;
> > continue;
> > }
> >
> >
> > Ie we're calling into firmware and waiting for an async event telling
> > us that the command has been aborted (ideally).
> > What I would like is some kind of synchronous call here, which would
> > guarantee us that we won't run into use-after-free issues.
> >
> > Also 'lpfc_is_link_up' is clearly deficient here as the link itself
> > most likely is up, it's the I_T Nexus which is not.
> >
> > James, is it safe to use 'CMD_CLOSE_XRI_CN' even when the link is up?
>
> No, it's not safe. The ABORT, which sends an ABTS, is mandated so that the
> other end and ourselves maintain proper (unique) exchange id
> state. CLOSE sends no link traffic - but can only be used if the login
> is broken (e.g. there's a different mechanism that communicated
> termination of exchange states). I don't believe I can trust the logic
> in the OS about frames laying in wait in the fabric (maybe sent earlier,
> delayed at a switch, delivered after os thinks nexus is gone), so driver needs
> to terminate them properly.
>
>
> >
> > Which makes me wonder, how _exactly_ is I_T nexus reset supposed to
> > work? After all, we're trying to tell the target port that we cannot
> > talk to it anymore, right?
> > Which has some hurdles, conceptually ...
> > So from my POV I_T nexus reset can only be implemented on the
> > _initiator_ side, disregarding any target implementation.
> > (which would be pointless anyway).
> >
> > Hmm. Probably have to ask T10 for clarification. Robert, any insights?
>
>
> The I_T nexus reset should be a FC transport implicit logout call to the LLDD.
> E.g. this becomes a transport-specific action on what it means to
> break the I_T nexus, which for FC, is to terminate the login. This
> logout call allows the driver to do all the implicit work to kill exchange
> contexts and allows it to adjust the state of the target in
> it's FC discovery engine. Question is - should the driver re-login ?
> Typically, this would be driven by a RSCN, which I'm guessing for this
> scenario, would not be occurring.
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
On 3/11/2013 1:05 PM, Hannes Reinecke wrote:
On 03/07/2013 09:35 PM, Jeremy Linton wrote:
On 3/7/2013 2:20 PM, Mike Christie wrote:
On 03/07/2013 02:13 PM, Jeremy Linton wrote:
For lpfc, you never get to the code. Or rather when I was
testing it, I
couldn't find any way to propagate an error beyond the initial
lpfc_reset_flush_io_context() call in lpfc_device_reset_handler().
That call pretty much always returns success indpependent of
the remote
device because the firmware acks the context clear aborts,
resulting in the
outstanding iocb count being zero (independent of both the mid
layer status
and the actual device state).
Your lpfc patch fixes that right?
Yes. It allows the device reset to fail if the device doesn't
respond to the
task mgmt request, or rejects it, etc.
It doesn't unjam the commands that get aborted by the
flush_io_context() call.
Those have to depend on their timeouts. That is another patch...
It's actually worse than that.
lpfc_terminate_rport_io() calls lpfc_sli_abort_iocb(), which has this:
if (lpfc_is_link_up(phba))
abtsiocb->iocb.ulpCommand = CMD_ABORT_XRI_CN;
else
abtsiocb->iocb.ulpCommand = CMD_CLOSE_XRI_CN;
/* Setup callback routine and issue the command. */
abtsiocb->iocb_cmpl = lpfc_sli_abort_fcp_cmpl;
ret_val = lpfc_sli_issue_iocb(phba, pring->ringno,
abtsiocb, 0);
if (ret_val == IOCB_ERROR) {
lpfc_sli_release_iocbq(phba, abtsiocb);
errcnt++;
continue;
}
Ie we're calling into firmware and waiting for an async event telling
us that the command has been aborted (ideally).
What I would like is some kind of synchronous call here, which would
guarantee us that we won't run into use-after-free issues.
Also 'lpfc_is_link_up' is clearly deficient here as the link itself
most likely is up, it's the I_T Nexus which is not.
James, is it safe to use 'CMD_CLOSE_XRI_CN' even when the link is up?
No, it's not safe. The ABORT, which sends an ABTS, is mandated so that
the other end and ourselves maintain proper (unique) exchange id
state. CLOSE sends no link traffic - but can only be used if the login
is broken (e.g. there's a different mechanism that communicated
termination of exchange states). I don't believe I can trust the logic
in the OS about frames laying in wait in the fabric (maybe sent earlier,
delayed at a switch, delivered after os thinks nexus is gone), so driver
needs to terminate them properly.
Which makes me wonder, how _exactly_ is I_T nexus reset supposed to
work? After all, we're trying to tell the target port that we cannot
talk to it anymore, right?
Which has some hurdles, conceptually ...
So from my POV I_T nexus reset can only be implemented on the
_initiator_ side, disregarding any target implementation.
(which would be pointless anyway).
Hmm. Probably have to ask T10 for clarification. Robert, any insights?
The I_T nexus reset should be a FC transport implicit logout call to the
LLDD. E.g. this becomes a transport-specific action on what it means to
break the I_T nexus, which for FC, is to terminate the login. This
logout call allows the driver to do all the implicit work to kill
exchange contexts and allows it to adjust the state of the target in
it's FC discovery engine. Question is - should the driver re-login ?
Typically, this would be driven by a RSCN, which I'm guessing for this
scenario, would not be occurring. If you knew it would, you could let
the driver respond to the RSCN and re-login later. If there's no RSCN,
then I would assume we put a heartbeat into the transport to retry login
(to a WWPN/WWNN basis - remembered from the I_T nexus reset) with the
LLDD - a new interface as well - call it "establish I_T_nexus".
In lpfc's case - the Logout would allow the driver to take the CLOSE_XRI
case, giving you the speed/asynchronicity you desire. Reuse of scsi job
structures still can't occur until the driver returns then via the
completion routines (as DMA related to them must be cancelled within the
card by the ABORT/CLOSE commands - even if we know there shouldn't be
something to DMA).
-- james s
Cheers,
Hannes
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
On 03/07/2013 09:35 PM, Jeremy Linton wrote:
On 3/7/2013 2:20 PM, Mike Christie wrote:
On 03/07/2013 02:13 PM, Jeremy Linton wrote:
For lpfc, you never get to the code. Or rather when I was testing it, I
couldn't find any way to propagate an error beyond the initial
lpfc_reset_flush_io_context() call in lpfc_device_reset_handler().
That call pretty much always returns success indpependent of the remote
device because the firmware acks the context clear aborts, resulting in the
outstanding iocb count being zero (independent of both the mid layer status
and the actual device state).
Your lpfc patch fixes that right?
Yes. It allows the device reset to fail if the device doesn't respond
to the
task mgmt request, or rejects it, etc.
It doesn't unjam the commands that get aborted by the
flush_io_context() call.
Those have to depend on their timeouts. That is another patch...
It's actually worse than that.
lpfc_terminate_rport_io() calls lpfc_sli_abort_iocb(), which has this:
if (lpfc_is_link_up(phba))
abtsiocb->iocb.ulpCommand = CMD_ABORT_XRI_CN;
else
abtsiocb->iocb.ulpCommand = CMD_CLOSE_XRI_CN;
/* Setup callback routine and issue the command. */
abtsiocb->iocb_cmpl = lpfc_sli_abort_fcp_cmpl;
ret_val = lpfc_sli_issue_iocb(phba, pring->ringno,
abtsiocb, 0);
if (ret_val == IOCB_ERROR) {
lpfc_sli_release_iocbq(phba, abtsiocb);
errcnt++;
continue;
}
Ie we're calling into firmware and waiting for an async event telling us
that the command has been aborted (ideally).
What I would like is some kind of synchronous call here, which would
guarantee us that we won't run into use-after-free issues.
Also 'lpfc_is_link_up' is clearly deficient here as the link itself most
likely is up, it's the I_T Nexus which is not.
James, is it safe to use 'CMD_CLOSE_XRI_CN' even when the link is up?
Which makes me wonder, how _exactly_ is I_T nexus reset supposed to
work? After all, we're trying to tell the target port that we cannot
talk to it anymore, right?
Which has some hurdles, conceptually ...
So from my POV I_T nexus reset can only be implemented on the
_initiator_ side, disregarding any target implementation.
(which would be pointless anyway).
Hmm. Probably have to ask T10 for clarification. Robert, any insights?
Cheers,
Hannes
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
On 13-03-07 03:13 PM, Jeremy Linton wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 3/7/2013 1:19 PM, Mike Christie wrote: What happens for lpfc? It seems __fc_remote_port_delete ends up calling the fast io fail code right away and that sets FC_RPORT_FAST_FAIL_TIMEDOUT. We will then call lpfc_terminate_rport_io which only will send aborts for the commands. We will then call fc_block_scsi_eh above and that returns FAST_IO_FAIL and we will pass that back up to the scsi eh right away. For lpfc, you never get to the code. Or rather when I was testing it, I couldn't find any way to propagate an error beyond the initial lpfc_reset_flush_io_context() call in lpfc_device_reset_handler(). That call pretty much always returns success indpependent of the remote device because the firmware acks the context clear aborts, resulting in the outstanding iocb count being zero (independent of both the mid layer status and the actual device state). Result: all the code beyond the device reset handler never gets called. Unsurprisingly, I found pretty well the same thing with megaraid and mpt2sas (SAS) drivers. A big thumbs up from the drivers if a LU reset was sent when there was no way through the expander (due to zoning) to the LU (disk) in question. Further, when that LU (disk) was viewed from another initiator, no UA condition had been set; more evidence that the LU reset did not get through. "Fire and forget" task management functions ... Doug Gilbert -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
On 3/7/2013 2:20 PM, Mike Christie wrote: > On 03/07/2013 02:13 PM, Jeremy Linton wrote: >> For lpfc, you never get to the code. Or rather when I was testing it, I >> couldn't find any way to propagate an error beyond the initial >> lpfc_reset_flush_io_context() call in lpfc_device_reset_handler(). >> >> That call pretty much always returns success indpependent of the remote >> device because the firmware acks the context clear aborts, resulting in the >> outstanding iocb count being zero (independent of both the mid layer status >> and the actual device state). >> > > Your lpfc patch fixes that right? Yes. It allows the device reset to fail if the device doesn't respond to the task mgmt request, or rejects it, etc. It doesn't unjam the commands that get aborted by the flush_io_context() call. Those have to depend on their timeouts. That is another patch... -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
On 03/07/2013 02:20 PM, Mike Christie wrote: > On 03/07/2013 02:13 PM, Jeremy Linton wrote: >> -BEGIN PGP SIGNED MESSAGE- >> Hash: SHA1 >> >> On 3/7/2013 1:19 PM, Mike Christie wrote: >>> What happens for lpfc? It seems __fc_remote_port_delete ends up calling the >>> fast io fail code right away and that sets FC_RPORT_FAST_FAIL_TIMEDOUT. We >>> will then call lpfc_terminate_rport_io which only will send aborts for the >>> commands. We will then call fc_block_scsi_eh above and that returns >>> FAST_IO_FAIL and we will pass that back up to the scsi eh right away. >> >> >> For lpfc, you never get to the code. Or rather when I was testing it, I >> couldn't find any way to propagate an error beyond the initial >> lpfc_reset_flush_io_context() call in lpfc_device_reset_handler(). >> >> That call pretty much always returns success indpependent of the remote >> device because the firmware acks the context clear aborts, resulting in the >> outstanding iocb count being zero (independent of both the mid layer status >> and the actual device state). >> > > Your lpfc patch fixes that right? > Nevermind. Found your patch. It looks like it does fix that problem. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
On 03/07/2013 02:13 PM, Jeremy Linton wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 3/7/2013 1:19 PM, Mike Christie wrote: >> What happens for lpfc? It seems __fc_remote_port_delete ends up calling the >> fast io fail code right away and that sets FC_RPORT_FAST_FAIL_TIMEDOUT. We >> will then call lpfc_terminate_rport_io which only will send aborts for the >> commands. We will then call fc_block_scsi_eh above and that returns >> FAST_IO_FAIL and we will pass that back up to the scsi eh right away. > > > For lpfc, you never get to the code. Or rather when I was testing it, I > couldn't find any way to propagate an error beyond the initial > lpfc_reset_flush_io_context() call in lpfc_device_reset_handler(). > > That call pretty much always returns success indpependent of the remote > device because the firmware acks the context clear aborts, resulting in the > outstanding iocb count being zero (independent of both the mid layer status > and the actual device state). > Your lpfc patch fixes that right? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 3/7/2013 1:19 PM, Mike Christie wrote: > What happens for lpfc? It seems __fc_remote_port_delete ends up calling the > fast io fail code right away and that sets FC_RPORT_FAST_FAIL_TIMEDOUT. We > will then call lpfc_terminate_rport_io which only will send aborts for the > commands. We will then call fc_block_scsi_eh above and that returns > FAST_IO_FAIL and we will pass that back up to the scsi eh right away. For lpfc, you never get to the code. Or rather when I was testing it, I couldn't find any way to propagate an error beyond the initial lpfc_reset_flush_io_context() call in lpfc_device_reset_handler(). That call pretty much always returns success indpependent of the remote device because the firmware acks the context clear aborts, resulting in the outstanding iocb count being zero (independent of both the mid layer status and the actual device state). Result: all the code beyond the device reset handler never gets called. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJROPTfAAoJEL5i86xrzcy7MSMIAKaUZV1sfE55/n95b28WTdAS 7HdUechq5JRh2jqW+PVQub3iADgjl5RZkj8T3vNTZgzR9pcQ6NE/qdkwho+p29Wx enBa68HMosO+oiqPVSz7mmyuOsubB/DxPC3D+5ODu3nTJNMBxE4wYgdfGYsXVZS7 f/HCLo0Ysg7SBzTBQKvk0E1UtMJv1miEsIgxxqYSvOAOcHtKwUaYtCclE2z9egby AnyVV1UrVa/cI8R4w0nArnyLCrLzG4IVAMByyb0KAQ3NKOdxGPqxPTkoY6GEpcQ9 GxzoZVWerGbzdjYXz2gckiN8oonBIB3esrrOTyq14sTqfOxtynH+8X3qS2uRFhg= =t9Gx -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
Sorry for the late reply.
On 12/11/2012 02:23 AM, Hannes Reinecke wrote:
> @@ -793,7 +793,8 @@ struct scsi_host_template bfad_im_scsi_host_template = {
> .queuecommand = bfad_im_queuecommand,
> .eh_abort_handler = bfad_im_abort_handler,
> .eh_device_reset_handler = bfad_im_reset_lun_handler,
> - .eh_bus_reset_handler = bfad_im_reset_bus_handler,
> + .eh_target_reset_handler = fc_eh_it_nexus_loss_handler,
> + .eh_bus_reset_handler = NULL,
Don't need to set to NULL in the final patch, and don't forget to send a
patch to remove all the code we do not need anymore :)
> +fc_eh_it_nexus_loss_handler(struct scsi_cmnd *cmnd)
> +{
> + struct fc_internal *i = to_fc_internal(cmnd->device->host->transportt);
> + struct scsi_target *starget = scsi_target(cmnd->device);
> + struct fc_rport *rport = starget_to_rport(starget);
> + int ret;
> +
> + ret = fc_block_scsi_eh(cmnd);
> + if (i->f->eh_it_nexus_loss)
> + ret = i->f->eh_it_nexus_loss(cmnd);
> +
> + /* FAST_IO_FAIL indicates the port is already blocked */
> + if (ret == FAST_IO_FAIL)
> + return ret;
> + if (ret == SUCCESS)
> + /* All outstanding I/O has been aborted */
> + __fc_remote_port_delete(rport, -1);
> + else {
> + /* Failed to abort outstanding I/O, trigger FAST_IO_FAIL */
> + __fc_remote_port_delete(rport, 0);
I think it looks ok from a high level, but I am not sure how the drivers
are working here.
What happens for lpfc? It seems __fc_remote_port_delete ends up calling
the fast io fail code right away and that sets
FC_RPORT_FAST_FAIL_TIMEDOUT. We will then call lpfc_terminate_rport_io
which only will send aborts for the commands. We will then call
fc_block_scsi_eh above and that returns FAST_IO_FAIL and we will pass
that back up to the scsi eh right away.
But it seems lpfc_terminate_rport_io does not wait for the abort
reposnses and clean up the affected scsi_cmnds, and it does not seem to
do something to prevent lpfc from touching affected scsi_cmnds, does it
(I could not find the code)? If lpfc ends up touching a scsi_cmnd after
we have return FAST_IO_FAIL from this function then both lpfc and some
other code could be using the same scsi_cmnd struct.
For qla2xxx, it seems qla2x00_terminate_rport_io aborts commands, but it
looks like there is a small race where if some other thread was actually
completing the command already, then that thread could be touching the
scsi command, but this function could return and the scsi eh could end
up giving the command to some other driver or retrying while the other
thread was still touching it.
It also seems like there is a race where since
qla2x00_terminate_rport_io also calls the logout functions for the port,
then if that path was fast enough it could it lead to
fc_remote_port_delete getting called by qla2xxx while
fc_eh_it_nexus_loss_handler's call to __fc_remote_port_delete was still
running?
> + ret = fc_block_scsi_eh(cmnd);
> + }
> + if (ret != FAST_IO_FAIL) {
> + if (rport->port_state == FC_PORTSTATE_ONLINE)
> + ret = SUCCESS;
> + else
> + ret = FAILED;
> + }
> + return ret;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
On 12/11/2012 01:46 PM, Martin Peschke wrote: Hello Hannes, fc_eh_it_nexus_loss_handler() is invoked as the eh_target_reset_handler() callback and the eh_bus_reset_handler() is removed. lpfc_target_reset_handler(), which is replaced by your patch, used to issue a TARGET_RESET task management function over FCP in the eh_target_reset_handler() callback. What's wrong with that? Nothing per se. Only that the TARGET_RESET TMF has been removed from SAM-3/FCP-3 onwards, so there might not be any functionality behind it. But drivers can supply the functionality via ->eh_it_nexus_loss callback. I didn't want to touch the existing eh_target_reset_handler myself as I'm not familiar with the firmware specifics. That is being left as an exercise to the reader :-) The main point here is that we're emulating REMOVE I_T NEXUS by setting the port state to BLOCKED and invoke dev_loss_tmo. This will prevent any further I/O to be send down. With the original handler the port state wasn't modified, which led to excessive recovery times when no RSCN was received. And yes, I had several bug reports now where the HBA did not receive RSCNs, either due to a switch malfunction or due to an error injection. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2][RFC] scsi_transport_fc: Implement I_T nexus reset
Hello Hannes, > fc_eh_it_nexus_loss_handler() is invoked as the > eh_target_reset_handler() callback and the > eh_bus_reset_handler() is removed. lpfc_target_reset_handler(), which is replaced by your patch, used to issue a TARGET_RESET task management function over FCP in the eh_target_reset_handler() callback. What's wrong with that? Martin -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
