On Fri, 2017-06-02 at 22:01 -0700, Nicholas A. Bellinger wrote:
> On Fri, 2017-06-02 at 18:14 +0000, Bart Van Assche wrote:
> > Hello Nic,
> >
> > When I reran the libiscsi test suite against your for-next branch a kernel oops
> > appeared in the system log that I hadn't seen before. There are no iSCSI patches
> > from me on that branch so this crash was likely introduced by one of the iSCSI
> > target driver patches that were added to your for-next branch after kernel v4.11
> > was released. The topmost commit in the kernel tree that triggered this oops is
> > commit acdd4716bc86 ("target: reject COMPARE_AND_WRITE if emulate_caw is not set").
> >
>
> Yep, nothing immediate comes to mind in the explicit logout path that
> has changed recently.
>
> > [ 321.546438] iscsi_target_mod:lio_release_cmd: Entering lio_release_cmd for se_cmd: ffff880063134890
> > [ 323.013563] 1 connection(s) still exist for iSCSI session to iqn.2007-10.com.github:sahlberg:libiscsi:iscsi-test-2
> > [ 323.014358] ------------[ cut here ]------------
> > [ 323.014864] kernel BUG at drivers/target/iscsi/iscsi_target.c:4346!
It turns out the only way to trigger this is to block tx thread context
from reaching iscsit_logout_post_handler_closesession() for longer than
SECONDS_FOR_LOGOUT_COMP (15 seconds), which causes sleep = 0 to be
processed while iscsit_close_connection() from rx thread context has
already cleared conn->tx_thread_active.
As it was, I have no idea what you did in your VM to cause a 15+ second
delay to reach iscsit_logout_post_handler_closesession(), but whatever
it was it's certainly not a regression in upstream. ;)
In any event, here's how to simulate the issue, and the proper fix to
just let existing iscsit_close_connection() logic clean up the failed
logout as if iscsit_logout_post_handler_closesession() was never
reached.
Enjoy.
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 0d8f815..03a0224 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -4414,6 +4414,11 @@ static void iscsit_logout_post_handler_closesession(
{
struct iscsi_session *sess = conn->sess;
int sleep = 1;
+
+ printk("Simulating broken out-of-tree codebase.\n");
+ ssleep(SECONDS_FOR_LOGOUT_COMP + 2);
+ printk("Simulation complete\n");
+
/*
* Traditional iscsi/tcp will invoke this logic from TX thread
* context during session logout, so clear tx_thread_active and
@@ -4423,8 +4428,11 @@ static void iscsit_logout_post_handler_closesession(
* always sleep waiting for RX/TX thread shutdown to complete
* within iscsit_close_connection().
*/
- if (!conn->conn_transport->rdma_shutdown)
+ if (!conn->conn_transport->rdma_shutdown) {
sleep = cmpxchg(&conn->tx_thread_active, true, false);
+ if (!sleep)
+ return;
+ }
atomic_set(&conn->conn_logout_remove, 0);
complete(&conn->conn_logout_comp);
@@ -4440,8 +4448,11 @@ static void iscsit_logout_post_handler_samecid(
{
int sleep = 1;
- if (!conn->conn_transport->rdma_shutdown)
+ if (!conn->conn_transport->rdma_shutdown) {
sleep = cmpxchg(&conn->tx_thread_active, true, false);
+ if (!sleep)
+ return;
+ }
atomic_set(&conn->conn_logout_remove, 0);
complete(&conn->conn_logout_comp);
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
