[Devel] [PATCH RHEL7 COMMIT] target: fix a locking scheme of persistent reservations

[Konstantin Khorenko]

The commit is pushed to "branch-rh7-3.10.0-862.3.2.vz7.61.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-862.3.2.vz7.61.3
------>
commit 775c7ce845ffd920183d75e2d268cc155becb2d7
Author: Andrei Vagin <avagin at openvz.org>
Date:   Thu Jun 14 09:35:26 2018 +0300

    target: fix a locking scheme of persistent reservations
    
    In this code, the next pattern is used:
    
    spin_lock(&pr_tmpl->registration_lock);
    list_for_each_entry_safe(pr_reg, pr_reg_tmp,
                    &pr_tmpl->registration_list, pr_reg_list) {
            ...
            atomic_inc_mb(&pr_reg->pr_res_holders);
            spin_unlock(&pr_tmpl->registration_lock);
            ...
            spin_lock(&pr_tmpl->registration_lock);
            atomic_dec_mb(&pr_reg->pr_res_holders);
            ...
    }
    spin_unlock(&pr_tmpl->registration_lock);
    
    It is wrong, because we get a reference for pr_reg and don't care about
    pr_reg_tmp, but it can be removed from the list. This unlock/lock in
    a middle of a loop is a very strange optimization, because there is no
    any heavy operations. This patch removes this temporary release of a
    spin lock.
    
    https://pmc.acronis.com/browse/VSTOR-10675
    
    Signed-off-by: Andrei Vagin <avagin at openvz.org>
---
 drivers/target/target_core_pr.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index d6e1c1a2db77..90ed1a9a407e 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -730,7 +730,6 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
 	list_for_each_entry_safe(lun_tmp, next, &dev->dev_sep_list, lun_dev_link) {
 		if (!percpu_ref_tryget_live(&lun_tmp->lun_ref))
 			continue;
-		spin_unlock(&dev->se_port_lock);
 
 		spin_lock(&lun_tmp->lun_deve_lock);
 		list_for_each_entry(deve_tmp, &lun_tmp->lun_deve_list, lun_link) {
@@ -806,7 +805,6 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
 		}
 		spin_unlock(&lun_tmp->lun_deve_lock);
 
-		spin_lock(&dev->se_port_lock);
 		percpu_ref_put(&lun_tmp->lun_ref);
 	}
 	spin_unlock(&dev->se_port_lock);
@@ -887,7 +885,9 @@ int core_scsi3_alloc_aptpl_registration(
 	 */
 	pr_reg->pr_res_holder = res_holder;
 
+	spin_lock(&pr_tmpl->aptpl_reg_lock);
 	list_add_tail(&pr_reg->pr_reg_aptpl_list, &pr_tmpl->aptpl_reg_list);
+	spin_unlock(&pr_tmpl->aptpl_reg_lock);
 	pr_debug("SPC-3 PR APTPL Successfully added registration%s from"
 			" metadata\n", (res_holder) ? "+reservation" : "");
 	return 0;
@@ -977,7 +977,6 @@ static int __core_scsi3_check_aptpl_registration(
 			pr_reg->pr_reg_nacl = nacl;
 			pr_reg->tg_pt_sep_rtpi = lun->lun_rtpi;
 			list_del(&pr_reg->pr_reg_aptpl_list);
-			spin_unlock(&pr_tmpl->aptpl_reg_lock);
 			/*
 			 * At this point all of the pointers in *pr_reg will
 			 * be setup, so go ahead and add the registration.
@@ -994,7 +993,6 @@ static int __core_scsi3_check_aptpl_registration(
 			 * Reenable pr_aptpl_active to accept new metadata
 			 * updates once the SCSI device is active again..
 			 */
-			spin_lock(&pr_tmpl->aptpl_reg_lock);
 			pr_tmpl->pr_aptpl_active = 1;
 		}
 	}
@@ -1297,7 +1295,6 @@ static void __core_scsi3_free_registration(
 {
 	const struct target_core_fabric_ops *tfo =
 			pr_reg->pr_reg_nacl->se_tpg->se_tpg_tfo;
-	struct t10_reservation *pr_tmpl = &dev->t10_pr;
 	struct se_node_acl *nacl = pr_reg->pr_reg_nacl;
 	struct se_dev_entry *deve;
 	char i_buf[PR_REG_ISID_ID_LEN];
@@ -1314,7 +1311,6 @@ static void __core_scsi3_free_registration(
 	if (dec_holders)
 		core_scsi3_put_pr_reg(pr_reg);
 
-	spin_unlock(&pr_tmpl->registration_lock);
 	/*
 	 * Wait until all reference from any other I_T nexuses for this
 	 * *pr_reg have been released.  Because list_del() is called above,
@@ -1333,7 +1329,6 @@ static void __core_scsi3_free_registration(
 		clear_bit(DEF_PR_REG_ACTIVE, &deve->deve_flags);
 	rcu_read_unlock();
 
-	spin_lock(&pr_tmpl->registration_lock);
 	pr_debug("SPC-3 PR [%s] Service Action: UNREGISTER Initiator"
 		" Node: %s%s\n", tfo->get_fabric_name(),
 		pr_reg->pr_reg_nacl->initiatorname,
@@ -3972,7 +3967,6 @@ core_scsi3_pri_read_full_status(struct se_cmd *cmd)
 		add_desc_len = 0;
 
 		atomic_inc_mb(&pr_reg->pr_res_holders);
-		spin_unlock(&pr_tmpl->registration_lock);
 		/*
 		 * Determine expected length of $FABRIC_MOD specific
 		 * TransportID full status descriptor..
@@ -3983,7 +3977,6 @@ core_scsi3_pri_read_full_status(struct se_cmd *cmd)
 		    exp_desc_len + add_len > cmd->data_length) {
 			pr_warn("SPC-3 PRIN READ_FULL_STATUS ran"
 				" out of buffer: %d\n", cmd->data_length);
-			spin_lock(&pr_tmpl->registration_lock);
 			atomic_dec_mb(&pr_reg->pr_res_holders);
 			break;
 		}
@@ -4052,7 +4045,6 @@ core_scsi3_pri_read_full_status(struct se_cmd *cmd)
 		desc_len = target_get_pr_transport_id(se_nacl, pr_reg,
 				&format_code, &buf[off+4]);
 
-		spin_lock(&pr_tmpl->registration_lock);
 		atomic_dec_mb(&pr_reg->pr_res_holders);
 
 		if (desc_len < 0)