[PATCH 1/1] [SCSI] Clear Devloss Callback Done flag in fc_remote_port_rolechg.

[PATCH 1/1] [SCSI] Clear Devloss Callback Done flag in fc_remote_port_rolechg.

[Alex.Iannicelli]

This patch fixes a bug where devloss is not called on fc_host teardown. The 
issues is seen if the LLDD uses rport_rolechg to add target role to rport.
When an rport is discovered the LLDD uses fc_rport_add to add the rport and then 
uses remote_port_rolechg to indicate that it is a target. If this rport goes away 
and the LLDD calls fc_report_port_delete. The transport calls devloss after the 
devloss timer fires and it sets the FC_RPORT_DEVLOSS_CALLBK_DONE flag. If the
LLDD discovers this target again and uses remote_port_add to add it, but it was
a target and there is no target role assigned yet then this function returns 
without clearing the FC_RPORT_DEVLOSS_CALLBACK_DONE flag. When the LLDD calls
again to change the role to a target the report_port_rolechg routine neglects to
clear the flag as well. Once in this state, if the LLDD tears down the fc_host
then devloss will not be called for this target because the DEVLOSS_CALLBK_DONE
flag is still set.
 
This patch adds the FC_RPORT_DEVLOSS_CALLBK_DONE flag to the group of flags that
it clears when flushing devloss timers.
 
Signed-off-by: Alex Iannicelli <alex.iannicelli@emulex.com>
Signed-off-by: James Smart <james.smart@emulex.com>
 
---
 
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 1b21491..f59d4a0 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -3048,7 +3048,8 @@ fc_remote_port_rolechg(struct fc_rport  *rport, u32 roles)
 
                spin_lock_irqsave(shost->host_lock, flags);
                rport->flags &= ~(FC_RPORT_FAST_FAIL_TIMEDOUT |
-                                 FC_RPORT_DEVLOSS_PENDING);
+                                 FC_RPORT_DEVLOSS_PENDING |
+                                 FC_RPORT_DEVLOSS_CALLBK_DONE);
                spin_unlock_irqrestore(shost->host_lock, flags);
 
                /* ensure any stgt delete functions are done */

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] [SCSI] Clear Devloss Callback Done flag in fc_remote_port_rolechg.

[James Bottomley]

On Thu, 2011-10-27 at 07:53 -0700, Alex.Iannicelli@Emulex.Com wrote:
> This patch fixes a bug where devloss is not called on fc_host teardown. The 
> issues is seen if the LLDD uses rport_rolechg to add target role to rport.
> When an rport is discovered the LLDD uses fc_rport_add to add the rport and then 
> uses remote_port_rolechg to indicate that it is a target. If this rport goes away 
> and the LLDD calls fc_report_port_delete. The transport calls devloss after the 
> devloss timer fires and it sets the FC_RPORT_DEVLOSS_CALLBK_DONE flag. If the
> LLDD discovers this target again and uses remote_port_add to add it, but it was
> a target and there is no target role assigned yet then this function returns 
> without clearing the FC_RPORT_DEVLOSS_CALLBACK_DONE flag. When the LLDD calls
> again to change the role to a target the report_port_rolechg routine neglects to
> clear the flag as well. Once in this state, if the LLDD tears down the fc_host
> then devloss will not be called for this target because the DEVLOSS_CALLBK_DONE
> flag is still set.
>  
> This patch adds the FC_RPORT_DEVLOSS_CALLBK_DONE flag to the group of flags that
> it clears when flushing devloss timers.
>  
> Signed-off-by: Alex Iannicelli <alex.iannicelli@emulex.com>
> Signed-off-by: James Smart <james.smart@emulex.com>

This patch has the usual outlook tab->space corruption.  Could you
resend it either with a working email client or as an attachment?

Thanks,

James

[PATCH 1/1] [SCSI] Clear Devloss Callback Done flag in fc_remote_port_rolechg

[James Smart]

This patch fixes a bug where devloss is not called on fc_host teardown.
The issue is seen if the LLDD uses rport_rolechg to add the target role
to an rport.

When an rport goes away, the LLDD will call fc_remote_port_delete, which
will start the devloss timer.  If the timer expires, the transport will
call the devloss callback and set the FC_RPORT_DEVLOSS_CALLBK_DONE flag.
However, the rport structure is not deleted, it is retained to store the
SCSI id mappings for the rport in case it comes back. In the scenario
where it does come back, and the driver calls fc_remote_port_add, but does
not indicate the "target" role for the rport - the create will clear the
structure, but forgets to clear FC_RPORT_DEVLOSS_CALLBK_DONE flag (which
is cleared if it's added with the target role). The secondary call, of
fc_remote_port_rolechg to add the target role also does not clear the flag.
Thus, the next time the rport goes away, the resulting devloss timer
expiration will not call the driver callback as the flag is still set.

This patch adds the FC_RPORT_DEVLOSS_CALLBK_DONE flags to the list of
those that are cleared upon reuse of the rport structure.

 Signed-off-by: Alex Iannicelli <alex.iannicelli@emulex.com>
 Signed-off-by: James Smart <james.smart@emulex.com>

 ---

 scsi_transport_fc.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff -upNr a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
--- a/drivers/scsi/scsi_transport_fc.c	2011-11-03 10:27:14.000000000 -0400
+++ b/drivers/scsi/scsi_transport_fc.c	2011-11-03 10:28:59.000000000 -0400
@@ -3048,7 +3048,8 @@ fc_remote_port_rolechg(struct fc_rport  
 
 		spin_lock_irqsave(shost->host_lock, flags);
 		rport->flags &= ~(FC_RPORT_FAST_FAIL_TIMEDOUT |
-				  FC_RPORT_DEVLOSS_PENDING);
+				  FC_RPORT_DEVLOSS_PENDING |
+				  FC_RPORT_DEVLOSS_CALLBK_DONE);
 		spin_unlock_irqrestore(shost->host_lock, flags);
 
 		/* ensure any stgt delete functions are done */

Re: [PATCH 1/1] [SCSI] Clear Devloss Callback Done flag in fc_remote_port_rolechg

[James Smart]

James,

Ping...  anything holding up inclusion of this patch ?

-- james s


On 11/3/2011 11:10 AM, James Smart wrote:
> This patch fixes a bug where devloss is not called on fc_host teardown.
> The issue is seen if the LLDD uses rport_rolechg to add the target role
> to an rport.
>
> When an rport goes away, the LLDD will call fc_remote_port_delete, which
> will start the devloss timer.  If the timer expires, the transport will
> call the devloss callback and set the FC_RPORT_DEVLOSS_CALLBK_DONE flag.
> However, the rport structure is not deleted, it is retained to store the
> SCSI id mappings for the rport in case it comes back. In the scenario
> where it does come back, and the driver calls fc_remote_port_add, but does
> not indicate the "target" role for the rport - the create will clear the
> structure, but forgets to clear FC_RPORT_DEVLOSS_CALLBK_DONE flag (which
> is cleared if it's added with the target role). The secondary call, of
> fc_remote_port_rolechg to add the target role also does not clear the flag.
> Thus, the next time the rport goes away, the resulting devloss timer
> expiration will not call the driver callback as the flag is still set.
>
> This patch adds the FC_RPORT_DEVLOSS_CALLBK_DONE flags to the list of
> those that are cleared upon reuse of the rport structure.
>
>   Signed-off-by: Alex Iannicelli<alex.iannicelli@emulex.com>
>   Signed-off-by: James Smart<james.smart@emulex.com>
>
>   ---
>
>   scsi_transport_fc.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
>
> diff -upNr a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> --- a/drivers/scsi/scsi_transport_fc.c	2011-11-03 10:27:14.000000000 -0400
> +++ b/drivers/scsi/scsi_transport_fc.c	2011-11-03 10:28:59.000000000 -0400
> @@ -3048,7 +3048,8 @@ fc_remote_port_rolechg(struct fc_rport
>
>   		spin_lock_irqsave(shost->host_lock, flags);
>   		rport->flags&= ~(FC_RPORT_FAST_FAIL_TIMEDOUT |
> -				  FC_RPORT_DEVLOSS_PENDING);
> +				  FC_RPORT_DEVLOSS_PENDING |
> +				  FC_RPORT_DEVLOSS_CALLBK_DONE);
>   		spin_unlock_irqrestore(shost->host_lock, flags);
>
>   		/* ensure any stgt delete functions are done */
>
>