On Sun, 2011-11-20 at 16:32 -0500, Christoph Hellwig wrote: > > /* ha->hardware_lock supposed to be held on entry */ > > @@ -3188,11 +3195,19 @@ static int qla_tgt_handle_cmd_for_atio(struct scsi_qla_host *vha, > > if (sess->tearing_down || tgt->tgt_stop) > > goto out_free_cmd; > > > > - res = qla_tgt_send_cmd_to_target(vha, cmd, sess); > > - if (unlikely(res != 0)) > > - goto out_free_cmd; > > + cmd->sess = sess; > > + cmd->loop_id = sess->loop_id; > > + cmd->conf_compl_supported = sess->conf_compl_supported; > > + /* > > + * Get the extra kref_get() before dropping qla_hw_data->hardware_lock, > > + * and call qla_tgt_sess_put() -> kref_put() in qla_tgt_do_work() process > > + * context to drop the extra reference. > > + */ > > + kref_get(&sess->sess_kref); > > > > - return res; > > + INIT_WORK(&cmd->work, qla_tgt_do_work); > > + queue_work(qla_tgt_wq, &cmd->work); > > + return 0; > > > > out_free_cmd: > > qla_tgt_free_cmd(cmd); > > @@ -3204,9 +3219,8 @@ out_sched: > > res = -EBUSY; > > goto out_free_cmd; > > } > > - > > - INIT_WORK(&cmd->work, qla_tgt_exec_cmd_work); > > - schedule_work(&cmd->work); > > + INIT_WORK(&cmd->work, qla_tgt_do_work); > > + queue_work(qla_tgt_wq, &cmd->work); > > return 0; > > This function now has two indentical pathes to queue up work, > which make little sense. Fixing this up the goto usage in qla_tgt_handle_cmd_for_atio().. > Is there any good reason grabbing the > reference to sess_kref can't unconditionally be done in user > context? > I'm not completely sure with current ->hardware_lock usage with the active I/O shutdown path when no session is present.. We may end up leaking these qla_tgt_cmd allocations w/o an session reference during session shutdown if qla_target code does not wait for these to be released. I still think we will need some form of session reference in interrupt context if we are going to completely leave qla_tgt_cmd->se_cmd init up to process context, but would be happily proved wrong if active I/O shutdown can still function as expected. --nab -- 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
