Instead of taking the buffer and length, update_and_write_aptpl() will
allocate the buffer as needed, and then free it. Instead, the function
takes an 'aptpl' boolean parameter.
This enables us to remove memory alloc/frees from struct
t10_pr_registration and other spots.
In emulate_pro_register, this patch also uses if/else for handling the
two values for sa_res_key, which (at least to me) seems easier to
understand.
There is a slight loss of functionality because each callsite doesn't get
its own pr_debug any more, but this info can be cleaned via ftrace if
necessary and I think the shorter code is worth it.
Signed-off-by: Andy Grover <agrover@xxxxxxxxxx>
---
drivers/target/target_core_pr.c | 239 ++++++++++---------------------------
include/target/target_core_base.h | 2 -
2 files changed, 66 insertions(+), 175 deletions(-)
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 3dd3fe5..6db3a46 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -606,13 +606,6 @@ static struct t10_pr_registration *__core_scsi3_do_alloc_registration(
return NULL;
}
- pr_reg->pr_aptpl_buf = kzalloc(PR_APTPL_BUF_LEN, GFP_ATOMIC);
- if (!pr_reg->pr_aptpl_buf) {
- pr_err("Unable to allocate pr_reg->pr_aptpl_buf\n");
- kmem_cache_free(t10_pr_reg_cache, pr_reg);
- return NULL;
- }
-
INIT_LIST_HEAD(&pr_reg->pr_reg_list);
INIT_LIST_HEAD(&pr_reg->pr_reg_abort_list);
INIT_LIST_HEAD(&pr_reg->pr_reg_aptpl_list);
@@ -803,7 +796,6 @@ int core_scsi3_alloc_aptpl_registration(
pr_err("Unable to allocate struct t10_pr_registration\n");
return -ENOMEM;
}
- pr_reg->pr_aptpl_buf = kzalloc(PR_APTPL_BUF_LEN, GFP_KERNEL);
INIT_LIST_HEAD(&pr_reg->pr_reg_list);
INIT_LIST_HEAD(&pr_reg->pr_reg_abort_list);
@@ -1272,7 +1264,6 @@ static void __core_scsi3_free_registration(
if (!preempt_and_abort_list) {
pr_reg->pr_reg_deve = NULL;
pr_reg->pr_reg_nacl = NULL;
- kfree(pr_reg->pr_aptpl_buf);
kmem_cache_free(t10_pr_reg_cache, pr_reg);
return;
}
@@ -1341,7 +1332,6 @@ void core_scsi3_free_all_registrations(
list_for_each_entry_safe(pr_reg, pr_reg_tmp, &pr_tmpl->aptpl_reg_list,
pr_reg_aptpl_list) {
list_del(&pr_reg->pr_reg_aptpl_list);
- kfree(pr_reg->pr_aptpl_buf);
kmem_cache_free(t10_pr_reg_cache, pr_reg);
}
spin_unlock(&pr_tmpl->aptpl_reg_lock);
@@ -1814,7 +1804,6 @@ out:
kmem_cache_free(t10_pr_reg_cache, pr_reg_tmp);
}
- kfree(dest_pr_reg->pr_aptpl_buf);
kmem_cache_free(t10_pr_reg_cache, dest_pr_reg);
if (dest_local_nexus)
@@ -1840,8 +1829,6 @@ static int core_scsi3_update_aptpl_buf(
int reg_count = 0;
int ret = 0;
- memset(buf, 0, pr_aptpl_buf_len);
-
spin_lock(&dev->dev_reservation_lock);
spin_lock(&dev->t10_pr.registration_lock);
/*
@@ -1965,31 +1952,45 @@ static int __core_scsi3_write_aptpl_to_file(
return ret ? -EIO : 0;
}
-static int
-core_scsi3_update_and_write_aptpl(struct se_device *dev, unsigned char *in_buf,
- u32 in_pr_aptpl_buf_len)
+/*
+ * Clear the APTPL metadata if APTPL has been disabled, otherwise
+ * write out the updated metadata to struct file for this SCSI device.
+ */
+static int core_scsi3_update_and_write_aptpl(struct se_device *dev, bool aptpl)
{
- unsigned char null_buf[64], *buf;
- int ret;
+ int ret = 0;
- /*
- * Can be called with a NULL pointer from PROUT service action CLEAR
- */
- if (!in_buf) {
- snprintf(null_buf, 64, "No Registrations or Reservations\n");
- buf = null_buf;
+ if (!aptpl) {
+ char *null_buf = "No Registrations or Reservations\n";
+
+ ret = __core_scsi3_write_aptpl_to_file(dev, null_buf);
+ dev->t10_pr.pr_aptpl_active = 0;
+ pr_debug("SPC-3 PR: Set APTPL Bit Deactivated\n");
} else {
- ret = core_scsi3_update_aptpl_buf(dev, in_buf, in_pr_aptpl_buf_len);
- if (ret != 0)
+ int ret;
+ unsigned char *buf;
+
+ buf = kzalloc(PR_APTPL_BUF_LEN, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = core_scsi3_update_aptpl_buf(dev, buf, PR_APTPL_BUF_LEN);
+ if (ret < 0) {
+ kfree(buf);
return ret;
- buf = in_buf;
+ }
+
+ ret = __core_scsi3_write_aptpl_to_file(dev, buf);
+ if (ret != 0) {
+ pr_err("SPC-3 PR: Could not update APTPL\n");
+ } else {
+ dev->t10_pr.pr_aptpl_active = 1;
+ pr_debug("SPC-3 PR: Set APTPL Bit Activated\n");
+ }
+ kfree(buf);
}
- /*
- * __core_scsi3_write_aptpl_to_file() will call strlen()
- * on the passed buf to determine pr_aptpl_buf_len.
- */
- return __core_scsi3_write_aptpl_to_file(dev, buf);
+ return ret;
}
static sense_reason_t
@@ -2003,8 +2004,6 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
struct se_portal_group *se_tpg;
struct t10_pr_registration *pr_reg, *pr_reg_p, *pr_reg_tmp, *pr_reg_e;
struct t10_reservation *pr_tmpl = &dev->t10_pr;
- /* Used for APTPL metadata w/ UNREGISTER */
- unsigned char *pr_aptpl_buf = NULL;
unsigned char isid_buf[PR_REG_ISID_LEN], *isid_ptr = NULL;
sense_reason_t ret = TCM_NO_SENSE;
int pr_holder = 0, type;
@@ -2066,31 +2065,8 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
if (ret != 0)
return ret;
}
- /*
- * Nothing left to do for the APTPL=0 case.
- */
- if (!aptpl) {
- pr_tmpl->pr_aptpl_active = 0;
- core_scsi3_update_and_write_aptpl(cmd->se_dev, NULL, 0);
- pr_debug("SPC-3 PR: Set APTPL Bit Deactivated for"
- " REGISTER\n");
- return 0;
- }
- /*
- * Locate the newly allocated local I_T Nexus *pr_reg, and
- * update the APTPL metadata information using its
- * preallocated *pr_reg->pr_aptpl_buf.
- */
- pr_reg = core_scsi3_locate_pr_reg(cmd->se_dev,
- se_sess->se_node_acl, se_sess);
- if (core_scsi3_update_and_write_aptpl(cmd->se_dev,
- pr_reg->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
- pr_tmpl->pr_aptpl_active = 1;
- pr_debug("SPC-3 PR: Set APTPL Bit Activated for REGISTER\n");
- }
-
- goto out_put_pr_reg;
+ return core_scsi3_update_and_write_aptpl(dev, aptpl);
}
/*
@@ -2131,28 +2107,30 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
}
/*
- * Allocate APTPL metadata buffer used for UNREGISTER ops
+ * sa_res_key=1 Change Reservation Key for registered I_T Nexus.
*/
- if (aptpl) {
- pr_aptpl_buf = kzalloc(PR_APTPL_BUF_LEN, GFP_KERNEL);
- if (!pr_aptpl_buf) {
- pr_err("Unable to allocate"
- " pr_aptpl_buf\n");
- ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
- goto out_put_pr_reg;
- }
- }
+ if (sa_res_key) {
+ /*
+ * Increment PRgeneration counter for struct se_device"
+ * upon a successful REGISTER, see spc4r17 section 6.3.2
+ * READ_KEYS service action.
+ */
+ pr_reg->pr_res_generation = core_scsi3_pr_generation(cmd->se_dev);
+ pr_reg->pr_res_key = sa_res_key;
+ pr_debug("SPC-3 PR [%s] REGISTER%s: Changed Reservation"
+ " Key for %s to: 0x%016Lx PRgeneration:"
+ " 0x%08x\n", cmd->se_tfo->get_fabric_name(),
+ (register_type == REGISTER_AND_IGNORE_EXISTING_KEY) ? "_AND_IGNORE_EXISTING_KEY" : "",
+ pr_reg->pr_reg_nacl->initiatorname,
+ pr_reg->pr_res_key, pr_reg->pr_res_generation);
- /*
- * sa_res_key=0 Unregister Reservation Key for registered I_T
- * Nexus sa_res_key=1 Change Reservation Key for registered I_T
- * Nexus.
- */
- if (!sa_res_key) {
+ } else {
+ /*
+ * sa_res_key=0 Unregister Reservation Key for registered I_T Nexus.
+ */
pr_holder = core_scsi3_check_implict_release(
cmd->se_dev, pr_reg);
if (pr_holder < 0) {
- kfree(pr_aptpl_buf);
ret = TCM_RESERVATION_CONFLICT;
goto out_put_pr_reg;
}
@@ -2212,57 +2190,12 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
ASCQ_2AH_RESERVATIONS_RELEASED);
}
}
- spin_unlock(&pr_tmpl->registration_lock);
-
- if (!aptpl) {
- pr_tmpl->pr_aptpl_active = 0;
- core_scsi3_update_and_write_aptpl(dev, NULL, 0);
- pr_debug("SPC-3 PR: Set APTPL Bit Deactivated"
- " for UNREGISTER\n");
- return 0;
- }
-
- if (!core_scsi3_update_and_write_aptpl(dev, pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
- pr_tmpl->pr_aptpl_active = 1;
- pr_debug("SPC-3 PR: Set APTPL Bit Activated"
- " for UNREGISTER\n");
- }
-
- goto out_free_aptpl_buf;
- }
- /*
- * Increment PRgeneration counter for struct se_device"
- * upon a successful REGISTER, see spc4r17 section 6.3.2
- * READ_KEYS service action.
- */
- pr_reg->pr_res_generation = core_scsi3_pr_generation(cmd->se_dev);
- pr_reg->pr_res_key = sa_res_key;
- pr_debug("SPC-3 PR [%s] REGISTER%s: Changed Reservation"
- " Key for %s to: 0x%016Lx PRgeneration:"
- " 0x%08x\n", cmd->se_tfo->get_fabric_name(),
- (register_type == REGISTER_AND_IGNORE_EXISTING_KEY) ? "_AND_IGNORE_EXISTING_KEY" : "",
- pr_reg->pr_reg_nacl->initiatorname,
- pr_reg->pr_res_key, pr_reg->pr_res_generation);
-
- if (!aptpl) {
- pr_tmpl->pr_aptpl_active = 0;
- core_scsi3_update_and_write_aptpl(dev, NULL, 0);
- pr_debug("SPC-3 PR: Set APTPL Bit Deactivated"
- " for REGISTER\n");
- ret = 0;
- goto out_put_pr_reg;
+ spin_unlock(&pr_tmpl->registration_lock);
}
- if (!core_scsi3_update_and_write_aptpl(dev, pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
- pr_tmpl->pr_aptpl_active = 1;
- pr_debug("SPC-3 PR: Set APTPL Bit Activated"
- " for REGISTER\n");
- }
+ ret = core_scsi3_update_and_write_aptpl(dev, aptpl);
-out_free_aptpl_buf:
- kfree(pr_aptpl_buf);
- ret = 0;
out_put_pr_reg:
core_scsi3_put_pr_reg(pr_reg);
return ret;
@@ -2437,13 +2370,8 @@ core_scsi3_pro_reserve(struct se_cmd *cmd, int type, int scope, u64 res_key)
i_buf);
spin_unlock(&dev->dev_reservation_lock);
- if (pr_tmpl->pr_aptpl_active) {
- if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
- pr_reg->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
- pr_debug("SPC-3 PR: Updated APTPL metadata"
- " for RESERVE\n");
- }
- }
+ if (pr_tmpl->pr_aptpl_active)
+ core_scsi3_update_and_write_aptpl(cmd->se_dev, true);
ret = 0;
out_put_pr_reg:
@@ -2657,12 +2585,9 @@ core_scsi3_emulate_pro_release(struct se_cmd *cmd, int type, int scope,
spin_unlock(&pr_tmpl->registration_lock);
write_aptpl:
- if (pr_tmpl->pr_aptpl_active) {
- if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
- pr_reg->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
- pr_debug("SPC-3 PR: Updated APTPL metadata for RELEASE\n");
- }
- }
+ if (pr_tmpl->pr_aptpl_active)
+ core_scsi3_update_and_write_aptpl(cmd->se_dev, true);
+
out_put_pr_reg:
core_scsi3_put_pr_reg(pr_reg);
return ret;
@@ -2746,11 +2671,7 @@ core_scsi3_emulate_pro_clear(struct se_cmd *cmd, u64 res_key)
pr_debug("SPC-3 PR [%s] Service Action: CLEAR complete\n",
cmd->se_tfo->get_fabric_name());
- if (pr_tmpl->pr_aptpl_active) {
- core_scsi3_update_and_write_aptpl(cmd->se_dev, NULL, 0);
- pr_debug("SPC-3 PR: Updated APTPL metadata"
- " for CLEAR\n");
- }
+ core_scsi3_update_and_write_aptpl(cmd->se_dev, false);
core_scsi3_pr_generation(dev);
return 0;
@@ -2822,7 +2743,6 @@ static void core_scsi3_release_preempt_and_abort(
pr_reg->pr_reg_deve = NULL;
pr_reg->pr_reg_nacl = NULL;
- kfree(pr_reg->pr_aptpl_buf);
kmem_cache_free(t10_pr_reg_cache, pr_reg);
}
}
@@ -2984,14 +2904,8 @@ core_scsi3_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_key,
}
spin_unlock(&dev->dev_reservation_lock);
- if (pr_tmpl->pr_aptpl_active) {
- if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
- pr_reg_n->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
- pr_debug("SPC-3 PR: Updated APTPL"
- " metadata for PREEMPT%s\n", (preempt_type == PREEMPT_AND_ABORT) ?
- "_AND_ABORT" : "");
- }
- }
+ if (pr_tmpl->pr_aptpl_active)
+ core_scsi3_update_and_write_aptpl(cmd->se_dev, true);
core_scsi3_put_pr_reg(pr_reg_n);
core_scsi3_pr_generation(cmd->se_dev);
@@ -3119,13 +3033,8 @@ core_scsi3_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_key,
pr_reg_n);
}
- if (pr_tmpl->pr_aptpl_active) {
- if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
- pr_reg_n->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
- pr_debug("SPC-3 PR: Updated APTPL metadata for PREEMPT"
- "%s\n", (preempt_type == PREEMPT_AND_ABORT) ? "_AND_ABORT" : "");
- }
- }
+ if (pr_tmpl->pr_aptpl_active)
+ core_scsi3_update_and_write_aptpl(cmd->se_dev, true);
core_scsi3_put_pr_reg(pr_reg_n);
core_scsi3_pr_generation(cmd->se_dev);
@@ -3552,23 +3461,7 @@ after_iport_check:
} else
core_scsi3_put_pr_reg(pr_reg);
- /*
- * Clear the APTPL metadata if APTPL has been disabled, otherwise
- * write out the updated metadata to struct file for this SCSI device.
- */
- if (!aptpl) {
- pr_tmpl->pr_aptpl_active = 0;
- core_scsi3_update_and_write_aptpl(cmd->se_dev, NULL, 0);
- pr_debug("SPC-3 PR: Set APTPL Bit Deactivated for"
- " REGISTER_AND_MOVE\n");
- } else {
- pr_tmpl->pr_aptpl_active = 1;
- if (!core_scsi3_update_and_write_aptpl(cmd->se_dev,
- dest_pr_reg->pr_aptpl_buf, PR_APTPL_BUF_LEN)) {
- pr_debug("SPC-3 PR: Set APTPL Bit Activated for"
- " REGISTER_AND_MOVE\n");
- }
- }
+ core_scsi3_update_and_write_aptpl(cmd->se_dev, aptpl);
transport_kunmap_data_sg(cmd);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index dca2e3c..3314c4b 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -339,8 +339,6 @@ struct t10_pr_registration {
/* Used during APTPL metadata reading */
#define PR_APTPL_MAX_TPORT_LEN 256
unsigned char pr_tport[PR_APTPL_MAX_TPORT_LEN];
- /* For writing out live meta data */
- unsigned char *pr_aptpl_buf;
u16 pr_aptpl_rpti;
u16 pr_reg_tpgt;
/* Reservation effects all target ports */
--
1.7.1
--
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
