Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1717246 > unrolled thread

[PATCH v3 00/17] Broadcom SBA-RAID driver improvements

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 00/17] Broadcom SBA-RAID driver improvements Anup Patel <anup.patel@broadcom.com> - 2017-08-22 12:00 +0200
    [PATCH v3 03/17] dmaengine: bcm-sba-raid: Common flags for sba_request state and fence Anup Patel <anup.patel@broadcom.com> - 2017-08-22 12:00 +0200
    [PATCH v3 04/17] dmaengine: bcm-sba-raid: Remove redundant next_count from sba_request Anup Patel <anup.patel@broadcom.com> - 2017-08-22 12:00 +0200
    [PATCH v3 02/17] dmaengine: bcm-sba-raid: Reduce locking context in sba_alloc_request() Anup Patel <anup.patel@broadcom.com> - 2017-08-22 12:00 +0200
    [PATCH v3 06/17] dmaengine: bcm-sba-raid: Remove reqs_free_count from sba_device Anup Patel <anup.patel@broadcom.com> - 2017-08-22 12:00 +0200
    [PATCH v3 05/17] dmaengine: bcm-sba-raid: Remove redundant resp_dma from sba_request Anup Patel <anup.patel@broadcom.com> - 2017-08-22 12:00 +0200
    [PATCH v3 09/17] dmaengine: bcm-sba-raid: Improve sba_issue_pending() run duration Anup Patel <anup.patel@broadcom.com> - 2017-08-22 12:00 +0200
    [PATCH v3 15/17] dmaengine: bcm-sba-raid: Add debugfs support Anup Patel <anup.patel@broadcom.com> - 2017-08-22 12:00 +0200

#1717246 — [PATCH v3 00/17] Broadcom SBA-RAID driver improvements

This patchset does various improvments to Broadcom SBA-RAID
driver.

The patches are based on Linux-4.13-rc3 and can also be found
at sba-raid-imp-v3 branch of
https://github.com/Broadcom/arm64-linux.git

Changes since v2:
 - Update patch description for PATCH1 and PATCH3
 - Add dev_err() for debugfs API failures instead of bailing out
 - Add PATCH17 to remove redundant SBA_REQUEST_STATE_COMPLETED

Changes since v1:
 - Split PATCH1 into 10 smaller patches for easier review
 - Added more info to commit description of PATCH2
 - Split PATCH4 into 2 patches for easier review
 - Dropped PATCH6 because it is now part of
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1456425.html

Anup Patel (17):
  dmaengine: bcm-sba-raid: Minor improvments in comments
  dmaengine: bcm-sba-raid: Reduce locking context in sba_alloc_request()
  dmaengine: bcm-sba-raid: Common flags for sba_request state and fence
  dmaengine: bcm-sba-raid: Remove redundant next_count from sba_request
  dmaengine: bcm-sba-raid: Remove redundant resp_dma from sba_request
  dmaengine: bcm-sba-raid: Remove reqs_free_count from sba_device
  dmaengine: bcm-sba-raid: Allow arbitrary number free sba_request
  dmaengine: bcm-sba-raid: Increase number of free sba_request
  dmaengine: bcm-sba-raid: Improve sba_issue_pending() run duration
  dmaengine: bcm-sba-raid: Alloc resources before registering DMA device
  dmaengine: bcm-sba-raid: Peek mbox when we have no free requests
  dmaengine: bcm-sba-raid: Pre-ack async tx descriptor
  dmaengine: bcm-sba-raid: Re-factor sba_process_deferred_requests()
  dmaengine: bcm-sba-raid: Remove redundant SBA_REQUEST_STATE_RECEIVED
  dmaengine: bcm-sba-raid: Add debugfs support
  dmaengine: bcm-sba-raid: Explicitly ACK mailbox message after sending
  dmaengine: bcm-sba-raid: Remove redundant SBA_REQUEST_STATE_COMPLETED

 drivers/dma/bcm-sba-raid.c | 538 ++++++++++++++++++++++++---------------------
 1 file changed, 292 insertions(+), 246 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1717247 — [PATCH v3 03/17] dmaengine: bcm-sba-raid: Common flags for sba_request state and fence

This patch merges sba_request state and fence into common
sba_request flags. The sba_request flags not only saves
memory but it can also be extended in-future without adding
new members.

We also make each sba_request state as separate bit in
sba_request flags to help debugging situations where a
sba_request is accidently in two states.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/dma/bcm-sba-raid.c | 66 ++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index f81d5ac..6fa3df1 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -91,22 +91,23 @@
 
 /* ===== Driver data structures ===== */
 
-enum sba_request_state {
-	SBA_REQUEST_STATE_FREE = 1,
-	SBA_REQUEST_STATE_ALLOCED = 2,
-	SBA_REQUEST_STATE_PENDING = 3,
-	SBA_REQUEST_STATE_ACTIVE = 4,
-	SBA_REQUEST_STATE_RECEIVED = 5,
-	SBA_REQUEST_STATE_COMPLETED = 6,
-	SBA_REQUEST_STATE_ABORTED = 7,
+enum sba_request_flags {
+	SBA_REQUEST_STATE_FREE		= 0x001,
+	SBA_REQUEST_STATE_ALLOCED	= 0x002,
+	SBA_REQUEST_STATE_PENDING	= 0x004,
+	SBA_REQUEST_STATE_ACTIVE	= 0x008,
+	SBA_REQUEST_STATE_RECEIVED	= 0x010,
+	SBA_REQUEST_STATE_COMPLETED	= 0x020,
+	SBA_REQUEST_STATE_ABORTED	= 0x040,
+	SBA_REQUEST_STATE_MASK		= 0x0ff,
+	SBA_REQUEST_FENCE		= 0x100,
 };
 
 struct sba_request {
 	/* Global state */
 	struct list_head node;
 	struct sba_device *sba;
-	enum sba_request_state state;
-	bool fence;
+	u32 flags;
 	/* Chained requests management */
 	struct sba_request *first;
 	struct list_head next;
@@ -217,8 +218,7 @@ static struct sba_request *sba_alloc_request(struct sba_device *sba)
 	if (!req)
 		return NULL;
 
-	req->state = SBA_REQUEST_STATE_ALLOCED;
-	req->fence = false;
+	req->flags = SBA_REQUEST_STATE_ALLOCED;
 	req->first = req;
 	INIT_LIST_HEAD(&req->next);
 	req->next_count = 1;
@@ -234,7 +234,8 @@ static void _sba_pending_request(struct sba_device *sba,
 				 struct sba_request *req)
 {
 	lockdep_assert_held(&sba->reqs_lock);
-	req->state = SBA_REQUEST_STATE_PENDING;
+	req->flags &= ~SBA_REQUEST_STATE_MASK;
+	req->flags |= SBA_REQUEST_STATE_PENDING;
 	list_move_tail(&req->node, &sba->reqs_pending_list);
 	if (list_empty(&sba->reqs_active_list))
 		sba->reqs_fence = false;
@@ -249,9 +250,10 @@ static bool _sba_active_request(struct sba_device *sba,
 		sba->reqs_fence = false;
 	if (sba->reqs_fence)
 		return false;
-	req->state = SBA_REQUEST_STATE_ACTIVE;
+	req->flags &= ~SBA_REQUEST_STATE_MASK;
+	req->flags |= SBA_REQUEST_STATE_ACTIVE;
 	list_move_tail(&req->node, &sba->reqs_active_list);
-	if (req->fence)
+	if (req->flags & SBA_REQUEST_FENCE)
 		sba->reqs_fence = true;
 	return true;
 }
@@ -261,7 +263,8 @@ static void _sba_abort_request(struct sba_device *sba,
 			       struct sba_request *req)
 {
 	lockdep_assert_held(&sba->reqs_lock);
-	req->state = SBA_REQUEST_STATE_ABORTED;
+	req->flags &= ~SBA_REQUEST_STATE_MASK;
+	req->flags |= SBA_REQUEST_STATE_ABORTED;
 	list_move_tail(&req->node, &sba->reqs_aborted_list);
 	if (list_empty(&sba->reqs_active_list))
 		sba->reqs_fence = false;
@@ -272,7 +275,8 @@ static void _sba_free_request(struct sba_device *sba,
 			      struct sba_request *req)
 {
 	lockdep_assert_held(&sba->reqs_lock);
-	req->state = SBA_REQUEST_STATE_FREE;
+	req->flags &= ~SBA_REQUEST_STATE_MASK;
+	req->flags |= SBA_REQUEST_STATE_FREE;
 	list_move_tail(&req->node, &sba->reqs_free_list);
 	if (list_empty(&sba->reqs_active_list))
 		sba->reqs_fence = false;
@@ -285,7 +289,8 @@ static void sba_received_request(struct sba_request *req)
 	struct sba_device *sba = req->sba;
 
 	spin_lock_irqsave(&sba->reqs_lock, flags);
-	req->state = SBA_REQUEST_STATE_RECEIVED;
+	req->flags &= ~SBA_REQUEST_STATE_MASK;
+	req->flags |= SBA_REQUEST_STATE_RECEIVED;
 	list_move_tail(&req->node, &sba->reqs_received_list);
 	spin_unlock_irqrestore(&sba->reqs_lock, flags);
 }
@@ -298,10 +303,12 @@ static void sba_complete_chained_requests(struct sba_request *req)
 
 	spin_lock_irqsave(&sba->reqs_lock, flags);
 
-	req->state = SBA_REQUEST_STATE_COMPLETED;
+	req->flags &= ~SBA_REQUEST_STATE_MASK;
+	req->flags |= SBA_REQUEST_STATE_COMPLETED;
 	list_move_tail(&req->node, &sba->reqs_completed_list);
 	list_for_each_entry(nreq, &req->next, next) {
-		nreq->state = SBA_REQUEST_STATE_COMPLETED;
+		nreq->flags &= ~SBA_REQUEST_STATE_MASK;
+		nreq->flags |= SBA_REQUEST_STATE_COMPLETED;
 		list_move_tail(&nreq->node, &sba->reqs_completed_list);
 	}
 	if (list_empty(&sba->reqs_active_list))
@@ -576,7 +583,7 @@ sba_prep_dma_interrupt(struct dma_chan *dchan, unsigned long flags)
 	 * Force fence so that no requests are submitted
 	 * until DMA callback for this request is invoked.
 	 */
-	req->fence = true;
+	req->flags |= SBA_REQUEST_FENCE;
 
 	/* Fillup request message */
 	sba_fillup_interrupt_msg(req, req->cmds, &req->msg);
@@ -659,7 +666,8 @@ sba_prep_dma_memcpy_req(struct sba_device *sba,
 	req = sba_alloc_request(sba);
 	if (!req)
 		return NULL;
-	req->fence = (flags & DMA_PREP_FENCE) ? true : false;
+	if (flags & DMA_PREP_FENCE)
+		req->flags |= SBA_REQUEST_FENCE;
 
 	/* Fillup request message */
 	sba_fillup_memcpy_msg(req, req->cmds, &req->msg,
@@ -796,7 +804,8 @@ sba_prep_dma_xor_req(struct sba_device *sba,
 	req = sba_alloc_request(sba);
 	if (!req)
 		return NULL;
-	req->fence = (flags & DMA_PREP_FENCE) ? true : false;
+	if (flags & DMA_PREP_FENCE)
+		req->flags |= SBA_REQUEST_FENCE;
 
 	/* Fillup request message */
 	sba_fillup_xor_msg(req, req->cmds, &req->msg,
@@ -1005,7 +1014,8 @@ sba_prep_dma_pq_req(struct sba_device *sba, dma_addr_t off,
 	req = sba_alloc_request(sba);
 	if (!req)
 		return NULL;
-	req->fence = (flags & DMA_PREP_FENCE) ? true : false;
+	if (flags & DMA_PREP_FENCE)
+		req->flags |= SBA_REQUEST_FENCE;
 
 	/* Fillup request messages */
 	sba_fillup_pq_msg(req, dmaf_continue(flags),
@@ -1258,7 +1268,8 @@ sba_prep_dma_pq_single_req(struct sba_device *sba, dma_addr_t off,
 	req = sba_alloc_request(sba);
 	if (!req)
 		return NULL;
-	req->fence = (flags & DMA_PREP_FENCE) ? true : false;
+	if (flags & DMA_PREP_FENCE)
+		req->flags |= SBA_REQUEST_FENCE;
 
 	/* Fillup request messages */
 	sba_fillup_pq_single_msg(req,  dmaf_continue(flags),
@@ -1425,7 +1436,7 @@ static void sba_receive_message(struct mbox_client *cl, void *msg)
 	req = req->first;
 
 	/* Update request */
-	if (req->state == SBA_REQUEST_STATE_RECEIVED)
+	if (req->flags & SBA_REQUEST_STATE_RECEIVED)
 		sba_dma_tx_actions(req);
 	else
 		sba_free_chained_requests(req);
@@ -1488,11 +1499,10 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
 		req = &sba->reqs[i];
 		INIT_LIST_HEAD(&req->node);
 		req->sba = sba;
-		req->state = SBA_REQUEST_STATE_FREE;
+		req->flags = SBA_REQUEST_STATE_FREE;
 		INIT_LIST_HEAD(&req->next);
 		req->next_count = 1;
 		atomic_set(&req->next_pending_count, 0);
-		req->fence = false;
 		req->resp = sba->resp_base + p;
 		req->resp_dma = sba->resp_dma_base + p;
 		p += sba->hw_resp_size;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1717248 — [PATCH v3 04/17] dmaengine: bcm-sba-raid: Remove redundant next_count from sba_request

The next_count in sba_request is redundant because same information
is captured by next_pending_count. This patch removes next_count
from sba_request.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/dma/bcm-sba-raid.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index 6fa3df1..e8863e9 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -111,7 +111,6 @@ struct sba_request {
 	/* Chained requests management */
 	struct sba_request *first;
 	struct list_head next;
-	unsigned int next_count;
 	atomic_t next_pending_count;
 	/* BRCM message data */
 	void *resp;
@@ -221,7 +220,6 @@ static struct sba_request *sba_alloc_request(struct sba_device *sba)
 	req->flags = SBA_REQUEST_STATE_ALLOCED;
 	req->first = req;
 	INIT_LIST_HEAD(&req->next);
-	req->next_count = 1;
 	atomic_set(&req->next_pending_count, 1);
 
 	dma_async_tx_descriptor_init(&req->tx, &sba->dma_chan);
@@ -342,8 +340,7 @@ static void sba_chain_request(struct sba_request *first,
 
 	list_add_tail(&req->next, &first->next);
 	req->first = first;
-	first->next_count++;
-	atomic_set(&first->next_pending_count, first->next_count);
+	atomic_inc(&first->next_pending_count);
 
 	spin_unlock_irqrestore(&sba->reqs_lock, flags);
 }
@@ -1501,7 +1498,6 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
 		req->sba = sba;
 		req->flags = SBA_REQUEST_STATE_FREE;
 		INIT_LIST_HEAD(&req->next);
-		req->next_count = 1;
 		atomic_set(&req->next_pending_count, 0);
 		req->resp = sba->resp_base + p;
 		req->resp_dma = sba->resp_dma_base + p;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1717249 — [PATCH v3 02/17] dmaengine: bcm-sba-raid: Reduce locking context in sba_alloc_request()

We don't require to hold "sba->reqs_lock" for long-time
in sba_alloc_request() because lock protection is not
required when initializing members of "struct sba_request".

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/dma/bcm-sba-raid.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index 76999b7..f81d5ac 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -207,24 +207,24 @@ static struct sba_request *sba_alloc_request(struct sba_device *sba)
 	struct sba_request *req = NULL;
 
 	spin_lock_irqsave(&sba->reqs_lock, flags);
-
 	req = list_first_entry_or_null(&sba->reqs_free_list,
 				       struct sba_request, node);
 	if (req) {
 		list_move_tail(&req->node, &sba->reqs_alloc_list);
-		req->state = SBA_REQUEST_STATE_ALLOCED;
-		req->fence = false;
-		req->first = req;
-		INIT_LIST_HEAD(&req->next);
-		req->next_count = 1;
-		atomic_set(&req->next_pending_count, 1);
-
 		sba->reqs_free_count--;
-
-		dma_async_tx_descriptor_init(&req->tx, &sba->dma_chan);
 	}
-
 	spin_unlock_irqrestore(&sba->reqs_lock, flags);
+	if (!req)
+		return NULL;
+
+	req->state = SBA_REQUEST_STATE_ALLOCED;
+	req->fence = false;
+	req->first = req;
+	INIT_LIST_HEAD(&req->next);
+	req->next_count = 1;
+	atomic_set(&req->next_pending_count, 1);
+
+	dma_async_tx_descriptor_init(&req->tx, &sba->dma_chan);
 
 	return req;
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1717250 — [PATCH v3 06/17] dmaengine: bcm-sba-raid: Remove reqs_free_count from sba_device

The reqs_free_count member of sba_device is not used anywhere
hence no point in tracking number of free sba_request.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/dma/bcm-sba-raid.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index 7d08d4e..59ef443 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -162,7 +162,6 @@ struct sba_device {
 	struct list_head reqs_completed_list;
 	struct list_head reqs_aborted_list;
 	struct list_head reqs_free_list;
-	int reqs_free_count;
 };
 
 /* ====== Command helper routines ===== */
@@ -207,10 +206,8 @@ static struct sba_request *sba_alloc_request(struct sba_device *sba)
 	spin_lock_irqsave(&sba->reqs_lock, flags);
 	req = list_first_entry_or_null(&sba->reqs_free_list,
 				       struct sba_request, node);
-	if (req) {
+	if (req)
 		list_move_tail(&req->node, &sba->reqs_alloc_list);
-		sba->reqs_free_count--;
-	}
 	spin_unlock_irqrestore(&sba->reqs_lock, flags);
 	if (!req)
 		return NULL;
@@ -276,7 +273,6 @@ static void _sba_free_request(struct sba_device *sba,
 	list_move_tail(&req->node, &sba->reqs_free_list);
 	if (list_empty(&sba->reqs_active_list))
 		sba->reqs_fence = false;
-	sba->reqs_free_count++;
 }
 
 static void sba_received_request(struct sba_request *req)
@@ -1523,8 +1519,6 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
 		list_add_tail(&req->node, &sba->reqs_free_list);
 	}
 
-	sba->reqs_free_count = sba->max_req;
-
 	return 0;
 
 fail_free_cmds_pool:
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1717251 — [PATCH v3 05/17] dmaengine: bcm-sba-raid: Remove redundant resp_dma from sba_request

Both resp and resp_dma are redundant in sba_request because
resp is unused and resp_dma carries same information present
in tx.phys of sba_request. This patch removes both resp and
resp_dma from sba_request.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/dma/bcm-sba-raid.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index e8863e9..7d08d4e 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -113,8 +113,6 @@ struct sba_request {
 	struct list_head next;
 	atomic_t next_pending_count;
 	/* BRCM message data */
-	void *resp;
-	dma_addr_t resp_dma;
 	struct brcm_sba_command *cmds;
 	struct brcm_message msg;
 	struct dma_async_tx_descriptor tx;
@@ -513,6 +511,7 @@ static void sba_fillup_interrupt_msg(struct sba_request *req,
 {
 	u64 cmd;
 	u32 c_mdata;
+	dma_addr_t resp_dma = req->tx.phys;
 	struct brcm_sba_command *cmdsp = cmds;
 
 	/* Type-B command to load dummy data into buf0 */
@@ -528,7 +527,7 @@ static void sba_fillup_interrupt_msg(struct sba_request *req,
 	cmdsp->cmd = cmd;
 	*cmdsp->cmd_dma = cpu_to_le64(cmd);
 	cmdsp->flags = BRCM_SBA_CMD_TYPE_B;
-	cmdsp->data = req->resp_dma;
+	cmdsp->data = resp_dma;
 	cmdsp->data_len = req->sba->hw_resp_size;
 	cmdsp++;
 
@@ -549,11 +548,11 @@ static void sba_fillup_interrupt_msg(struct sba_request *req,
 	cmdsp->flags = BRCM_SBA_CMD_TYPE_A;
 	if (req->sba->hw_resp_size) {
 		cmdsp->flags |= BRCM_SBA_CMD_HAS_RESP;
-		cmdsp->resp = req->resp_dma;
+		cmdsp->resp = resp_dma;
 		cmdsp->resp_len = req->sba->hw_resp_size;
 	}
 	cmdsp->flags |= BRCM_SBA_CMD_HAS_OUTPUT;
-	cmdsp->data = req->resp_dma;
+	cmdsp->data = resp_dma;
 	cmdsp->data_len = req->sba->hw_resp_size;
 	cmdsp++;
 
@@ -600,6 +599,7 @@ static void sba_fillup_memcpy_msg(struct sba_request *req,
 {
 	u64 cmd;
 	u32 c_mdata;
+	dma_addr_t resp_dma = req->tx.phys;
 	struct brcm_sba_command *cmdsp = cmds;
 
 	/* Type-B command to load data into buf0 */
@@ -636,7 +636,7 @@ static void sba_fillup_memcpy_msg(struct sba_request *req,
 	cmdsp->flags = BRCM_SBA_CMD_TYPE_A;
 	if (req->sba->hw_resp_size) {
 		cmdsp->flags |= BRCM_SBA_CMD_HAS_RESP;
-		cmdsp->resp = req->resp_dma;
+		cmdsp->resp = resp_dma;
 		cmdsp->resp_len = req->sba->hw_resp_size;
 	}
 	cmdsp->flags |= BRCM_SBA_CMD_HAS_OUTPUT;
@@ -719,6 +719,7 @@ static void sba_fillup_xor_msg(struct sba_request *req,
 	u64 cmd;
 	u32 c_mdata;
 	unsigned int i;
+	dma_addr_t resp_dma = req->tx.phys;
 	struct brcm_sba_command *cmdsp = cmds;
 
 	/* Type-B command to load data into buf0 */
@@ -774,7 +775,7 @@ static void sba_fillup_xor_msg(struct sba_request *req,
 	cmdsp->flags = BRCM_SBA_CMD_TYPE_A;
 	if (req->sba->hw_resp_size) {
 		cmdsp->flags |= BRCM_SBA_CMD_HAS_RESP;
-		cmdsp->resp = req->resp_dma;
+		cmdsp->resp = resp_dma;
 		cmdsp->resp_len = req->sba->hw_resp_size;
 	}
 	cmdsp->flags |= BRCM_SBA_CMD_HAS_OUTPUT;
@@ -863,6 +864,7 @@ static void sba_fillup_pq_msg(struct sba_request *req,
 	u64 cmd;
 	u32 c_mdata;
 	unsigned int i;
+	dma_addr_t resp_dma = req->tx.phys;
 	struct brcm_sba_command *cmdsp = cmds;
 
 	if (pq_continue) {
@@ -956,7 +958,7 @@ static void sba_fillup_pq_msg(struct sba_request *req,
 		cmdsp->flags = BRCM_SBA_CMD_TYPE_A;
 		if (req->sba->hw_resp_size) {
 			cmdsp->flags |= BRCM_SBA_CMD_HAS_RESP;
-			cmdsp->resp = req->resp_dma;
+			cmdsp->resp = resp_dma;
 			cmdsp->resp_len = req->sba->hw_resp_size;
 		}
 		cmdsp->flags |= BRCM_SBA_CMD_HAS_OUTPUT;
@@ -983,7 +985,7 @@ static void sba_fillup_pq_msg(struct sba_request *req,
 		cmdsp->flags = BRCM_SBA_CMD_TYPE_A;
 		if (req->sba->hw_resp_size) {
 			cmdsp->flags |= BRCM_SBA_CMD_HAS_RESP;
-			cmdsp->resp = req->resp_dma;
+			cmdsp->resp = resp_dma;
 			cmdsp->resp_len = req->sba->hw_resp_size;
 		}
 		cmdsp->flags |= BRCM_SBA_CMD_HAS_OUTPUT;
@@ -1037,6 +1039,7 @@ static void sba_fillup_pq_single_msg(struct sba_request *req,
 	u64 cmd;
 	u32 c_mdata;
 	u8 pos, dpos = raid6_gflog[scf];
+	dma_addr_t resp_dma = req->tx.phys;
 	struct brcm_sba_command *cmdsp = cmds;
 
 	if (!dst_p)
@@ -1115,7 +1118,7 @@ static void sba_fillup_pq_single_msg(struct sba_request *req,
 	cmdsp->flags = BRCM_SBA_CMD_TYPE_A;
 	if (req->sba->hw_resp_size) {
 		cmdsp->flags |= BRCM_SBA_CMD_HAS_RESP;
-		cmdsp->resp = req->resp_dma;
+		cmdsp->resp = resp_dma;
 		cmdsp->resp_len = req->sba->hw_resp_size;
 	}
 	cmdsp->flags |= BRCM_SBA_CMD_HAS_OUTPUT;
@@ -1236,7 +1239,7 @@ static void sba_fillup_pq_single_msg(struct sba_request *req,
 	cmdsp->flags = BRCM_SBA_CMD_TYPE_A;
 	if (req->sba->hw_resp_size) {
 		cmdsp->flags |= BRCM_SBA_CMD_HAS_RESP;
-		cmdsp->resp = req->resp_dma;
+		cmdsp->resp = resp_dma;
 		cmdsp->resp_len = req->sba->hw_resp_size;
 	}
 	cmdsp->flags |= BRCM_SBA_CMD_HAS_OUTPUT;
@@ -1458,7 +1461,7 @@ static void sba_receive_message(struct mbox_client *cl, void *msg)
 
 static int sba_prealloc_channel_resources(struct sba_device *sba)
 {
-	int i, j, p, ret = 0;
+	int i, j, ret = 0;
 	struct sba_request *req = NULL;
 
 	sba->resp_base = dma_alloc_coherent(sba->dma_dev.dev,
@@ -1492,16 +1495,13 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
 		goto fail_free_cmds_pool;
 	}
 
-	for (i = 0, p = 0; i < sba->max_req; i++) {
+	for (i = 0; i < sba->max_req; i++) {
 		req = &sba->reqs[i];
 		INIT_LIST_HEAD(&req->node);
 		req->sba = sba;
 		req->flags = SBA_REQUEST_STATE_FREE;
 		INIT_LIST_HEAD(&req->next);
 		atomic_set(&req->next_pending_count, 0);
-		req->resp = sba->resp_base + p;
-		req->resp_dma = sba->resp_dma_base + p;
-		p += sba->hw_resp_size;
 		req->cmds = devm_kcalloc(sba->dev, sba->max_cmd_per_req,
 					 sizeof(*req->cmds), GFP_KERNEL);
 		if (!req->cmds) {
@@ -1519,7 +1519,7 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
 		memset(&req->msg, 0, sizeof(req->msg));
 		dma_async_tx_descriptor_init(&req->tx, &sba->dma_chan);
 		req->tx.tx_submit = sba_tx_submit;
-		req->tx.phys = req->resp_dma;
+		req->tx.phys = sba->resp_dma_base + i * sba->hw_resp_size;
 		list_add_tail(&req->node, &sba->reqs_free_list);
 	}
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1717252 — [PATCH v3 09/17] dmaengine: bcm-sba-raid: Improve sba_issue_pending() run duration

The pending sba_request list can become very long in real-life usage
(e.g. setting up RAID array) which can cause sba_issue_pending() to
run for long duration.

This patch adds common sba_process_deferred_requests() to process
few completed and pending requests so that it finishes in short
duration. We use this common sba_process_deferred_requests() in
both sba_issue_pending() and sba_receive_message().

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/dma/bcm-sba-raid.c | 234 ++++++++++++++++++++++++---------------------
 1 file changed, 125 insertions(+), 109 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index af6cc8e..f6616da 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -277,38 +277,28 @@ static void _sba_free_request(struct sba_device *sba,
 		sba->reqs_fence = false;
 }
 
-static void sba_received_request(struct sba_request *req)
+/* Note: Must be called with sba->reqs_lock held */
+static void _sba_complete_request(struct sba_device *sba,
+				  struct sba_request *req)
 {
-	unsigned long flags;
-	struct sba_device *sba = req->sba;
-
-	spin_lock_irqsave(&sba->reqs_lock, flags);
+	lockdep_assert_held(&sba->reqs_lock);
 	req->flags &= ~SBA_REQUEST_STATE_MASK;
-	req->flags |= SBA_REQUEST_STATE_RECEIVED;
-	list_move_tail(&req->node, &sba->reqs_received_list);
-	spin_unlock_irqrestore(&sba->reqs_lock, flags);
+	req->flags |= SBA_REQUEST_STATE_COMPLETED;
+	list_move_tail(&req->node, &sba->reqs_completed_list);
+	if (list_empty(&sba->reqs_active_list))
+		sba->reqs_fence = false;
 }
 
-static void sba_complete_chained_requests(struct sba_request *req)
+/* Note: Must be called with sba->reqs_lock held */
+static void _sba_received_request(struct sba_device *sba,
+				  struct sba_request *req)
 {
-	unsigned long flags;
-	struct sba_request *nreq;
-	struct sba_device *sba = req->sba;
-
-	spin_lock_irqsave(&sba->reqs_lock, flags);
-
+	lockdep_assert_held(&sba->reqs_lock);
 	req->flags &= ~SBA_REQUEST_STATE_MASK;
-	req->flags |= SBA_REQUEST_STATE_COMPLETED;
-	list_move_tail(&req->node, &sba->reqs_completed_list);
-	list_for_each_entry(nreq, &req->next, next) {
-		nreq->flags &= ~SBA_REQUEST_STATE_MASK;
-		nreq->flags |= SBA_REQUEST_STATE_COMPLETED;
-		list_move_tail(&nreq->node, &sba->reqs_completed_list);
-	}
+	req->flags |= SBA_REQUEST_STATE_RECEIVED;
+	list_move_tail(&req->node, &sba->reqs_received_list);
 	if (list_empty(&sba->reqs_active_list))
 		sba->reqs_fence = false;
-
-	spin_unlock_irqrestore(&sba->reqs_lock, flags);
 }
 
 static void sba_free_chained_requests(struct sba_request *req)
@@ -386,26 +376,6 @@ static void sba_cleanup_pending_requests(struct sba_device *sba)
 	spin_unlock_irqrestore(&sba->reqs_lock, flags);
 }
 
-/* ====== DMAENGINE callbacks ===== */
-
-static void sba_free_chan_resources(struct dma_chan *dchan)
-{
-	/*
-	 * Channel resources are pre-alloced so we just free-up
-	 * whatever we can so that we can re-use pre-alloced
-	 * channel resources next time.
-	 */
-	sba_cleanup_nonpending_requests(to_sba_device(dchan));
-}
-
-static int sba_device_terminate_all(struct dma_chan *dchan)
-{
-	/* Cleanup all pending requests */
-	sba_cleanup_pending_requests(to_sba_device(dchan));
-
-	return 0;
-}
-
 static int sba_send_mbox_request(struct sba_device *sba,
 				 struct sba_request *req)
 {
@@ -431,17 +401,27 @@ static int sba_send_mbox_request(struct sba_device *sba,
 	return 0;
 }
 
-static void sba_issue_pending(struct dma_chan *dchan)
+static void sba_process_deferred_requests(struct sba_device *sba)
 {
 	int ret;
+	u32 count;
 	unsigned long flags;
-	struct sba_request *req, *req1;
-	struct sba_device *sba = to_sba_device(dchan);
+	struct sba_request *req;
+	struct dma_async_tx_descriptor *tx;
 
 	spin_lock_irqsave(&sba->reqs_lock, flags);
 
-	/* Process all pending request */
-	list_for_each_entry_safe(req, req1, &sba->reqs_pending_list, node) {
+	/* Count pending requests */
+	count = 0;
+	list_for_each_entry(req, &sba->reqs_pending_list, node)
+		count++;
+
+	/* Process pending requests */
+	while (!list_empty(&sba->reqs_pending_list) && count) {
+		/* Get the first pending request */
+		req = list_first_entry(&sba->reqs_pending_list,
+				       struct sba_request, node);
+
 		/* Try to make request active */
 		if (!_sba_active_request(sba, req))
 			break;
@@ -456,11 +436,102 @@ static void sba_issue_pending(struct dma_chan *dchan)
 			_sba_pending_request(sba, req);
 			break;
 		}
+
+		count--;
+	}
+
+	/* Count completed requests */
+	count = 0;
+	list_for_each_entry(req, &sba->reqs_completed_list, node)
+		count++;
+
+	/* Process completed requests */
+	while (!list_empty(&sba->reqs_completed_list) && count) {
+		req = list_first_entry(&sba->reqs_completed_list,
+					struct sba_request, node);
+		list_del_init(&req->node);
+		tx = &req->tx;
+
+		spin_unlock_irqrestore(&sba->reqs_lock, flags);
+
+		WARN_ON(tx->cookie < 0);
+		if (tx->cookie > 0) {
+			dma_cookie_complete(tx);
+			dmaengine_desc_get_callback_invoke(tx, NULL);
+			dma_descriptor_unmap(tx);
+			tx->callback = NULL;
+			tx->callback_result = NULL;
+		}
+
+		dma_run_dependencies(tx);
+
+		spin_lock_irqsave(&sba->reqs_lock, flags);
+
+		/* If waiting for 'ack' then move to completed list */
+		if (!async_tx_test_ack(&req->tx))
+			_sba_complete_request(sba, req);
+		else
+			_sba_free_request(sba, req);
+
+		count--;
 	}
 
+	/* Re-check pending and completed work */
+	count = 0;
+	if (!list_empty(&sba->reqs_pending_list) ||
+	    !list_empty(&sba->reqs_completed_list))
+		count = 1;
+
 	spin_unlock_irqrestore(&sba->reqs_lock, flags);
 }
 
+static void sba_process_received_request(struct sba_device *sba,
+					 struct sba_request *req)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&sba->reqs_lock, flags);
+
+	/* Mark request as received */
+	_sba_received_request(sba, req);
+
+	/* Update request */
+	if (!atomic_dec_return(&req->first->next_pending_count))
+		_sba_complete_request(sba, req->first);
+	if (req->first != req)
+		_sba_free_request(sba, req);
+
+	spin_unlock_irqrestore(&sba->reqs_lock, flags);
+}
+
+/* ====== DMAENGINE callbacks ===== */
+
+static void sba_free_chan_resources(struct dma_chan *dchan)
+{
+	/*
+	 * Channel resources are pre-alloced so we just free-up
+	 * whatever we can so that we can re-use pre-alloced
+	 * channel resources next time.
+	 */
+	sba_cleanup_nonpending_requests(to_sba_device(dchan));
+}
+
+static int sba_device_terminate_all(struct dma_chan *dchan)
+{
+	/* Cleanup all pending requests */
+	sba_cleanup_pending_requests(to_sba_device(dchan));
+
+	return 0;
+}
+
+static void sba_issue_pending(struct dma_chan *dchan)
+{
+	struct sba_device *sba = to_sba_device(dchan);
+
+	/* Process deferred requests */
+	sba_process_deferred_requests(sba);
+}
+
 static dma_cookie_t sba_tx_submit(struct dma_async_tx_descriptor *tx)
 {
 	unsigned long flags;
@@ -1382,40 +1453,10 @@ sba_prep_dma_pq(struct dma_chan *dchan, dma_addr_t *dst, dma_addr_t *src,
 
 /* ====== Mailbox callbacks ===== */
 
-static void sba_dma_tx_actions(struct sba_request *req)
-{
-	struct dma_async_tx_descriptor *tx = &req->tx;
-
-	WARN_ON(tx->cookie < 0);
-
-	if (tx->cookie > 0) {
-		dma_cookie_complete(tx);
-
-		/*
-		 * Call the callback (must not sleep or submit new
-		 * operations to this channel)
-		 */
-		if (tx->callback)
-			tx->callback(tx->callback_param);
-
-		dma_descriptor_unmap(tx);
-	}
-
-	/* Run dependent operations */
-	dma_run_dependencies(tx);
-
-	/* If waiting for 'ack' then move to completed list */
-	if (!async_tx_test_ack(&req->tx))
-		sba_complete_chained_requests(req);
-	else
-		sba_free_chained_requests(req);
-}
-
 static void sba_receive_message(struct mbox_client *cl, void *msg)
 {
-	unsigned long flags;
 	struct brcm_message *m = msg;
-	struct sba_request *req = m->ctx, *req1;
+	struct sba_request *req = m->ctx;
 	struct sba_device *sba = req->sba;
 
 	/* Error count if message has error */
@@ -1423,36 +1464,11 @@ static void sba_receive_message(struct mbox_client *cl, void *msg)
 		dev_err(sba->dev, "%s got message with error %d",
 			dma_chan_name(&sba->dma_chan), m->error);
 
-	/* Mark request as received */
-	sba_received_request(req);
-
-	/* Wait for all chained requests to be completed */
-	if (atomic_dec_return(&req->first->next_pending_count))
-		goto done;
-
-	/* Point to first request */
-	req = req->first;
-
-	/* Update request */
-	if (req->flags & SBA_REQUEST_STATE_RECEIVED)
-		sba_dma_tx_actions(req);
-	else
-		sba_free_chained_requests(req);
-
-	spin_lock_irqsave(&sba->reqs_lock, flags);
-
-	/* Re-check all completed request waiting for 'ack' */
-	list_for_each_entry_safe(req, req1, &sba->reqs_completed_list, node) {
-		spin_unlock_irqrestore(&sba->reqs_lock, flags);
-		sba_dma_tx_actions(req);
-		spin_lock_irqsave(&sba->reqs_lock, flags);
-	}
-
-	spin_unlock_irqrestore(&sba->reqs_lock, flags);
+	/* Process received request */
+	sba_process_received_request(sba, req);
 
-done:
-	/* Try to submit pending request */
-	sba_issue_pending(&sba->dma_chan);
+	/* Process deferred requests */
+	sba_process_deferred_requests(sba);
 }
 
 /* ====== Platform driver routines ===== */
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1717254 — [PATCH v3 15/17] dmaengine: bcm-sba-raid: Add debugfs support

This patch adds debugfs support to report stats via debugfs
which in-turn will help debug hang or error situations.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 drivers/dma/bcm-sba-raid.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index f0a0e80..12310aa 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -36,6 +36,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/debugfs.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
 #include <linux/list.h>
@@ -162,6 +163,9 @@ struct sba_device {
 	struct list_head reqs_completed_list;
 	struct list_head reqs_aborted_list;
 	struct list_head reqs_free_list;
+	/* DebugFS directory entries */
+	struct dentry *root;
+	struct dentry *stats;
 };
 
 /* ====== Command helper routines ===== */
@@ -486,6 +490,45 @@ static void sba_process_received_request(struct sba_device *sba,
 	}
 }
 
+static void sba_write_stats_in_seqfile(struct sba_device *sba,
+				       struct seq_file *file)
+{
+	unsigned long flags;
+	struct sba_request *req;
+	u32 free_count = 0, alloced_count = 0, pending_count = 0;
+	u32 active_count = 0, aborted_count = 0, completed_count = 0;
+
+	spin_lock_irqsave(&sba->reqs_lock, flags);
+
+	list_for_each_entry(req, &sba->reqs_free_list, node)
+		free_count++;
+
+	list_for_each_entry(req, &sba->reqs_alloc_list, node)
+		alloced_count++;
+
+	list_for_each_entry(req, &sba->reqs_pending_list, node)
+		pending_count++;
+
+	list_for_each_entry(req, &sba->reqs_active_list, node)
+		active_count++;
+
+	list_for_each_entry(req, &sba->reqs_aborted_list, node)
+		aborted_count++;
+
+	list_for_each_entry(req, &sba->reqs_completed_list, node)
+		completed_count++;
+
+	spin_unlock_irqrestore(&sba->reqs_lock, flags);
+
+	seq_printf(file, "maximum requests   = %d\n", sba->max_req);
+	seq_printf(file, "free requests      = %d\n", free_count);
+	seq_printf(file, "alloced requests   = %d\n", alloced_count);
+	seq_printf(file, "pending requests   = %d\n", pending_count);
+	seq_printf(file, "active requests    = %d\n", active_count);
+	seq_printf(file, "aborted requests   = %d\n", aborted_count);
+	seq_printf(file, "completed requests = %d\n", completed_count);
+}
+
 /* ====== DMAENGINE callbacks ===== */
 
 static void sba_free_chan_resources(struct dma_chan *dchan)
@@ -1451,6 +1494,19 @@ static void sba_receive_message(struct mbox_client *cl, void *msg)
 	sba_process_received_request(sba, req);
 }
 
+/* ====== Debugfs callbacks ====== */
+
+static int sba_debugfs_stats_show(struct seq_file *file, void *offset)
+{
+	struct platform_device *pdev = to_platform_device(file->private);
+	struct sba_device *sba = platform_get_drvdata(pdev);
+
+	/* Write stats in file */
+	sba_write_stats_in_seqfile(sba, file);
+
+	return 0;
+}
+
 /* ====== Platform driver routines ===== */
 
 static int sba_prealloc_channel_resources(struct sba_device *sba)
@@ -1721,6 +1777,25 @@ static int sba_probe(struct platform_device *pdev)
 	if (ret)
 		goto fail_free_mchans;
 
+	/* Check availability of debugfs */
+	if (!debugfs_initialized())
+		goto skip_debugfs;
+
+	/* Create debugfs root entry */
+	sba->root = debugfs_create_dir(dev_name(sba->dev), NULL);
+	if (IS_ERR_OR_NULL(sba->root)) {
+		dev_err(sba->dev, "failed to create debugfs root entry\n");
+		sba->root = NULL;
+		goto skip_debugfs;
+	}
+
+	/* Create debugfs stats entry */
+	sba->stats = debugfs_create_devm_seqfile(sba->dev, "stats", sba->root,
+						 sba_debugfs_stats_show);
+	if (IS_ERR_OR_NULL(sba->stats))
+		dev_err(sba->dev, "failed to create debugfs stats file\n");
+skip_debugfs:
+
 	/* Register DMA device with Linux async framework */
 	ret = sba_async_register(sba);
 	if (ret)
@@ -1734,6 +1809,7 @@ static int sba_probe(struct platform_device *pdev)
 	return 0;
 
 fail_free_resources:
+	debugfs_remove_recursive(sba->root);
 	sba_freeup_channel_resources(sba);
 fail_free_mchans:
 	for (i = 0; i < sba->mchans_count; i++)
@@ -1748,6 +1824,8 @@ static int sba_remove(struct platform_device *pdev)
 
 	dma_async_device_unregister(&sba->dma_dev);
 
+	debugfs_remove_recursive(sba->root);
+
 	sba_freeup_channel_resources(sba);
 
 	for (i = 0; i < sba->mchans_count; i++)
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web