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