| Message ID | 1357394162-26316-1-git-send-email-dongsu.park@profitbricks.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 01/05/13 14:56, Dongsu Park wrote: > There has been a nasty problem upon removing an SRP target when the SRP > target machine crashed accidently without giving back any IB events. In > that case, the admin cannot make use of deleting remote ports for the > purpose of tearing down SRP targets as well as SCSI host. One of the > reasons was a completion on target->done, the other was the invocation > order of srp_disconnect_target() and scsi_remove_host(). Consequence of > the latter was unfortunately hanging forever on device_del(), until the > target machine comes up again after having rebooted. > > That symptom is simply reproducible via sysfs. First of all, trigger an > immediate reboot via /proc/sysrq-trigger on the SRP target. > > target# echo b > /proc/sysrq-trigger > > After doing that, the Infiniband connection will be completely gone for > several minutes at least. Then on the initiator's side, delete a remote > port by writing 1 to /sys/class/srp_remote_ports/port-*\:1/delete, e.g.: > > initiator# echo 1 > /sys/class/srp_remote_ports/port-6\:1/delete > > , where the SRP remote port to be deleted has its number 6. > > Then you will see stale SCSI targets remaining despite of rport delete, > which is not expected though. That was resulted from device_del() > hanging forever on destroying SCSI LLD. > > The solution consists of two modifications. The first one was already > committed to jejb/for-next. See the commit 55d93898 "IB/srp: send > disconnect request without waiting for CM timewait exit" by Vu Pham > <vu@mellanox.com>. That will prevent from waiting for completion. > > The next one, which this commit is saying about, is changing the > invocation order in srp_remove_target(). Call srp_disconnect_target() > before scsi_remove_host(). This change will prevent device_del() from > hanging indefinitely. > > This patch is based on the srp-ha-v3.7 tree by Bart Van Assche. > See also <https://github.com/advance38/linux/tree/ib-srp-remove-target-v3.7>. > If necessary, I could rebase it on the stable tree. > > Signed-off-by: Dongsu Park <dongsu.park@profitbricks.com> > Cc: Sebastian Riemer <sebastian.riemer@profitbricks.com> > Cc: Bart Van Assche <bvanassche@acm.org> > Cc: David Dillow <dillowda@ornl.gov> > Cc: Roland Dreier <roland@kernel.org> > Cc: Sean Hefty <sean.hefty@intel.com> > Cc: Hal Rosenstock <hal.rosenstock@gmail.com> > --- > drivers/infiniband/ulp/srp/ib_srp.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c > index 307430e..ca4bf40 100644 > --- a/drivers/infiniband/ulp/srp/ib_srp.c > +++ b/drivers/infiniband/ulp/srp/ib_srp.c > @@ -553,10 +553,11 @@ static void srp_remove_target(struct srp_target_port *target) > if (scsi_host_added) { > srp_del_scsi_host_attr(shost); > srp_remove_host(shost); > + srp_disconnect_target(target); > scsi_remove_host(shost); > - } > + } else > + srp_disconnect_target(target); > > - srp_disconnect_target(target); > ib_destroy_cm_id(target->cm_id); > cancel_work_sync(&target->tl_err_work); > srp_free_target_ib(target); Sorry but this patch looks wrong to me, and that because of the following reasons: - A root cause analysis is missing. It has been mentioned in the patch description that device_del() did hang but an analysis of why that hang occurred is missing. - An explanation of why the above patch prevents device_del() to hang is missing. - Invoking srp_disconnect_target() before scsi_remove_host() is wrong because it prevents the SYNCHRONIZE CACHE command issued by sd_shutdown() to reach the SRP target. Note: although I'm not sure which issue exactly you ran into, this patch may help: "[PATCH for-next] IB/srp: Make SCSI error handling finish" (http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg13711.html). Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2013-01-07 at 06:34 -0500, Bart Van Assche wrote: > Sorry but this patch looks wrong to me, and that because of the > following reasons: > - A root cause analysis is missing. It has been mentioned in the patch > description that device_del() did hang but an analysis of why that > hang occurred is missing. > - An explanation of why the above patch prevents device_del() to hang is > missing. Agreed, this needs more explanation -- at least in the email thread, and boiled down in the commit message. > - Invoking srp_disconnect_target() before scsi_remove_host() is wrong > because it prevents the SYNCHRONIZE CACHE command issued by > sd_shutdown() to reach the SRP target. Exactly; we need to understand the hang so that we get a proper shutdown.
Hi Bart, On 07.01.2013 12:34, Bart Van Assche wrote: > Sorry but this patch looks wrong to me, and that because of the > following reasons: > - A root cause analysis is missing. It has been mentioned in the patch > description that device_del() did hang but an analysis of why that > hang occurred is missing. > - An explanation of why the above patch prevents device_del() to hang is > missing. > - Invoking srp_disconnect_target() before scsi_remove_host() is wrong > because it prevents the SYNCHRONIZE CACHE command issued by > sd_shutdown() to reach the SRP target. I'm sorry for confusion. Please ignore my patch. I agree with you, that was not the solution. Last week there was a confusion in our local test environment. Let me describe the problem more clearly. Let's say an SRP target machine crashed accidently without giving back any IB events. Immediately after that crash, on the initiator-side, the administrator tries to destroy the SRP target or deleting remote port, in order to perform any emergency action. However, that action will hang forever until the target machine comes up again. Precisely it's blocked on scsi_execute() directly after sending SYNCHRONIZE_CACHE command to the first target of the host. As IB stack is not able to give any response, further target remove cannot be done. After doing git bisect, I found out 1 commit that causes the problem: 4b2e8ea "IB/srp: Keep processing commands during host removal". Reverting this commit, the problem is gone. No blocking any more. But the symptom seems to differ slightly according to kernel versions. With srp-ha v4/v5 on top of kernel 3.4.23, the revert is necessary. But on your current tree srp-ha-v3.7 on top of kernel 3.7, the revert is not needed any more. It just works without blocking on target remove. I'm not sure exactly what has changed between 3.4.23 and 3.7. Anyway I'm now satisfied with the current revert patch on top of 3.4.23. See also my github branches. <https://github.com/advance38/linux/tree/srp-ha-v4-3.4.23> > Note: although I'm not sure which issue exactly you ran into, this > patch may help: "[PATCH for-next] IB/srp: Make SCSI error handling > finish" (http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg13711.html). No, that was not the problem I meant. The patch didn't bring anything either. Regards, Dongsu -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/11/13 15:07, Dongsu Park wrote: > However, that action will hang forever until the target machine comes up > again. Precisely it's blocked on scsi_execute() directly after sending > SYNCHRONIZE_CACHE command to the first target of the host. As IB stack > is not able to give any response, further target remove cannot be done. This is the part that needs further research. If a SCSI command is not yet finished after the SCSI timeout expired the SCSI error handler will try to abort the command. scsi_execute() should finish in time. If not, something is wrong either in the LLD (ib_srp), the SCSI core or in the block layer. It would help to know what exactly was going on. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 307430e..ca4bf40 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -553,10 +553,11 @@ static void srp_remove_target(struct srp_target_port *target) if (scsi_host_added) { srp_del_scsi_host_attr(shost); srp_remove_host(shost); + srp_disconnect_target(target); scsi_remove_host(shost); - } + } else + srp_disconnect_target(target); - srp_disconnect_target(target); ib_destroy_cm_id(target->cm_id); cancel_work_sync(&target->tl_err_work); srp_free_target_ib(target);
There has been a nasty problem upon removing an SRP target when the SRP target machine crashed accidently without giving back any IB events. In that case, the admin cannot make use of deleting remote ports for the purpose of tearing down SRP targets as well as SCSI host. One of the reasons was a completion on target->done, the other was the invocation order of srp_disconnect_target() and scsi_remove_host(). Consequence of the latter was unfortunately hanging forever on device_del(), until the target machine comes up again after having rebooted. That symptom is simply reproducible via sysfs. First of all, trigger an immediate reboot via /proc/sysrq-trigger on the SRP target. target# echo b > /proc/sysrq-trigger After doing that, the Infiniband connection will be completely gone for several minutes at least. Then on the initiator's side, delete a remote port by writing 1 to /sys/class/srp_remote_ports/port-*\:1/delete, e.g.: initiator# echo 1 > /sys/class/srp_remote_ports/port-6\:1/delete , where the SRP remote port to be deleted has its number 6. Then you will see stale SCSI targets remaining despite of rport delete, which is not expected though. That was resulted from device_del() hanging forever on destroying SCSI LLD. The solution consists of two modifications. The first one was already committed to jejb/for-next. See the commit 55d93898 "IB/srp: send disconnect request without waiting for CM timewait exit" by Vu Pham <vu@mellanox.com>. That will prevent from waiting for completion. The next one, which this commit is saying about, is changing the invocation order in srp_remove_target(). Call srp_disconnect_target() before scsi_remove_host(). This change will prevent device_del() from hanging indefinitely. This patch is based on the srp-ha-v3.7 tree by Bart Van Assche. See also <https://github.com/advance38/linux/tree/ib-srp-remove-target-v3.7>. If necessary, I could rebase it on the stable tree. Signed-off-by: Dongsu Park <dongsu.park@profitbricks.com> Cc: Sebastian Riemer <sebastian.riemer@profitbricks.com> Cc: Bart Van Assche <bvanassche@acm.org> Cc: David Dillow <dillowda@ornl.gov> Cc: Roland Dreier <roland@kernel.org> Cc: Sean Hefty <sean.hefty@intel.com> Cc: Hal Rosenstock <hal.rosenstock@gmail.com> --- drivers/infiniband/ulp/srp/ib_srp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)