[PATCH 0/9] target: Optimizations + cleanups for v3.11

12 views
Skip to first unread message

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:15 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

Hi folks,

This series contains a number of target optimizations added during
recent prototype scsi-mq performance profiling in order to avoid
unnecessary se_cmd->t_state_lock acquire/release in fast path I/O
submission and completion code.

All of the target optimizations are independent of driver code, so
all fabrics should benefit from these improvements.

Also included are two vhost/scsi specific patches, one in the same
vein to avoid unnecessary se_cmd->t_state_lock access, and the second
to convert vhost/scsi to proper cmd_kref TARGET_SCF_ACK_KREF usage.

Please have a look.

Thanks,

--nab

Nicholas Bellinger (9):
target: Add transport_cmd_check_stop write_pending bit
target: Drop unnecessary CMD_T_DEV_ACTIVE check from
transport_lun_remove_cmd
target: Remove legacy t_fe_count + avoid t_state_lock access in
transport_put_cmd
target: Avoid extra t_state_lock access in __target_execute_cmd
target: Drop unnecessary t_state_lock access for
SCF_SUPPORTED_SAM_OPCODE assignment
iscsi-target: Avoid unnecessary t_state_lock during unsolicited
data-out check
target: Drop legacy se_cmd->check_release bit
vhost/scsi: Drop unnecessary wait_for_tasks=true usage with
transport_generic_free_cmd
vhost/scsi: Convert to se_cmd->cmd_kref TARGET_SCF_ACK_KREF usage

drivers/target/iscsi/iscsi_target.c | 6 ---
drivers/target/target_core_tmr.c | 12 +----
drivers/target/target_core_transport.c | 72 ++++++++-----------------------
drivers/vhost/scsi.c | 33 +++++++++------
include/target/target_core_base.h | 3 -
5 files changed, 41 insertions(+), 85 deletions(-)

--
1.7.2.5

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:16 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch adds a new transport_cmd_check_stop() parameter for signaling
when TRANSPORT_WRITE_PENDING needs to be set.

This allows transport_generic_new_cmd() to avoid the extra lock acquire/release
of ->t_state_lock in the fast path for DMA_TO_DEVICE operations ahead of
transport_cmd_check_stop() + se_tfo->write_pending().

Cc: Christoph Hellwig <h...@lst.de>
Cc: Roland Dreier <rol...@kernel.org>
Cc: Kent Overstreet <kover...@google.com>
Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Moussa Ba <mous...@micron.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_transport.c | 19 +++++++++----------
1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 4a79336..15afa83 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -445,11 +445,15 @@ static void target_remove_from_state_list(struct se_cmd *cmd)
spin_unlock_irqrestore(&dev->execute_task_lock, flags);
}

-static int transport_cmd_check_stop(struct se_cmd *cmd, bool remove_from_lists)
+static int transport_cmd_check_stop(struct se_cmd *cmd, bool remove_from_lists,
+ bool write_pending)
{
unsigned long flags;

spin_lock_irqsave(&cmd->t_state_lock, flags);
+ if (write_pending)
+ cmd->t_state = TRANSPORT_WRITE_PENDING;
+
/*
* Determine if IOCTL context caller in requesting the stopping of this
* command for LUN shutdown purposes.
@@ -514,7 +518,7 @@ static int transport_cmd_check_stop(struct se_cmd *cmd, bool remove_from_lists)

static int transport_cmd_check_stop_to_fabric(struct se_cmd *cmd)
{
- return transport_cmd_check_stop(cmd, true);
+ return transport_cmd_check_stop(cmd, true, false);
}

static void transport_lun_remove_cmd(struct se_cmd *cmd)
@@ -2116,12 +2120,7 @@ transport_generic_new_cmd(struct se_cmd *cmd)
target_execute_cmd(cmd);
return 0;
}
-
- spin_lock_irq(&cmd->t_state_lock);
- cmd->t_state = TRANSPORT_WRITE_PENDING;
- spin_unlock_irq(&cmd->t_state_lock);
-
- transport_cmd_check_stop(cmd, false);
+ transport_cmd_check_stop(cmd, false, true);

ret = cmd->se_tfo->write_pending(cmd);
if (ret == -EAGAIN || ret == -ENOMEM)
@@ -2325,7 +2324,7 @@ static int transport_lun_wait_for_tasks(struct se_cmd *cmd, struct se_lun *lun)
pr_debug("ConfigFS ITT[0x%08x] - CMD_T_STOP, skipping\n",
cmd->se_tfo->get_task_tag(cmd));
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
- transport_cmd_check_stop(cmd, false);
+ transport_cmd_check_stop(cmd, false, false);
return -EPERM;
}
cmd->transport_state |= CMD_T_LUN_FE_STOP;
@@ -2433,7 +2432,7 @@ check_cond:

spin_unlock_irqrestore(&cmd->t_state_lock,
cmd_flags);
- transport_cmd_check_stop(cmd, false);
+ transport_cmd_check_stop(cmd, false, false);
complete(&cmd->transport_lun_fe_stop_comp);
spin_lock_irqsave(&lun->lun_cmd_lock, lun_flags);
continue;
--
1.7.2.5

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:17 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch drops an unnecessary acquire/release of se_cmd->t_state_lock within
transport_lun_remove_cmd() when checking CMD_T_DEV_ACTIVE for invoking
target_remove_from_state_list().

For all fast path completion cases, transport_lun_remove_cmd() is always
called ahead of transport_cmd_check_stop(), and since transport_cmd_check_stop()
is calling target_remove_from_state_list() when remove_from_lists=true,
the t_state_lock usage in transport_lun_remove_cmd() can safely be removed.

Cc: Christoph Hellwig <h...@lst.de>
Cc: Roland Dreier <rol...@kernel.org>
Cc: Kent Overstreet <kover...@google.com>
Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Moussa Ba <mous...@micron.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_transport.c | 7 -------
1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 15afa83..d062878 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -529,13 +529,6 @@ static void transport_lun_remove_cmd(struct se_cmd *cmd)
if (!lun)
return;

- spin_lock_irqsave(&cmd->t_state_lock, flags);
- if (cmd->transport_state & CMD_T_DEV_ACTIVE) {
- cmd->transport_state &= ~CMD_T_DEV_ACTIVE;
- target_remove_from_state_list(cmd);
- }
- spin_unlock_irqrestore(&cmd->t_state_lock, flags);
-
spin_lock_irqsave(&lun->lun_cmd_lock, flags);
if (!list_empty(&cmd->se_lun_node))
list_del_init(&cmd->se_lun_node);
--
1.7.2.5

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:18 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch removes legacy se_cmd->t_fe_count usage in order to avoid
se_cmd->t_state_lock access within transport_put_cmd() during normal
fast path se_cmd descriptor release.

Also drop the left-over parameter usage within core_tmr_handle_tas_abort()

Cc: Christoph Hellwig <h...@lst.de>
Cc: Roland Dreier <rol...@kernel.org>
Cc: Kent Overstreet <kover...@google.com>
Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Moussa Ba <mous...@micron.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_tmr.c | 12 ++----------
drivers/target/target_core_transport.c | 19 -------------------
include/target/target_core_base.h | 1 -
3 files changed, 2 insertions(+), 30 deletions(-)

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index d0b4dd9..0d7cacb 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -85,13 +85,8 @@ void core_tmr_release_req(
static void core_tmr_handle_tas_abort(
struct se_node_acl *tmr_nacl,
struct se_cmd *cmd,
- int tas,
- int fe_count)
+ int tas)
{
- if (!fe_count) {
- transport_cmd_finish_abort(cmd, 1);
- return;
- }
/*
* TASK ABORTED status (TAS) bit support
*/
@@ -253,7 +248,6 @@ static void core_tmr_drain_state_list(
LIST_HEAD(drain_task_list);
struct se_cmd *cmd, *next;
unsigned long flags;
- int fe_count;

/*
* Complete outstanding commands with TASK_ABORTED SAM status.
@@ -329,12 +323,10 @@ static void core_tmr_drain_state_list(
spin_lock_irqsave(&cmd->t_state_lock, flags);
target_stop_cmd(cmd, &flags);

- fe_count = atomic_read(&cmd->t_fe_count);
-
cmd->transport_state |= CMD_T_ABORTED;
spin_unlock_irqrestore(&cmd->t_state_lock, flags);

- core_tmr_handle_tas_abort(tmr_nacl, cmd, tas, fe_count);
+ core_tmr_handle_tas_abort(tmr_nacl, cmd, tas);
}
}

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index d062878..daa7aa8 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1967,24 +1967,8 @@ static void transport_release_cmd(struct se_cmd *cmd)
*/
static void transport_put_cmd(struct se_cmd *cmd)
{
- unsigned long flags;
-
- spin_lock_irqsave(&cmd->t_state_lock, flags);
- if (atomic_read(&cmd->t_fe_count) &&
- !atomic_dec_and_test(&cmd->t_fe_count)) {
- spin_unlock_irqrestore(&cmd->t_state_lock, flags);
- return;
- }
-
- if (cmd->transport_state & CMD_T_DEV_ACTIVE) {
- cmd->transport_state &= ~CMD_T_DEV_ACTIVE;
- target_remove_from_state_list(cmd);
- }
- spin_unlock_irqrestore(&cmd->t_state_lock, flags);
-
transport_free_pages(cmd);
transport_release_cmd(cmd);
- return;
}

void *transport_kmap_data_sg(struct se_cmd *cmd)
@@ -2100,9 +2084,6 @@ transport_generic_new_cmd(struct se_cmd *cmd)
if (ret < 0)
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
}
-
- atomic_inc(&cmd->t_fe_count);
-
/*
* If this command is not a write we can execute it right here,
* for write buffers we need to notify the fabric driver first
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index e773dfa..3b337b9 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -458,7 +458,6 @@ struct se_cmd {
unsigned char *t_task_cdb;
unsigned char __t_task_cdb[TCM_MAX_COMMAND_SIZE];
unsigned long long t_task_lba;
- atomic_t t_fe_count;
unsigned int transport_state;
#define CMD_T_ABORTED (1 << 0)
#define CMD_T_ACTIVE (1 << 1)
--
1.7.2.5

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:19 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch makes target_execute_cmd() set CMD_T_BUSY|CMD_T_SENT while
holding se_cmd->t_state_lock, in order to avoid the extra aquire/release
in __target_execute_cmd().

It also clears these bits in case of a target_handle_task_attr()
failure.

Cc: Christoph Hellwig <h...@lst.de>
Cc: Roland Dreier <rol...@kernel.org>
Cc: Kent Overstreet <kover...@google.com>
Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Moussa Ba <mous...@micron.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_transport.c | 16 +++++++++-------
1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index daa7aa8..f9e69f5 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1579,10 +1579,6 @@ static void __target_execute_cmd(struct se_cmd *cmd)
{
sense_reason_t ret;

- spin_lock_irq(&cmd->t_state_lock);
- cmd->transport_state |= (CMD_T_BUSY|CMD_T_SENT);
- spin_unlock_irq(&cmd->t_state_lock);
-
if (cmd->execute_cmd) {
ret = cmd->execute_cmd(cmd);
if (ret) {
@@ -1689,11 +1685,17 @@ void target_execute_cmd(struct se_cmd *cmd)
}

cmd->t_state = TRANSPORT_PROCESSING;
- cmd->transport_state |= CMD_T_ACTIVE;
+ cmd->transport_state |= CMD_T_ACTIVE|CMD_T_BUSY|CMD_T_SENT;
spin_unlock_irq(&cmd->t_state_lock);

- if (!target_handle_task_attr(cmd))
- __target_execute_cmd(cmd);
+ if (target_handle_task_attr(cmd)) {
+ spin_lock_irq(&cmd->t_state_lock);
+ cmd->transport_state &= ~CMD_T_BUSY|CMD_T_SENT;
+ spin_unlock_irq(&cmd->t_state_lock);
+ return;
+ }
+
+ __target_execute_cmd(cmd);
}
EXPORT_SYMBOL(target_execute_cmd);

--
1.7.2.5

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:20 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch drops the se_cmd->t_state_lock access around SCF_SUPPORTED_SAM_OPCODE
assignment within target_setup_cmd_from_cdb().

Original v4.0 target code required this as fabrics would be checking for
this values in different process contexts for setup and I/O submission.

Given that modern v4.1 target code performs setup and I/O submission
from the same process context, this t_state_lock access is no longer
required.

Cc: Christoph Hellwig <h...@lst.de>
Cc: Roland Dreier <rol...@kernel.org>
Cc: Kent Overstreet <kover...@google.com>
Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Moussa Ba <mous...@micron.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_transport.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index f9e69f5..77c0b29 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1088,7 +1088,6 @@ sense_reason_t
target_setup_cmd_from_cdb(struct se_cmd *cmd, unsigned char *cdb)
{
struct se_device *dev = cmd->se_dev;
- unsigned long flags;
sense_reason_t ret;

/*
@@ -1148,9 +1147,7 @@ target_setup_cmd_from_cdb(struct se_cmd *cmd, unsigned char *cdb)
if (ret)
return ret;

- spin_lock_irqsave(&cmd->t_state_lock, flags);
cmd->se_cmd_flags |= SCF_SUPPORTED_SAM_OPCODE;
- spin_unlock_irqrestore(&cmd->t_state_lock, flags);

spin_lock(&cmd->se_lun->lun_sep_lock);
if (cmd->se_lun->lun_sep)
--
1.7.2.5

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:21 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

In modern iscsi-target code, the setup and I/O submission is done within a
single process context, so there is no need to acquire se_cmd->t_state_lock while
checking SCF_SUPPORTED_SAM_OPCODE for determining when unsolicited data-out
should be dumped.

Cc: Christoph Hellwig <h...@lst.de>
Cc: Roland Dreier <rol...@kernel.org>
Cc: Kent Overstreet <kover...@google.com>
Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Moussa Ba <mous...@micron.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/iscsi/iscsi_target.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 262ef1f..24783ee 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1277,7 +1277,6 @@ iscsit_check_dataout_hdr(struct iscsi_conn *conn, unsigned char *buf,
struct iscsi_data *hdr = (struct iscsi_data *)buf;
struct iscsi_cmd *cmd = NULL;
struct se_cmd *se_cmd;
- unsigned long flags;
u32 payload_length = ntoh24(hdr->dlength);
int rc;

@@ -1356,14 +1355,9 @@ iscsit_check_dataout_hdr(struct iscsi_conn *conn, unsigned char *buf,
*/

/* Something's amiss if we're not in WRITE_PENDING state... */
- spin_lock_irqsave(&se_cmd->t_state_lock, flags);
WARN_ON(se_cmd->t_state != TRANSPORT_WRITE_PENDING);
- spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);
-
- spin_lock_irqsave(&se_cmd->t_state_lock, flags);
if (!(se_cmd->se_cmd_flags & SCF_SUPPORTED_SAM_OPCODE))
dump_unsolicited_data = 1;
- spin_unlock_irqrestore(&se_cmd->t_state_lock, flags);

if (dump_unsolicited_data) {
/*
--
1.7.2.5

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:22 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

Now with iscsi-target using modern se_cmd->cmd_kref accounting in
v3.10 code, it's safe to go ahead and drop the legacy release
codepath + se_cmd->check_release bit in transport_release_cmd()

Cc: Christoph Hellwig <h...@lst.de>
Cc: Roland Dreier <rol...@kernel.org>
Cc: Kent Overstreet <kover...@google.com>
Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Moussa Ba <mous...@micron.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_transport.c | 8 +-------
include/target/target_core_base.h | 2 --
2 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 77c0b29..abb7e40 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1951,11 +1951,7 @@ static void transport_release_cmd(struct se_cmd *cmd)
* If this cmd has been setup with target_get_sess_cmd(), drop
* the kref and call ->release_cmd() in kref callback.
*/
- if (cmd->check_release != 0) {
- target_put_sess_cmd(cmd->se_sess, cmd);
- return;
- }
- cmd->se_tfo->release_cmd(cmd);
+ target_put_sess_cmd(cmd->se_sess, cmd);
}

/**
@@ -2171,8 +2167,6 @@ int target_get_sess_cmd(struct se_session *se_sess, struct se_cmd *se_cmd,
goto out;
}
list_add_tail(&se_cmd->se_cmd_list, &se_sess->sess_cmd_list);
- se_cmd->check_release = 1;
-
out:
spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
return ret;
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 3b337b9..a5c97db 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -424,8 +424,6 @@ struct se_cmd {
int sam_task_attr;
/* Transport protocol dependent state, see transport_state_table */
enum transport_state_table t_state;
- /* Used to signal cmd->se_tfo->check_release_cmd() usage per cmd */
- unsigned check_release:1;
unsigned cmd_wait_set:1;
unsigned unknown_data_length:1;
/* See se_cmd_flags_table */
--
1.7.2.5

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:23 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch changes vhost_scsi_free_cmd() to call transport_generic_free_cmd()
with wait_for_tasks=false in order to avoid the extra se_cmd->t_state_lock
access for the wait_for_tasks=true case.

This is unnecessary because vhost_scsi_free_cmd() is only ever called by
vhost_scsi_complete_cmd_work() after TCM completion handoff, and by
vhost_scsi_handle_vq() exception code before TCM submission handoff, so
there is never a case where se_cmd is still active from TCM's perspective
when transport_generic_free_cmd() is called.

Cc: Christoph Hellwig <h...@lst.de>
Cc: Roland Dreier <rol...@kernel.org>
Cc: Kent Overstreet <kover...@google.com>
Cc: Asias He <as...@redhat.com>
Cc: Michael S. Tsirkin <m...@redhat.com>
Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Moussa Ba <mous...@micron.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/vhost/scsi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 7014202..aacf71e 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -557,7 +557,7 @@ static void vhost_scsi_free_cmd(struct tcm_vhost_cmd *tv_cmd)
struct se_cmd *se_cmd = &tv_cmd->tvc_se_cmd;

/* TODO locking against target/backend threads? */
- transport_generic_free_cmd(se_cmd, 1);
+ transport_generic_free_cmd(se_cmd, 0);

if (tv_cmd->tvc_sgl_count) {
u32 i;
--
1.7.2.5

Nicholas A. Bellinger

unread,
Jun 7, 2013, 5:34:24 PM6/7/13
to target-devel, linux-scsi, Christoph Hellwig, Roland Dreier, Kent Overstreet, Asias He, Michael S. Tsirkin, Or Gerlitz, Moussa Ba, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

This patch coverts vhost/scsi to se_cmd->cmd_kref TARGET_SCF_ACK_KREF
usage, instead of assuming that vhost_scsi_free_cmd() is always called
before TCM processing is completed in the response fast path.

This includes adding vhost_scsi_check_stop_free() -> target_put_sess_cmd()
to perform the second se_cmd->cmd_kref put, and moving vhost_scsi_free_cmd()
resource release into tcm_vhost_release_cmd() that is invoked once the last
se_cmd->cmd_kref put occurs.

Cc: Christoph Hellwig <h...@lst.de>
Cc: Roland Dreier <rol...@kernel.org>
Cc: Kent Overstreet <kover...@google.com>
Cc: Asias He <as...@redhat.com>
Cc: Michael S. Tsirkin <m...@redhat.com>
Cc: Or Gerlitz <oger...@mellanox.com>
Cc: Moussa Ba <mous...@micron.com>
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/vhost/scsi.c | 31 +++++++++++++++++++------------
1 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index aacf71e..1e5e820 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -446,7 +446,19 @@ static u32 tcm_vhost_tpg_get_inst_index(struct se_portal_group *se_tpg)

static void tcm_vhost_release_cmd(struct se_cmd *se_cmd)
{
- return;
+ struct tcm_vhost_cmd *tv_cmd = container_of(se_cmd,
+ struct tcm_vhost_cmd, tvc_se_cmd);
+
+ if (tv_cmd->tvc_sgl_count) {
+ u32 i;
+ for (i = 0; i < tv_cmd->tvc_sgl_count; i++)
+ put_page(sg_page(&tv_cmd->tvc_sgl[i]));
+
+ kfree(tv_cmd->tvc_sgl);
+ }
+
+ tcm_vhost_put_inflight(tv_cmd->inflight);
+ kfree(tv_cmd);
}

static int tcm_vhost_shutdown_session(struct se_session *se_sess)
@@ -559,17 +571,11 @@ static void vhost_scsi_free_cmd(struct tcm_vhost_cmd *tv_cmd)
/* TODO locking against target/backend threads? */
transport_generic_free_cmd(se_cmd, 0);

- if (tv_cmd->tvc_sgl_count) {
- u32 i;
- for (i = 0; i < tv_cmd->tvc_sgl_count; i++)
- put_page(sg_page(&tv_cmd->tvc_sgl[i]));
-
- kfree(tv_cmd->tvc_sgl);
- }
-
- tcm_vhost_put_inflight(tv_cmd->inflight);
+}

- kfree(tv_cmd);
+static int vhost_scsi_check_stop_free(struct se_cmd *se_cmd)
+{
+ return target_put_sess_cmd(se_cmd->se_sess, se_cmd);
}

static void tcm_vhost_do_evt_work(struct vhost_scsi *vs,
@@ -847,7 +853,7 @@ static void tcm_vhost_submission_work(struct work_struct *work)
tv_cmd->tvc_cdb, &tv_cmd->tvc_sense_buf[0],
tv_cmd->tvc_lun, tv_cmd->tvc_exp_data_len,
tv_cmd->tvc_task_attr, tv_cmd->tvc_data_direction,
- 0, sg_ptr, tv_cmd->tvc_sgl_count,
+ TARGET_SCF_ACK_KREF, sg_ptr, tv_cmd->tvc_sgl_count,
sg_bidi_ptr, sg_no_bidi);
if (rc < 0) {
transport_send_check_condition_and_sense(se_cmd,
@@ -2008,6 +2014,7 @@ static struct target_core_fabric_ops tcm_vhost_ops = {
.tpg_release_fabric_acl = tcm_vhost_release_fabric_acl,
.tpg_get_inst_index = tcm_vhost_tpg_get_inst_index,
.release_cmd = tcm_vhost_release_cmd,
+ .check_stop_free = vhost_scsi_check_stop_free,
.shutdown_session = tcm_vhost_shutdown_session,
.close_session = tcm_vhost_close_session,
.sess_get_index = tcm_vhost_sess_get_index,
--
1.7.2.5

Reply all
Reply to author
Forward
0 new messages