All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Blanchard <anton@samba.org>
To: Alexander Graf <agraf@suse.de>
Cc: Nishanth Aravamudan <nacc@linux.vnet.ibm.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	linux-nfs@vger.kernel.org, Jan Kara <jack@suse.cz>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	LKML List <linux-kernel@vger.kernel.org>,
	"J. Bruce Fields" <bfields@fieldses.org>,
	skinsbursky@parallels.com, bfields@redhat.com,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: [PATCH] powerpc/iommu: Fix multiple issues with IOMMU pools code
Date: Thu, 4 Oct 2012 14:57:10 +1000	[thread overview]
Message-ID: <20121004145710.2cf95dcd@kryten> (raw)
In-Reply-To: <E141CA01-F58C-47B1-8ED5-A314D1DEC968@suse.de>


Hi Alex,

Looks to be a preempt issue with the iommu pools code. I did find a
couple more bugs along the way too.

Anton
--

There are a number of issues in the recent IOMMU pools code:

- On a preempt kernel we might switch CPUs in the middle of building
  a scatter gather list. When this happens the handle hint passed in
  no longer falls within the local CPU's pool. Check for this and
  fall back to the pool hint.

- We were missing a spin_unlock/spin_lock in one spot where we
  switch pools.

- We need to provide locking around dart_tlb_invalidate_all and
  dart_tlb_invalidate_one now that the global lock is gone.

Reported-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Anton Blanchard <anton@samba.org>
---

There is still an issue with the lazy u3 flushing, but I wanted
to get this out for testing.

Index: b/arch/powerpc/kernel/iommu.c
===================================================================
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -215,7 +215,8 @@ static unsigned long iommu_range_alloc(s
 	spin_lock_irqsave(&(pool->lock), flags);
 
 again:
-	if ((pass == 0) && handle && *handle)
+	if ((pass == 0) && handle && *handle &&
+	    (*handle >= pool->start) && (*handle < pool->end))
 		start = *handle;
 	else
 		start = pool->hint;
@@ -236,7 +237,9 @@ again:
 		 * but on second pass, start at 0 in pool 0.
 		 */
 		if ((start & mask) >= limit || pass > 0) {
+			spin_unlock(&(pool->lock));
 			pool = &(tbl->pools[0]);
+			spin_lock(&(pool->lock));
 			start = pool->start;
 		} else {
 			start &= mask;
Index: b/arch/powerpc/sysdev/dart_iommu.c
===================================================================
--- a/arch/powerpc/sysdev/dart_iommu.c
+++ b/arch/powerpc/sysdev/dart_iommu.c
@@ -74,11 +74,16 @@ static int dart_is_u4;
 
 #define DBG(...)
 
+static DEFINE_SPINLOCK(invalidate_lock);
+
 static inline void dart_tlb_invalidate_all(void)
 {
 	unsigned long l = 0;
 	unsigned int reg, inv_bit;
 	unsigned long limit;
+	unsigned long flags;
+
+	spin_lock_irqsave(&invalidate_lock, flags);
 
 	DBG("dart: flush\n");
 
@@ -111,12 +116,17 @@ retry:
 			panic("DART: TLB did not flush after waiting a long "
 			      "time. Buggy U3 ?");
 	}
+
+	spin_unlock_irqrestore(&invalidate_lock, flags);
 }
 
 static inline void dart_tlb_invalidate_one(unsigned long bus_rpn)
 {
 	unsigned int reg;
 	unsigned int l, limit;
+	unsigned long flags;
+
+	spin_lock_irqsave(&invalidate_lock, flags);
 
 	reg = DART_CNTL_U4_ENABLE | DART_CNTL_U4_IONE |
 		(bus_rpn & DART_CNTL_U4_IONE_MASK);
@@ -138,6 +148,8 @@ wait_more:
 			panic("DART: TLB did not flush after waiting a long "
 			      "time. Buggy U4 ?");
 	}
+
+	spin_unlock_irqrestore(&invalidate_lock, flags);
 }
 
 static void dart_flush(struct iommu_table *tbl)

WARNING: multiple messages have this Message-ID (diff)
From: Anton Blanchard <anton@samba.org>
To: Alexander Graf <agraf@suse.de>
Cc: linux-nfs@vger.kernel.org, Jan Kara <jack@suse.cz>,
	Nishanth Aravamudan <nacc@linux.vnet.ibm.com>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	LKML List <linux-kernel@vger.kernel.org>,
	"J. Bruce Fields" <bfields@fieldses.org>,
	skinsbursky@parallels.com, bfields@redhat.com,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH] powerpc/iommu: Fix multiple issues with IOMMU pools code
Date: Thu, 4 Oct 2012 14:57:10 +1000	[thread overview]
Message-ID: <20121004145710.2cf95dcd@kryten> (raw)
In-Reply-To: <E141CA01-F58C-47B1-8ED5-A314D1DEC968@suse.de>


Hi Alex,

Looks to be a preempt issue with the iommu pools code. I did find a
couple more bugs along the way too.

Anton
--

There are a number of issues in the recent IOMMU pools code:

- On a preempt kernel we might switch CPUs in the middle of building
  a scatter gather list. When this happens the handle hint passed in
  no longer falls within the local CPU's pool. Check for this and
  fall back to the pool hint.

- We were missing a spin_unlock/spin_lock in one spot where we
  switch pools.

- We need to provide locking around dart_tlb_invalidate_all and
  dart_tlb_invalidate_one now that the global lock is gone.

Reported-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Anton Blanchard <anton@samba.org>
---

There is still an issue with the lazy u3 flushing, but I wanted
to get this out for testing.

Index: b/arch/powerpc/kernel/iommu.c
===================================================================
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -215,7 +215,8 @@ static unsigned long iommu_range_alloc(s
 	spin_lock_irqsave(&(pool->lock), flags);
 
 again:
-	if ((pass == 0) && handle && *handle)
+	if ((pass == 0) && handle && *handle &&
+	    (*handle >= pool->start) && (*handle < pool->end))
 		start = *handle;
 	else
 		start = pool->hint;
@@ -236,7 +237,9 @@ again:
 		 * but on second pass, start at 0 in pool 0.
 		 */
 		if ((start & mask) >= limit || pass > 0) {
+			spin_unlock(&(pool->lock));
 			pool = &(tbl->pools[0]);
+			spin_lock(&(pool->lock));
 			start = pool->start;
 		} else {
 			start &= mask;
Index: b/arch/powerpc/sysdev/dart_iommu.c
===================================================================
--- a/arch/powerpc/sysdev/dart_iommu.c
+++ b/arch/powerpc/sysdev/dart_iommu.c
@@ -74,11 +74,16 @@ static int dart_is_u4;
 
 #define DBG(...)
 
+static DEFINE_SPINLOCK(invalidate_lock);
+
 static inline void dart_tlb_invalidate_all(void)
 {
 	unsigned long l = 0;
 	unsigned int reg, inv_bit;
 	unsigned long limit;
+	unsigned long flags;
+
+	spin_lock_irqsave(&invalidate_lock, flags);
 
 	DBG("dart: flush\n");
 
@@ -111,12 +116,17 @@ retry:
 			panic("DART: TLB did not flush after waiting a long "
 			      "time. Buggy U3 ?");
 	}
+
+	spin_unlock_irqrestore(&invalidate_lock, flags);
 }
 
 static inline void dart_tlb_invalidate_one(unsigned long bus_rpn)
 {
 	unsigned int reg;
 	unsigned int l, limit;
+	unsigned long flags;
+
+	spin_lock_irqsave(&invalidate_lock, flags);
 
 	reg = DART_CNTL_U4_ENABLE | DART_CNTL_U4_IONE |
 		(bus_rpn & DART_CNTL_U4_IONE_MASK);
@@ -138,6 +148,8 @@ wait_more:
 			panic("DART: TLB did not flush after waiting a long "
 			      "time. Buggy U4 ?");
 	}
+
+	spin_unlock_irqrestore(&invalidate_lock, flags);
 }
 
 static void dart_flush(struct iommu_table *tbl)

  parent reply	other threads:[~2012-10-04  4:57 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-28  1:55 [REGRESSION] nfsd crashing with 3.6.0-rc7 on PowerPC Alexander Graf
2012-09-28  1:55 ` Alexander Graf
2012-09-28  2:04 ` Linus Torvalds
2012-09-28  2:04   ` Linus Torvalds
2012-09-28  2:19   ` Alexander Graf
2012-09-28  2:19     ` Alexander Graf
2012-09-28 15:10     ` J. Bruce Fields
2012-09-28 15:10       ` J. Bruce Fields
2012-09-28 15:34       ` Alexander Graf
2012-09-28 15:34         ` Alexander Graf
2012-10-01 14:03       ` Alexander Graf
2012-10-01 14:03         ` Alexander Graf
2012-10-01 15:21         ` J. Bruce Fields
2012-10-01 15:21           ` J. Bruce Fields
2012-10-02  0:58         ` Benjamin Herrenschmidt
2012-10-02  0:58           ` Benjamin Herrenschmidt
2012-10-02 21:43           ` Nishanth Aravamudan
2012-10-02 21:43             ` Nishanth Aravamudan
2012-10-02 21:47             ` Alexander Graf
2012-10-02 21:47               ` Alexander Graf
2012-10-02 22:17               ` Nishanth Aravamudan
2012-10-02 22:17                 ` Nishanth Aravamudan
2012-10-02 22:31                 ` Alexander Graf
2012-10-02 22:31                   ` Alexander Graf
2012-10-04  0:26                   ` Anton Blanchard
2012-10-04  0:26                     ` Anton Blanchard
2012-10-04  4:57                   ` Anton Blanchard [this message]
2012-10-04  4:57                     ` [PATCH] powerpc/iommu: Fix multiple issues with IOMMU pools code Anton Blanchard
2012-10-04 10:54                     ` Alexander Graf
2012-10-04 10:54                       ` Alexander Graf
2012-10-03  4:22             ` [REGRESSION] nfsd crashing with 3.6.0-rc7 on PowerPC Benjamin Herrenschmidt
2012-10-03  4:22               ` Benjamin Herrenschmidt

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=20121004145710.2cf95dcd@kryten \
    --to=anton@samba.org \
    --cc=agraf@suse.de \
    --cc=benh@kernel.crashing.org \
    --cc=bfields@fieldses.org \
    --cc=bfields@redhat.com \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=nacc@linux.vnet.ibm.com \
    --cc=skinsbursky@parallels.com \
    --cc=torvalds@linux-foundation.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.