On Thu, 2015-10-22 at 15:57 -0700, Bart Van Assche wrote:
> This patch fixes the following kernel warning because it avoids that
> IRQs are disabled while ft_release_cmd() is invoked (fc_seq_set_resp()
> invokes spin_unlock_bh()):
>
> WARNING: CPU: 3 PID: 117 at kernel/softirq.c:150 __local_bh_enable_ip+0xaa/0x110()
> Call Trace:
> [<ffffffff814f71eb>] dump_stack+0x4f/0x7b
> [<ffffffff8105e56a>] warn_slowpath_common+0x8a/0xc0
> [<ffffffff8105e65a>] warn_slowpath_null+0x1a/0x20
> [<ffffffff81062b2a>] __local_bh_enable_ip+0xaa/0x110
> [<ffffffff814ff229>] _raw_spin_unlock_bh+0x39/0x40
> [<ffffffffa03a7f94>] fc_seq_set_resp+0xe4/0x100 [libfc]
> [<ffffffffa02e604a>] ft_free_cmd+0x4a/0x90 [tcm_fc]
> [<ffffffffa02e6972>] ft_release_cmd+0x12/0x20 [tcm_fc]
> [<ffffffffa042bd66>] target_release_cmd_kref+0x56/0x90 [target_core_mod]
> [<ffffffffa042caf0>] target_put_sess_cmd+0xc0/0x110 [target_core_mod]
> [<ffffffffa042cb81>] transport_release_cmd+0x41/0x70 [target_core_mod]
> [<ffffffffa042d975>] transport_generic_free_cmd+0x35/0x420 [target_core_mod]
>
> Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
> Acked-by: Joern Engel <joern@xxxxxxxxx>
> Reviewed-by: Andy Grover <agrover@xxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Hannes Reinecke <hare@xxxxxxx>
> Cc: Sagi Grimberg <sagig@xxxxxxxxxxxx>
> ---
> drivers/target/target_core_tmr.c | 7 ++++++-
> drivers/target/target_core_transport.c | 12 ++++++------
> 2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
> index 7137042..fcdcb11 100644
> --- a/drivers/target/target_core_tmr.c
> +++ b/drivers/target/target_core_tmr.c
> @@ -130,6 +130,9 @@ void core_tmr_abort_task(
> if (tmr->ref_task_tag != ref_tag)
> continue;
>
> + if (!kref_get_unless_zero(&se_cmd->cmd_kref))
> + continue;
> +
> printk("ABORT_TASK: Found referenced %s task_tag: %llu\n",
> se_cmd->se_tfo->get_fabric_name(), ref_tag);
>
> @@ -139,13 +142,15 @@ void core_tmr_abort_task(
> " skipping\n", ref_tag);
> spin_unlock(&se_cmd->t_state_lock);
> spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
> +
> + target_put_sess_cmd(se_cmd);
> +
> goto out;
> }
> se_cmd->transport_state |= CMD_T_ABORTED;
> spin_unlock(&se_cmd->t_state_lock);
>
> list_del_init(&se_cmd->se_cmd_list);
> - kref_get(&se_cmd->cmd_kref);
> spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
>
> cancel_work_sync(&se_cmd->work);
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index eae4924..7f4d3ac 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -2507,23 +2507,24 @@ out:
> EXPORT_SYMBOL(target_get_sess_cmd);
>
> static void target_release_cmd_kref(struct kref *kref)
> - __releases(&se_cmd->se_sess->sess_cmd_lock)
> {
> struct se_cmd *se_cmd = container_of(kref, struct se_cmd, cmd_kref);
> struct se_session *se_sess = se_cmd->se_sess;
> + unsigned long flags;
>
> + spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
> if (list_empty(&se_cmd->se_cmd_list)) {
> - spin_unlock(&se_sess->sess_cmd_lock);
> + spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
> se_cmd->se_tfo->release_cmd(se_cmd);
> return;
> }
> if (se_sess->sess_tearing_down && se_cmd->cmd_wait_set) {
> - spin_unlock(&se_sess->sess_cmd_lock);
> + spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
> complete(&se_cmd->cmd_wait_comp);
> return;
> }
> list_del(&se_cmd->se_cmd_list);
> - spin_unlock(&se_sess->sess_cmd_lock);
> + spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
>
> se_cmd->se_tfo->release_cmd(se_cmd);
> }
> @@ -2539,8 +2540,7 @@ int target_put_sess_cmd(struct se_cmd *se_cmd)
> se_cmd->se_tfo->release_cmd(se_cmd);
> return 1;
> }
> - return kref_put_spinlock_irqsave(&se_cmd->cmd_kref, target_release_cmd_kref,
> - &se_sess->sess_cmd_lock);
> + return kref_put(&se_cmd->cmd_kref, target_release_cmd_kref);
> }
> EXPORT_SYMBOL(target_put_sess_cmd);
>
Being able to get rid of kref_put_spinlock_irqsave(), and avoid having
to invoke TFO->release_cmd() with interrupts disabled is a nice
improvement.
Since this addresses a warning with FCoE, it's been added to
target-pending/queue-fixes and will go out in the next -rc fixes PULL.
For patch #21 to drop the now unused code in kref.h, it has been added
to queue-next as v4.5 code.
--
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
