On Mon, 2012-07-16 at 16:05 -0700, Roland Dreier wrote: > On Mon, Jul 16, 2012 at 4:00 PM, Nicholas A. Bellinger > <nab@xxxxxxxxxxxxxxx> wrote: > > Mmmm. The original target_submit_cmd() code had been propagating up a > > return value, but then we decided (via Agrover's patch) that it made > > more sense for target_submit_cmd() to always handle exceptions via > > normal TFO callbacks, and not have the fabric worry about the return > > here.. > > > > Also, I'm not sure if the error paths that this patch now accesses after > > target_submit_cmd() failure are going to deal with different types of > > target_submit_cmd() failures correctly. > > > > So NACK for the moment, as I don't really see why this is necessary in > > the first place..? > > Read on in the series to see why this is needed; in short, for qla2xxx > at least, we need a race-free way to check for sess_tearing_down > atomically with actually adding the command to sess_cmd_list. > <nod> > I'm OK with returning failure via callback, but > > a) I'm not sure we can use the normal TFO callbacks in case > we can't add the command to sess_cmd_list (seems like it > opens the door to other use-after-frees in qla2xxx at least) The TFO callback to release qla_tgt_cmd memory would be the same here, so it would be a internal release for target_get_sess_cmd() failures that is careful with items have not been setup in the passed fabric_tgt_cmd->se_cmd due to the session shutdown exception. As commands are received during session shutdown, dispatching their handling to tcm_qla2xxx_free_wq to do extra HW descriptor cleanup would be nice if we can get away without actually sending a proper SCSI response during session shutdown.. > b) Maybe it's OK if we say that failure to add the command to > the sess_cmd_list is the only time submit cmd fails? > I'd still like to avoid (re)propagating up a return value of target_submit_cmd() if at all possible.. > The qla2xxx race/use-after-free is definitely real, we hit it in testing > here with active IO across ACL changes. > > - R. > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
