Re: [PATCH 4/7] target: Allow for target_submit_cmd() returning errors

[Roland Dreier <roland@xxxxxxxxxx>]

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