On Tue, 2014-03-18 at 10:43 -0700, Alex Leung wrote:
> It looks to me like there's an extra se_cmd kref "put"
> (target_put_sess_cmd()) after transport_lookup_cmd_lun() fails and
> sends check condition via the fabric driver. With the extra call
> to target_put_sess_cmd(), the given se_cmd is "released" prior
> to my fabric driver completing the check condition initiated from
> the prev line. When I make the following change, the se_cmd is
> not released until the fabric driver completes the check
> condition and calls transport_generic_free_cmd() -- which is
> what I expect.
>
>
> --- a/drivers/target/target_core_transport.c 2014-02-13 14:00:14.000000000 -0800
> +++ b/drivers/target/target_core_transport.c 2014-03-17 15:50:50.235119894 -0700
> @@ -1336,7 +1336,6 @@ int target_submit_cmd_map_sgls(struct se
> rc = transport_lookup_cmd_lun(se_cmd, unpacked_lun);
> if (rc) {
> transport_send_check_condition_and_sense(se_cmd, rc, 0);
> - target_put_sess_cmd(se_sess, se_cmd);
> return 0;
> }
>
So, as far as existing code is concerned this is in fact correct.
The reason is because all fabric drivers that require an ACK (eg:
everything except tcm_loop + vhost-scsi) are passing TARGET_SCF_ACK_KREF
into this codepath. This makes the above target_put_sess_cmd() not the
last kref_put for those callers, and another target_put_sess_cmd() is
required (usually via transport_generic_free_cmd) once the fabric ACK
has been received in order to release the descriptor.
This done in order to avoid a race between the posting of a response ->
accounting of se_cmd in transport_cmd_check_stop(), and the reception of
a ACK -> transport_generic_free_cmd() -> final target_put_sess_cmd()
occurring before transport_cmd_check_stop() has finished processing.
So what your code needs to do is pass TARGET_SCF_ACK_KREF into both
target_submit_cmd() + target_submit_tmr(), and TFO->check_stop_free()
needs to call target_put_sess_cmd().
This will follow what all other HW fabric drivers are currently doing.
--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
