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