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