tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
fork atom

iscsi-target: Fix iscsit_free_cmd() se_cmd->cmd_kref shutdown handling

With the introduction of target_get_sess_cmd() referencing counting for
ISCSI_OP_SCSI_CMD processing with iser-target, iscsit_free_cmd() usage
in traditional iscsi-target driver code now needs to be aware of the
active I/O shutdown case when a remaining se_cmd->cmd_kref reference may
exist after transport_generic_free_cmd() completes, requiring a final
target_put_sess_cmd() to release iscsi_cmd descriptor memory.

This patch changes iscsit_free_cmd() to invoke __iscsit_free_cmd() before
transport_generic_free_cmd() -> target_put_sess_cmd(), and also avoids
aquiring the per-connection queue locks for typical fast-path calls
during normal ISTATE_REMOVE operation.

Also update iscsit_free_cmd() usage throughout iscsi-target to
use the new 'bool shutdown' parameter.

This patch fixes a regression bug introduced during v3.10-rc1 in
commit 3e1c81a95, that was causing the following WARNING to appear:

[ 257.235153] ------------[ cut here]------------
[ 257.240314] WARNING: at kernel/softirq.c:160 local_bh_enable_ip+0x3c/0x86()
[ 257.248089] Modules linked in: vhost_scsi ib_srpt ib_cm ib_sa ib_mad ib_core tcm_qla2xxx tcm_loop
tcm_fc libfc iscsi_target_mod target_core_pscsi target_core_file
target_core_iblock target_core_mod configfs ipv6 iscsi_tcp libiscsi_tcp
libiscsi scsi_transport_iscsi loop acpi_cpufreq freq_table mperf
kvm_intel kvm crc32c_intel button ehci_pci pcspkr joydev i2c_i801
microcode ext3 jbd raid10 raid456 async_pq async_xor xor async_memcpy
async_raid6_recov raid6_pq async_tx raid1 raid0 linear igb hwmon
i2c_algo_bit i2c_core ptp ata_piix libata qla2xxx uhci_hcd ehci_hcd
mlx4_core scsi_transport_fc scsi_tgt pps_core
[ 257.308748] CPU: 1 PID: 3295 Comm: iscsi_ttx Not tainted 3.10.0-rc2+ #103
[ 257.316329] Hardware name: Intel Corporation S5520HC/S5520HC, BIOS S5500.86B.01.00.0057.031020111721 03/10/2011
[ 257.327597] ffffffff814c24b7 ffff880458331b58 ffffffff8138eef2 ffff880458331b98
[ 257.335892] ffffffff8102c052 ffff880400000008 0000000000000000 ffff88085bdf0000
[ 257.344191] ffff88085bdf00d8 ffff88085bdf00e0 ffff88085bdf00f8 ffff880458331ba8
[ 257.352488] Call Trace:
[ 257.355223] [<ffffffff8138eef2>] dump_stack+0x19/0x1f
[ 257.360963] [<ffffffff8102c052>] warn_slowpath_common+0x62/0x7b
[ 257.367669] [<ffffffff8102c080>] warn_slowpath_null+0x15/0x17
[ 257.374181] [<ffffffff81032345>] local_bh_enable_ip+0x3c/0x86
[ 257.380697] [<ffffffff813917fd>] _raw_spin_unlock_bh+0x10/0x12
[ 257.387311] [<ffffffffa029069c>] iscsit_free_r2ts_from_list+0x5e/0x67 [iscsi_target_mod]
[ 257.396438] [<ffffffffa02906c5>] iscsit_release_cmd+0x20/0x223 [iscsi_target_mod]
[ 257.404893] [<ffffffffa02977a4>] lio_release_cmd+0x3a/0x3e [iscsi_target_mod]
[ 257.412964] [<ffffffffa01d59a1>] target_release_cmd_kref+0x7a/0x7c [target_core_mod]
[ 257.421712] [<ffffffffa01d69bc>] target_put_sess_cmd+0x5f/0x7f [target_core_mod]
[ 257.430071] [<ffffffffa01d6d6d>] transport_release_cmd+0x59/0x6f [target_core_mod]
[ 257.438625] [<ffffffffa01d6eb4>] transport_put_cmd+0x131/0x140 [target_core_mod]
[ 257.446985] [<ffffffffa01d6192>] ? transport_wait_for_tasks+0xfa/0x1d5 [target_core_mod]
[ 257.456121] [<ffffffffa01d6f11>] transport_generic_free_cmd+0x4e/0x52 [target_core_mod]
[ 257.465159] [<ffffffff81050537>] ? __migrate_task+0x110/0x110
[ 257.471674] [<ffffffffa02904ba>] iscsit_free_cmd+0x46/0x55 [iscsi_target_mod]
[ 257.479741] [<ffffffffa0291edb>] iscsit_immediate_queue+0x301/0x353 [iscsi_target_mod]
[ 257.488683] [<ffffffffa0292f7e>] iscsi_target_tx_thread+0x1c6/0x2a8 [iscsi_target_mod]
[ 257.497623] [<ffffffff81047486>] ? wake_up_bit+0x25/0x25
[ 257.503652] [<ffffffffa0292db8>] ? iscsit_ack_from_expstatsn+0xd5/0xd5 [iscsi_target_mod]
[ 257.512882] [<ffffffff81046f89>] kthread+0xb0/0xb8
[ 257.518329] [<ffffffff81046ed9>] ? kthread_freezable_should_stop+0x60/0x60
[ 257.526105] [<ffffffff81396fec>] ret_from_fork+0x7c/0xb0
[ 257.532133] [<ffffffff81046ed9>] ? kthread_freezable_should_stop+0x60/0x60
[ 257.539906] ---[ end trace 5520397d0f2e0800 ]---

Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

Nicholas Bellinger aafc9d15 d5ddad41

options
unified split
Changed files
+52 -28
drivers
target
iscsi
iscsi_target.c
iscsi_target_erl2.c
iscsi_target_util.c
iscsi_target_util.h
+6 -6
drivers/target/iscsi/iscsi_target.c
··· 651 651 cmd->buf_ptr = kmemdup(buf, ISCSI_HDR_LEN, GFP_KERNEL); 652 652 if (!cmd->buf_ptr) { 653 653 pr_err("Unable to allocate memory for cmd->buf_ptr\n"); 654 - iscsit_release_cmd(cmd); 654 + iscsit_free_cmd(cmd, false); 655 655 return -1; 656 656 } 657 657 ··· 697 697 cmd->buf_ptr = kmemdup(buf, ISCSI_HDR_LEN, GFP_KERNEL); 698 698 if (!cmd->buf_ptr) { 699 699 pr_err("Unable to allocate memory for cmd->buf_ptr\n"); 700 - iscsit_release_cmd(cmd); 700 + iscsit_free_cmd(cmd, false); 701 701 return -1; 702 702 } 703 703 ··· 1743 1743 return 0; 1744 1744 out: 1745 1745 if (cmd) 1746 - iscsit_release_cmd(cmd); 1746 + iscsit_free_cmd(cmd, false); 1747 1747 ping_out: 1748 1748 kfree(ping_data); 1749 1749 return ret; ··· 2251 2251 if (conn->conn_state != TARG_CONN_STATE_LOGGED_IN) { 2252 2252 pr_err("Received logout request on connection that" 2253 2253 " is not in logged in state, ignoring request.\n"); 2254 - iscsit_release_cmd(cmd); 2254 + iscsit_free_cmd(cmd, false); 2255 2255 return 0; 2256 2256 } 2257 2257 ··· 3665 3665 list_del(&cmd->i_conn_node); 3666 3666 spin_unlock_bh(&conn->cmd_lock); 3667 3667 3668 - iscsit_free_cmd(cmd); 3668 + iscsit_free_cmd(cmd, false); 3669 3669 break; 3670 3670 case ISTATE_SEND_NOPIN_WANT_RESPONSE: 3671 3671 iscsit_mod_nopin_response_timer(conn); ··· 4122 4122 4123 4123 iscsit_increment_maxcmdsn(cmd, sess); 4124 4124 4125 - iscsit_free_cmd(cmd); 4125 + iscsit_free_cmd(cmd, true); 4126 4126 4127 4127 spin_lock_bh(&conn->cmd_lock); 4128 4128 }
+6 -6
drivers/target/iscsi/iscsi_target_erl2.c
··· 143 143 list_del(&cmd->i_conn_node); 144 144 cmd->conn = NULL; 145 145 spin_unlock(&cr->conn_recovery_cmd_lock); 146 - iscsit_free_cmd(cmd); 146 + iscsit_free_cmd(cmd, true); 147 147 spin_lock(&cr->conn_recovery_cmd_lock); 148 148 } 149 149 spin_unlock(&cr->conn_recovery_cmd_lock); ··· 165 165 list_del(&cmd->i_conn_node); 166 166 cmd->conn = NULL; 167 167 spin_unlock(&cr->conn_recovery_cmd_lock); 168 - iscsit_free_cmd(cmd); 168 + iscsit_free_cmd(cmd, true); 169 169 spin_lock(&cr->conn_recovery_cmd_lock); 170 170 } 171 171 spin_unlock(&cr->conn_recovery_cmd_lock); ··· 248 248 iscsit_remove_cmd_from_connection_recovery(cmd, sess); 249 249 250 250 spin_unlock(&cr->conn_recovery_cmd_lock); 251 - iscsit_free_cmd(cmd); 251 + iscsit_free_cmd(cmd, true); 252 252 spin_lock(&cr->conn_recovery_cmd_lock); 253 253 } 254 254 spin_unlock(&cr->conn_recovery_cmd_lock); ··· 302 302 list_del(&cmd->i_conn_node); 303 303 304 304 spin_unlock_bh(&conn->cmd_lock); 305 - iscsit_free_cmd(cmd); 305 + iscsit_free_cmd(cmd, true); 306 306 spin_lock_bh(&conn->cmd_lock); 307 307 } 308 308 spin_unlock_bh(&conn->cmd_lock); ··· 355 355 356 356 list_del(&cmd->i_conn_node); 357 357 spin_unlock_bh(&conn->cmd_lock); 358 - iscsit_free_cmd(cmd); 358 + iscsit_free_cmd(cmd, true); 359 359 spin_lock_bh(&conn->cmd_lock); 360 360 continue; 361 361 } ··· 375 375 iscsi_sna_gte(cmd->cmd_sn, conn->sess->exp_cmd_sn)) { 376 376 list_del(&cmd->i_conn_node); 377 377 spin_unlock_bh(&conn->cmd_lock); 378 - iscsit_free_cmd(cmd); 378 + iscsit_free_cmd(cmd, true); 379 379 spin_lock_bh(&conn->cmd_lock); 380 380 continue; 381 381 }
+39 -15
drivers/target/iscsi/iscsi_target_util.c
··· 676 676 677 677 void iscsit_release_cmd(struct iscsi_cmd *cmd) 678 678 { 679 - struct iscsi_conn *conn = cmd->conn; 680 - 681 - iscsit_free_r2ts_from_list(cmd); 682 - iscsit_free_all_datain_reqs(cmd); 683 - 684 679 kfree(cmd->buf_ptr); 685 680 kfree(cmd->pdu_list); 686 681 kfree(cmd->seq_list); 687 682 kfree(cmd->tmr_req); 688 683 kfree(cmd->iov_data); 689 684 690 - if (conn) { 691 - iscsit_remove_cmd_from_immediate_queue(cmd, conn); 692 - iscsit_remove_cmd_from_response_queue(cmd, conn); 693 - } 694 - 695 685 kmem_cache_free(lio_cmd_cache, cmd); 696 686 } 697 687 698 - void iscsit_free_cmd(struct iscsi_cmd *cmd) 688 + static void __iscsit_free_cmd(struct iscsi_cmd *cmd, bool scsi_cmd, 689 + bool check_queues) 699 690 { 691 + struct iscsi_conn *conn = cmd->conn; 692 + 693 + if (scsi_cmd) { 694 + if (cmd->data_direction == DMA_TO_DEVICE) { 695 + iscsit_stop_dataout_timer(cmd); 696 + iscsit_free_r2ts_from_list(cmd); 697 + } 698 + if (cmd->data_direction == DMA_FROM_DEVICE) 699 + iscsit_free_all_datain_reqs(cmd); 700 + } 701 + 702 + if (conn && check_queues) { 703 + iscsit_remove_cmd_from_immediate_queue(cmd, conn); 704 + iscsit_remove_cmd_from_response_queue(cmd, conn); 705 + } 706 + } 707 + 708 + void iscsit_free_cmd(struct iscsi_cmd *cmd, bool shutdown) 709 + { 710 + struct se_cmd *se_cmd = NULL; 711 + int rc; 700 712 /* 701 713 * Determine if a struct se_cmd is associated with 702 714 * this struct iscsi_cmd. 703 715 */ 704 716 switch (cmd->iscsi_opcode) { 705 717 case ISCSI_OP_SCSI_CMD: 706 - if (cmd->data_direction == DMA_TO_DEVICE) 707 - iscsit_stop_dataout_timer(cmd); 718 + se_cmd = &cmd->se_cmd; 719 + __iscsit_free_cmd(cmd, true, shutdown); 708 720 /* 709 721 * Fallthrough 710 722 */ 711 723 case ISCSI_OP_SCSI_TMFUNC: 712 - transport_generic_free_cmd(&cmd->se_cmd, 1); 724 + rc = transport_generic_free_cmd(&cmd->se_cmd, 1); 725 + if (!rc && shutdown && se_cmd && se_cmd->se_sess) { 726 + __iscsit_free_cmd(cmd, true, shutdown); 727 + target_put_sess_cmd(se_cmd->se_sess, se_cmd); 728 + } 713 729 break; 714 730 case ISCSI_OP_REJECT: 715 731 /* ··· 734 718 * associated cmd->se_cmd needs to be released. 735 719 */ 736 720 if (cmd->se_cmd.se_tfo != NULL) { 737 - transport_generic_free_cmd(&cmd->se_cmd, 1); 721 + se_cmd = &cmd->se_cmd; 722 + __iscsit_free_cmd(cmd, true, shutdown); 723 + 724 + rc = transport_generic_free_cmd(&cmd->se_cmd, 1); 725 + if (!rc && shutdown && se_cmd->se_sess) { 726 + __iscsit_free_cmd(cmd, true, shutdown); 727 + target_put_sess_cmd(se_cmd->se_sess, se_cmd); 728 + } 738 729 break; 739 730 } 740 731 /* Fall-through */ 741 732 default: 733 + __iscsit_free_cmd(cmd, false, shutdown); 742 734 cmd->release_cmd(cmd); 743 735 break; 744 736 }
+1 -1
drivers/target/iscsi/iscsi_target_util.h
··· 29 29 extern bool iscsit_conn_all_queues_empty(struct iscsi_conn *); 30 30 extern void iscsit_free_queue_reqs_for_conn(struct iscsi_conn *); 31 31 extern void iscsit_release_cmd(struct iscsi_cmd *); 32 - extern void iscsit_free_cmd(struct iscsi_cmd *); 32 + extern void iscsit_free_cmd(struct iscsi_cmd *, bool); 33 33 extern int iscsit_check_session_usage_count(struct iscsi_session *); 34 34 extern void iscsit_dec_session_usage_count(struct iscsi_session *); 35 35 extern void iscsit_inc_session_usage_count(struct iscsi_session *);