| Message ID | 20170330171244.8346-7-bart.vanassche@sandisk.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2017-03-30 at 10:12 -0700, Bart Van Assche wrote: > The approach in the iSCSI target driver with regard to command > reference counting is as follows: > - When a command or TMF is submitted to the LIO core, > target_get_sess_cmd() is called with the second argument set > to true such that cmd_kref is initialized to two. > - During regular execution of a command target_put_sess_cmd() is > called once. However, if a connection is shut down it is not > sure that target_put_sess_cmd() has already been called. > - If the 'shutdown' argument of iscsit_free_cmd() is true, this > function is responsible for calling target_put_sess_cmd() if > that has not yet happened before that function was called. > > Since it is not possible to determine whether or not > target_put_sess_cmd() has already been called by examining the > CMD_T_ABORTED flag and cmd_kref only (e.g. if a LUN RESET is in > progress concurrently with iscsit_free_cmd()), use the refcount > that was introduced by the previous patch to figure out whether > or not target_put_sess_cmd() has to be called from inside > iscsit_free_cmd(). > > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> > --- > drivers/target/iscsi/iscsi_target_util.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c > index 1cacfe1003e3..04c7356e4402 100644 > --- a/drivers/target/iscsi/iscsi_target_util.c > +++ b/drivers/target/iscsi/iscsi_target_util.c > @@ -770,12 +770,11 @@ void __iscsit_free_cmd(struct iscsi_cmd *cmd, bool check_queues) > void iscsit_free_cmd(struct iscsi_cmd *cmd, bool shutdown) > { > struct se_cmd *se_cmd = cmd->se_cmd.se_tfo ? &cmd->se_cmd : NULL; > - int rc; > > __iscsit_free_cmd(cmd, shutdown); > if (se_cmd) { > - rc = transport_generic_free_cmd(&cmd->se_cmd, shutdown); > - if (!rc && shutdown && se_cmd && se_cmd->se_sess) { > + transport_generic_free_cmd(&cmd->se_cmd, shutdown); > + if (shutdown && atomic_read(&cmd->refcnt)) { > __iscsit_free_cmd(cmd, shutdown); > iscsit_put_cmd(cmd); > } Also based on incorrect assumptions about transport_generic_free_cmd(). There is no reason to add a new driver specific reference count for handling this. The correct bug-fix for this is: iscsi-target: Fix TMR reference leak during session shutdown https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git/commit/?id=efb2ea770bb3b0f40007530bc8b0c22f36e1c5eb -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 1cacfe1003e3..04c7356e4402 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -770,12 +770,11 @@ void __iscsit_free_cmd(struct iscsi_cmd *cmd, bool check_queues) void iscsit_free_cmd(struct iscsi_cmd *cmd, bool shutdown) { struct se_cmd *se_cmd = cmd->se_cmd.se_tfo ? &cmd->se_cmd : NULL; - int rc; __iscsit_free_cmd(cmd, shutdown); if (se_cmd) { - rc = transport_generic_free_cmd(&cmd->se_cmd, shutdown); - if (!rc && shutdown && se_cmd && se_cmd->se_sess) { + transport_generic_free_cmd(&cmd->se_cmd, shutdown); + if (shutdown && atomic_read(&cmd->refcnt)) { __iscsit_free_cmd(cmd, shutdown); iscsit_put_cmd(cmd); }
The approach in the iSCSI target driver with regard to command reference counting is as follows: - When a command or TMF is submitted to the LIO core, target_get_sess_cmd() is called with the second argument set to true such that cmd_kref is initialized to two. - During regular execution of a command target_put_sess_cmd() is called once. However, if a connection is shut down it is not sure that target_put_sess_cmd() has already been called. - If the 'shutdown' argument of iscsit_free_cmd() is true, this function is responsible for calling target_put_sess_cmd() if that has not yet happened before that function was called. Since it is not possible to determine whether or not target_put_sess_cmd() has already been called by examining the CMD_T_ABORTED flag and cmd_kref only (e.g. if a LUN RESET is in progress concurrently with iscsit_free_cmd()), use the refcount that was introduced by the previous patch to figure out whether or not target_put_sess_cmd() has to be called from inside iscsit_free_cmd(). Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> --- drivers/target/iscsi/iscsi_target_util.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)