[PATCH v2 0/3] mv_xor: Spinlock cleanup

[PATCH v2 0/3] mv_xor: Spinlock cleanup

[Ezequiel Garcia]

Vinod, Dan,

This series is v2 of the one previously sent by Thomas Petazzoni [1].
I've tried to address the comments made by Andi and Dan, and also kept
Thomas' ultimate goal: get rid of the mv_xor_slot_cleanup() function
(which takes the spinlock).

The first patch removes the unneeded call to mv_xor_clean_completed_slots(),
in mv_xor_as explained by Dan [2].

The other two patches replace the calls to mv_xor_slot_cleanup() with
calls to __mv_xor_slot_cleanup(). This allows to have the same "spinlock held"
requirement on all callers.

The patchset is based on v3.14-rc5 and has been tested on an Armada XP GP board.

[1] http://www.spinics.net/lists/arm-kernel/msg297176.html
[2] http://www.spinics.net/lists/arm-kernel/msg297518.html

Ezequiel Garcia (3):
  dma: mv_xor: Remove unneeded mv_xor_clean_completed_slots() call
  dma: mv_xor: Remove all callers of mv_xor_slot_cleanup()
  dma: mv_xor: Rename __mv_xor_slot_cleanup() to mv_xor_slot_cleanup()

 drivers/dma/mv_xor.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

-- 
1.9.0

[PATCH v2 1/3] dma: mv_xor: Remove unneeded mv_xor_clean_completed_slots() call

[Ezequiel Garcia]

In mv_xor_status(), we are currently calling mv_xor_clean_completed_slots()
when the transaction is complete (the cookie status is DMA_COMPLETE).
However, a completed status means that mv_xor_slot_cleanup() was called,
which cleans the completed slots.

In other words, there's nothing to cleanup for a completed transaction in
mv_xor_status(). Remove the unneeded call to mv_xor_clean_completed_slots().

Reported-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/dma/mv_xor.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 766b68e..589917b 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -702,10 +702,8 @@ static enum dma_status mv_xor_status(struct dma_chan *chan,
 	enum dma_status ret;
 
 	ret = dma_cookie_status(chan, cookie, txstate);
-	if (ret == DMA_COMPLETE) {
-		mv_xor_clean_completed_slots(mv_chan);
+	if (ret == DMA_COMPLETE)
 		return ret;
-	}
 	mv_xor_slot_cleanup(mv_chan);
 
 	return dma_cookie_status(chan, cookie, txstate);
-- 
1.9.0

[PATCH v2 2/3] dma: mv_xor: Remove all callers of mv_xor_slot_cleanup()

[Ezequiel Garcia]

In order to simplify the code, remove all the calls to the locked
mv_xor_slot_cleanup() and instead use the unlocked version only,

It's less error prone to have just one function, and require the caller
to ensure proper locking.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/dma/mv_xor.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 589917b..c60662a 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -379,7 +379,10 @@ mv_xor_slot_cleanup(struct mv_xor_chan *mv_chan)
 static void mv_xor_tasklet(unsigned long data)
 {
 	struct mv_xor_chan *chan = (struct mv_xor_chan *) data;
-	mv_xor_slot_cleanup(chan);
+
+	spin_lock_bh(&chan->lock);
+	__mv_xor_slot_cleanup(chan);
+	spin_unlock_bh(&chan->lock);
 }
 
 static struct mv_xor_desc_slot *
@@ -658,9 +661,10 @@ static void mv_xor_free_chan_resources(struct dma_chan *chan)
 	struct mv_xor_desc_slot *iter, *_iter;
 	int in_use_descs = 0;
 
-	mv_xor_slot_cleanup(mv_chan);
-
 	spin_lock_bh(&mv_chan->lock);
+
+	__mv_xor_slot_cleanup(mv_chan);
+
 	list_for_each_entry_safe(iter, _iter, &mv_chan->chain,
 					chain_node) {
 		in_use_descs++;
@@ -704,7 +708,10 @@ static enum dma_status mv_xor_status(struct dma_chan *chan,
 	ret = dma_cookie_status(chan, cookie, txstate);
 	if (ret == DMA_COMPLETE)
 		return ret;
-	mv_xor_slot_cleanup(mv_chan);
+
+	spin_lock_bh(&mv_chan->lock);
+	__mv_xor_slot_cleanup(mv_chan);
+	spin_unlock_bh(&mv_chan->lock);
 
 	return dma_cookie_status(chan, cookie, txstate);
 }
-- 
1.9.0

[PATCH v2 3/3] dma: mv_xor: Rename __mv_xor_slot_cleanup() to mv_xor_slot_cleanup()

[Ezequiel Garcia]

Now that mv_xor_slot_cleanup() has no remaining callers, we remove it
and rename __mv_xor_slot_cleanup() to mv_xor_slot_cleanup().

We take this opportunity to add a comment that makes it clear that the
channel spinlock should be held before calling mv_xor_slot_cleanup().

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/dma/mv_xor.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index c60662a..5031b78 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -312,7 +312,8 @@ mv_xor_clean_slot(struct mv_xor_desc_slot *desc,
 	return 0;
 }
 
-static void __mv_xor_slot_cleanup(struct mv_xor_chan *mv_chan)
+/* This function must be called with the mv_xor_chan spinlock held */
+static void mv_xor_slot_cleanup(struct mv_xor_chan *mv_chan)
 {
 	struct mv_xor_desc_slot *iter, *_iter;
 	dma_cookie_t cookie = 0;
@@ -368,20 +369,12 @@ static void __mv_xor_slot_cleanup(struct mv_xor_chan *mv_chan)
 		mv_chan->dmachan.completed_cookie = cookie;
 }
 
-static void
-mv_xor_slot_cleanup(struct mv_xor_chan *mv_chan)
-{
-	spin_lock_bh(&mv_chan->lock);
-	__mv_xor_slot_cleanup(mv_chan);
-	spin_unlock_bh(&mv_chan->lock);
-}
-
 static void mv_xor_tasklet(unsigned long data)
 {
 	struct mv_xor_chan *chan = (struct mv_xor_chan *) data;
 
 	spin_lock_bh(&chan->lock);
-	__mv_xor_slot_cleanup(chan);
+	mv_xor_slot_cleanup(chan);
 	spin_unlock_bh(&chan->lock);
 }
 
@@ -663,7 +656,7 @@ static void mv_xor_free_chan_resources(struct dma_chan *chan)
 
 	spin_lock_bh(&mv_chan->lock);
 
-	__mv_xor_slot_cleanup(mv_chan);
+	mv_xor_slot_cleanup(mv_chan);
 
 	list_for_each_entry_safe(iter, _iter, &mv_chan->chain,
 					chain_node) {
@@ -710,7 +703,7 @@ static enum dma_status mv_xor_status(struct dma_chan *chan,
 		return ret;
 
 	spin_lock_bh(&mv_chan->lock);
-	__mv_xor_slot_cleanup(mv_chan);
+	mv_xor_slot_cleanup(mv_chan);
 	spin_unlock_bh(&mv_chan->lock);
 
 	return dma_cookie_status(chan, cookie, txstate);
-- 
1.9.0

[PATCH v2 0/3] mv_xor: Spinlock cleanup

[Ezequiel Garcia]

On Mar 07, Ezequiel Garcia wrote:
> Vinod, Dan,
> 
> This series is v2 of the one previously sent by Thomas Petazzoni [1].
> I've tried to address the comments made by Andi and Dan, and also kept
> Thomas' ultimate goal: get rid of the mv_xor_slot_cleanup() function
> (which takes the spinlock).
> 
> The first patch removes the unneeded call to mv_xor_clean_completed_slots(),
> in mv_xor_as explained by Dan [2].
> 
> The other two patches replace the calls to mv_xor_slot_cleanup() with
> calls to __mv_xor_slot_cleanup(). This allows to have the same "spinlock held"
> requirement on all callers.
> 
> The patchset is based on v3.14-rc5 and has been tested on an Armada XP GP board.
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg297176.html
> [2] http://www.spinics.net/lists/arm-kernel/msg297518.html
> 
> Ezequiel Garcia (3):
>   dma: mv_xor: Remove unneeded mv_xor_clean_completed_slots() call
>   dma: mv_xor: Remove all callers of mv_xor_slot_cleanup()
>   dma: mv_xor: Rename __mv_xor_slot_cleanup() to mv_xor_slot_cleanup()
> 
>  drivers/dma/mv_xor.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 

Ping?

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH v2 0/3] mv_xor: Spinlock cleanup

[Ezequiel Garcia]

On Mar 13, Ezequiel Garcia wrote:
> On Mar 07, Ezequiel Garcia wrote:
> > Vinod, Dan,
> > 
> > This series is v2 of the one previously sent by Thomas Petazzoni [1].
> > I've tried to address the comments made by Andi and Dan, and also kept
> > Thomas' ultimate goal: get rid of the mv_xor_slot_cleanup() function
> > (which takes the spinlock).
> > 
> > The first patch removes the unneeded call to mv_xor_clean_completed_slots(),
> > in mv_xor_as explained by Dan [2].
> > 
> > The other two patches replace the calls to mv_xor_slot_cleanup() with
> > calls to __mv_xor_slot_cleanup(). This allows to have the same "spinlock held"
> > requirement on all callers.
> > 
> > The patchset is based on v3.14-rc5 and has been tested on an Armada XP GP board.
> > 
> > [1] http://www.spinics.net/lists/arm-kernel/msg297176.html
> > [2] http://www.spinics.net/lists/arm-kernel/msg297518.html
> > 
> > Ezequiel Garcia (3):
> >   dma: mv_xor: Remove unneeded mv_xor_clean_completed_slots() call
> >   dma: mv_xor: Remove all callers of mv_xor_slot_cleanup()
> >   dma: mv_xor: Rename __mv_xor_slot_cleanup() to mv_xor_slot_cleanup()
> > 
> >  drivers/dma/mv_xor.c | 24 +++++++++++-------------
> >  1 file changed, 11 insertions(+), 13 deletions(-)
> > 
> 
> Ping?
> 

Ping^2 ?

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com