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 linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to