All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>, Nic Bellinger <nab@daterainc.com>,
	target-devel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: [PATCH 4/6] target: Send UA on ALUA target port group change
Date: Tue, 23 Jun 2015 00:54:24 -0700	[thread overview]
Message-ID: <1435046064.7460.23.camel@haakon3.risingtidesystems.com> (raw)
In-Reply-To: <20150619130519.GA7783@lst.de>

On Fri, 2015-06-19 at 15:05 +0200, Christoph Hellwig wrote:
> > --- a/drivers/target/target_core_alua.c
> > +++ b/drivers/target/target_core_alua.c
> > @@ -1880,12 +1880,19 @@ static void core_alua_put_tg_pt_gp_from_name(
> >  static void __target_attach_tg_pt_gp(struct se_lun *lun,
> >  		struct t10_alua_tg_pt_gp *tg_pt_gp)
> >  {
> > +	struct se_dev_entry *se_deve;
> > +
> >  	assert_spin_locked(&lun->lun_tg_pt_gp_lock);
> >  
> >  	spin_lock(&tg_pt_gp->tg_pt_gp_lock);
> >  	lun->lun_tg_pt_gp = tg_pt_gp;
> >  	list_add_tail(&lun->lun_tg_pt_gp_link, &tg_pt_gp->tg_pt_gp_lun_list);
> >  	tg_pt_gp->tg_pt_gp_members++;
> > +	spin_lock_bh(&lun->lun_deve_lock);
> > +	list_for_each_entry(se_deve, &lun->lun_deve_list, lun_link)
> > +		core_scsi3_ua_allocate(se_deve, 0x3f,
> > +				       ASCQ_3FH_INQUIRY_DATA_HAS_CHANGED);
> > +	spin_unlock_bh(&lun->lun_deve_lock);
> >  	spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
> 
> Taking a _bh lock inside a regular spinlock is completely broken.
> 

...

> Fortunately I don't think lun_deve_lock needs to disable bottom halves,
> but this needs to be fixed first.

<nod>

Applying the following + updating this original patch to use normal
spinlock_t access.

>From 1adff1b3a7f75a1c255b7fcab5676edf29d4a5d8 Mon Sep 17 00:00:00 2001
From: Nicholas Bellinger <nab@linux-iscsi.org>
Date: Mon, 22 Jun 2015 23:44:05 -0700
Subject: [PATCH 65/76] target: Convert se_lun->lun_deve_lock to normal
 spinlock

This patch converts se_lun->lun_deve_lock acquire/release access
to use a normal, non bottom-half spin_lock_t for protecting
se_lun->lun_deve_list access.

Reported-by: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_alua.c   |  4 ++--
 drivers/target/target_core_device.c | 12 ++++++------
 drivers/target/target_core_pr.c     |  8 ++++----
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index aa2e4b1..c56ae02 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -968,7 +968,7 @@ static void core_alua_queue_state_change_ua(struct t10_alua_tg_pt_gp *tg_pt_gp)
 			continue;
 		spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
 
-		spin_lock_bh(&lun->lun_deve_lock);
+		spin_lock(&lun->lun_deve_lock);
 		list_for_each_entry(se_deve, &lun->lun_deve_list, lun_link) {
 			lacl = rcu_dereference_check(se_deve->se_lun_acl,
 					lockdep_is_held(&lun->lun_deve_lock));
@@ -1000,7 +1000,7 @@ static void core_alua_queue_state_change_ua(struct t10_alua_tg_pt_gp *tg_pt_gp)
 			core_scsi3_ua_allocate(se_deve, 0x2A,
 				ASCQ_2AH_ASYMMETRIC_ACCESS_STATE_CHANGED);
 		}
-		spin_unlock_bh(&lun->lun_deve_lock);
+		spin_unlock(&lun->lun_deve_lock);
 
 		spin_lock(&tg_pt_gp->tg_pt_gp_lock);
 		percpu_ref_put(&lun->lun_ref);
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index ed08402..b6df5b9 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -352,10 +352,10 @@ int core_enable_device_list_for_node(
 		hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist);
 		mutex_unlock(&nacl->lun_entry_mutex);
 
-		spin_lock_bh(&lun->lun_deve_lock);
+		spin_lock(&lun->lun_deve_lock);
 		list_del(&orig->lun_link);
 		list_add_tail(&new->lun_link, &lun->lun_deve_list);
-		spin_unlock_bh(&lun->lun_deve_lock);
+		spin_unlock(&lun->lun_deve_lock);
 
 		kref_put(&orig->pr_kref, target_pr_kref_release);
 		wait_for_completion(&orig->pr_comp);
@@ -369,9 +369,9 @@ int core_enable_device_list_for_node(
 	hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist);
 	mutex_unlock(&nacl->lun_entry_mutex);
 
-	spin_lock_bh(&lun->lun_deve_lock);
+	spin_lock(&lun->lun_deve_lock);
 	list_add_tail(&new->lun_link, &lun->lun_deve_list);
-	spin_unlock_bh(&lun->lun_deve_lock);
+	spin_unlock(&lun->lun_deve_lock);
 
 	return 0;
 }
@@ -403,9 +403,9 @@ void core_disable_device_list_for_node(
 	 * NodeACL context specific PR metadata for demo-mode
 	 * MappedLUN *deve will be released below..
 	 */
-	spin_lock_bh(&lun->lun_deve_lock);
+	spin_lock(&lun->lun_deve_lock);
 	list_del(&orig->lun_link);
-	spin_unlock_bh(&lun->lun_deve_lock);
+	spin_unlock(&lun->lun_deve_lock);
 	/*
 	 * Disable struct se_dev_entry LUN ACL mapping
 	 */
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 0bb3292..7403b03 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -709,7 +709,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
 			continue;
 		spin_unlock(&dev->se_port_lock);
 
-		spin_lock_bh(&lun_tmp->lun_deve_lock);
+		spin_lock(&lun_tmp->lun_deve_lock);
 		list_for_each_entry(deve_tmp, &lun_tmp->lun_deve_list, lun_link) {
 			/*
 			 * This pointer will be NULL for demo mode MappedLUNs
@@ -742,7 +742,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
 				continue;
 
 			kref_get(&deve_tmp->pr_kref);
-			spin_unlock_bh(&lun_tmp->lun_deve_lock);
+			spin_unlock(&lun_tmp->lun_deve_lock);
 			/*
 			 * Grab a configfs group dependency that is released
 			 * for the exception path at label out: below, or upon
@@ -779,9 +779,9 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
 
 			list_add_tail(&pr_reg_atp->pr_reg_atp_mem_list,
 				      &pr_reg->pr_reg_atp_list);
-			spin_lock_bh(&lun_tmp->lun_deve_lock);
+			spin_lock(&lun_tmp->lun_deve_lock);
 		}
-		spin_unlock_bh(&lun_tmp->lun_deve_lock);
+		spin_unlock(&lun_tmp->lun_deve_lock);
 
 		spin_lock(&dev->se_port_lock);
 		percpu_ref_put(&lun_tmp->lun_ref);
-- 
1.9.1

  parent reply	other threads:[~2015-06-23  7:54 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-11  8:01 [PATCH 0/6] target: Update UA handling Hannes Reinecke
2015-06-11  8:01 ` [PATCH 1/6] target_core_alua: Correct UA handling when switching states Hannes Reinecke
2015-06-11  8:01 ` [PATCH 2/6] target: Remove 'ua_nacl' pointer from se_ua structure Hannes Reinecke
2015-06-11  8:01 ` [PATCH 3/6] target: use 'se_dev_entry' when allocating UAs Hannes Reinecke
2015-06-17  6:06   ` Nicholas A. Bellinger
2015-06-17  6:20     ` Hannes Reinecke
2015-06-11  8:01 ` [PATCH 4/6] target: Send UA on ALUA target port group change Hannes Reinecke
2015-06-19 13:05   ` Christoph Hellwig
2015-06-19 13:09     ` Hannes Reinecke
2015-06-19 13:13       ` Christoph Hellwig
2015-06-23  7:54     ` Nicholas A. Bellinger [this message]
2015-06-11  8:01 ` [PATCH 5/6] target: Send UA upon LUN RESET tmr completion Hannes Reinecke
2015-06-19 13:06   ` Christoph Hellwig
2015-06-19 13:07     ` Hannes Reinecke
2015-06-23  7:54     ` Nicholas A. Bellinger
2015-06-11  8:01 ` [PATCH 6/6] target: Send UA when changing LUN inventory Hannes Reinecke
2015-06-19 13:07   ` Christoph Hellwig
2015-06-19 13:10     ` Hannes Reinecke
2015-06-23  7:56     ` Nicholas A. Bellinger
2015-06-17  6:10 ` [PATCH 0/6] target: Update UA handling Nicholas A. Bellinger
2015-06-17  6:25   ` Hannes Reinecke
2015-06-17  7:01     ` Nicholas A. Bellinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1435046064.7460.23.camel@haakon3.risingtidesystems.com \
    --to=nab@linux-iscsi.org \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=linux-scsi@vger.kernel.org \
    --cc=nab@daterainc.com \
    --cc=target-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.