On Tue, 2016-01-12 at 16:25 +0100, Christoph Hellwig wrote:
> > if (dev) {
> > spin_lock_irqsave(&dev->se_tmr_lock, flags);
> > - list_del(&tmr->tmr_list);
> > + if (!list_empty(&tmr->tmr_list))
> > + list_del_init(&tmr->tmr_list);
>
> list_del_init on a empty list is fine.
>
Done.
> > + sess = cmd->se_sess;
> > + if (!sess) {
> > + dump_stack();
> > + continue;
> > + }
>
> Why not something like:
>
> if (WARN_ON_ONCE(!sess))
> continue;
>
> same for the previous patch.
>
Done.
> > + spin_lock(&sess->sess_cmd_lock);
> > + rc = kref_get_unless_zero(&cmd->cmd_kref);
> > + spin_unlock(&sess->sess_cmd_lock);
>
> no need to take a lock around kref_get_unless_zero, it's not going to
> help with anything.
So for -v2, this lock is being obtained before ->t_state_lock above, to
follow how __target_check_io_state() works with aborted I/O + LUN_RESET
and explicit TMR_ABORT_TASK.
It's been made consistent with the other cases for now, but depending on
how the bug-fix for se_session shutdown plus remote port LUN_RESET works
out, this may or may-not be able to go away for -v3.
--
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
