2017-08-22 09:57:28

by Anup Patel

[permalink] [raw]
Subject: [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/[email protected]/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


2017-08-22 09:57:37

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 01/17] dmaengine: bcm-sba-raid: Minor improvments in comments

This patch does following improvments to comments:
1. Make section comments consistent across the driver by
avoiding " SBA " in some of the comments
2. Add/update few more section comments

Signed-off-by: Anup Patel <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index e41bbc7..76999b7 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -48,7 +48,8 @@

#include "dmaengine.h"

-/* SBA command related defines */
+/* ====== Driver macros and defines ===== */
+
#define SBA_TYPE_SHIFT 48
#define SBA_TYPE_MASK GENMASK(1, 0)
#define SBA_TYPE_A 0x0
@@ -88,6 +89,8 @@
#define to_sba_device(dchan) \
container_of(dchan, struct sba_device, dma_chan)

+/* ===== Driver data structures ===== */
+
enum sba_request_state {
SBA_REQUEST_STATE_FREE = 1,
SBA_REQUEST_STATE_ALLOCED = 2,
@@ -164,7 +167,7 @@ struct sba_device {
int reqs_free_count;
};

-/* ====== SBA command helper routines ===== */
+/* ====== Command helper routines ===== */

static inline u64 __pure sba_cmd_enc(u64 cmd, u32 val, u32 shift, u32 mask)
{
@@ -196,7 +199,7 @@ static inline u32 __pure sba_cmd_pq_c_mdata(u32 d, u32 b1, u32 b0)
((d & SBA_C_MDATA_DNUM_MASK) << SBA_C_MDATA_DNUM_SHIFT);
}

-/* ====== Channel resource management routines ===== */
+/* ====== General helper routines ===== */

static struct sba_request *sba_alloc_request(struct sba_device *sba)
{
--
2.7.4

2017-08-22 09:57:43

by Anup Patel

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2017-08-22 09:57:51

by Anup Patel

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2017-08-22 09:57:57

by Anup Patel

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2017-08-22 09:58:05

by Anup Patel

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2017-08-22 09:58:11

by Anup Patel

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2017-08-22 09:58:16

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 07/17] dmaengine: bcm-sba-raid: Allow arbitrary number free sba_request

Currently, we cannot have any arbitrary number of free sba_request
because sba_prealloc_channel_resources() allocates an array of
sba_request using devm_kcalloc() and kcalloc() cannot provide
memory beyond certain size.

This patch removes "reqs" (sba_request array) from sba_device
and makes "cmds" as variable array (instead of pointer) in
sba_request. This helps sba_prealloc_channel_resources() to
allocate sba_request and associated SBA command in one allocation
which in-turn allows arbitrary number of free sba_request.

Signed-off-by: Anup Patel <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index 59ef443..cd6e3cd 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -113,9 +113,10 @@ struct sba_request {
struct list_head next;
atomic_t next_pending_count;
/* BRCM message data */
- struct brcm_sba_command *cmds;
struct brcm_message msg;
struct dma_async_tx_descriptor tx;
+ /* SBA commands */
+ struct brcm_sba_command cmds[0];
};

enum sba_version {
@@ -153,7 +154,6 @@ struct sba_device {
void *cmds_base;
dma_addr_t cmds_dma_base;
spinlock_t reqs_lock;
- struct sba_request *reqs;
bool reqs_fence;
struct list_head reqs_alloc_list;
struct list_head reqs_pending_list;
@@ -1484,26 +1484,20 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
INIT_LIST_HEAD(&sba->reqs_aborted_list);
INIT_LIST_HEAD(&sba->reqs_free_list);

- sba->reqs = devm_kcalloc(sba->dev, sba->max_req,
- sizeof(*req), GFP_KERNEL);
- if (!sba->reqs) {
- ret = -ENOMEM;
- goto fail_free_cmds_pool;
- }
-
for (i = 0; i < sba->max_req; i++) {
- req = &sba->reqs[i];
+ req = devm_kzalloc(sba->dev,
+ sizeof(*req) +
+ sba->max_cmd_per_req * sizeof(req->cmds[0]),
+ GFP_KERNEL);
+ if (!req) {
+ ret = -ENOMEM;
+ goto fail_free_cmds_pool;
+ }
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->cmds = devm_kcalloc(sba->dev, sba->max_cmd_per_req,
- sizeof(*req->cmds), GFP_KERNEL);
- if (!req->cmds) {
- ret = -ENOMEM;
- goto fail_free_cmds_pool;
- }
for (j = 0; j < sba->max_cmd_per_req; j++) {
req->cmds[j].cmd = 0;
req->cmds[j].cmd_dma = sba->cmds_base +
--
2.7.4

2017-08-22 09:58:23

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 08/17] dmaengine: bcm-sba-raid: Increase number of free sba_request

Currently, we have only 1024 free sba_request created
by sba_prealloc_channel_resources(). This is too low
and the prep_xxx() callbacks start failing more often
at time of RAID array setup over NVMe disks.

This patch sets number of free sba_request created by
sba_prealloc_channel_resources() to be:
<number_of_mailbox_channels> x 8192

Due to above, we will have sufficient number of free
sba_request and prep_xxx() callbacks failing is very
unlikely.

Signed-off-by: Anup Patel <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index cd6e3cd..af6cc8e 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -83,6 +83,8 @@
#define SBA_CMD_WRITE_BUFFER 0xc
#define SBA_CMD_GALOIS 0xe

+#define SBA_MAX_REQ_PER_MBOX_CHANNEL 8192
+
/* Driver helper macros */
#define to_sba_request(tx) \
container_of(tx, struct sba_request, tx)
@@ -1622,6 +1624,13 @@ static int sba_probe(struct platform_device *pdev)
sba->dev = &pdev->dev;
platform_set_drvdata(pdev, sba);

+ /* Number of channels equals number of mailbox channels */
+ ret = of_count_phandle_with_args(pdev->dev.of_node,
+ "mboxes", "#mbox-cells");
+ if (ret <= 0)
+ return -ENODEV;
+ mchans_count = ret;
+
/* Determine SBA version from DT compatible string */
if (of_device_is_compatible(sba->dev->of_node, "brcm,iproc-sba"))
sba->ver = SBA_VER_1;
@@ -1634,14 +1643,12 @@ static int sba_probe(struct platform_device *pdev)
/* Derived Configuration parameters */
switch (sba->ver) {
case SBA_VER_1:
- sba->max_req = 1024;
sba->hw_buf_size = 4096;
sba->hw_resp_size = 8;
sba->max_pq_coefs = 6;
sba->max_pq_srcs = 6;
break;
case SBA_VER_2:
- sba->max_req = 1024;
sba->hw_buf_size = 4096;
sba->hw_resp_size = 8;
sba->max_pq_coefs = 30;
@@ -1655,6 +1662,7 @@ static int sba_probe(struct platform_device *pdev)
default:
return -EINVAL;
}
+ sba->max_req = SBA_MAX_REQ_PER_MBOX_CHANNEL * mchans_count;
sba->max_cmd_per_req = sba->max_pq_srcs + 3;
sba->max_xor_srcs = sba->max_cmd_per_req - 1;
sba->max_resp_pool_size = sba->max_req * sba->hw_resp_size;
@@ -1668,22 +1676,14 @@ static int sba_probe(struct platform_device *pdev)
sba->client.knows_txdone = false;
sba->client.tx_tout = 0;

- /* Number of channels equals number of mailbox channels */
- ret = of_count_phandle_with_args(pdev->dev.of_node,
- "mboxes", "#mbox-cells");
- if (ret <= 0)
- return -ENODEV;
- mchans_count = ret;
- sba->mchans_count = 0;
- atomic_set(&sba->mchans_current, 0);
-
/* Allocate mailbox channel array */
- sba->mchans = devm_kcalloc(&pdev->dev, sba->mchans_count,
+ sba->mchans = devm_kcalloc(&pdev->dev, mchans_count,
sizeof(*sba->mchans), GFP_KERNEL);
if (!sba->mchans)
return -ENOMEM;

/* Request mailbox channels */
+ sba->mchans_count = 0;
for (i = 0; i < mchans_count; i++) {
sba->mchans[i] = mbox_request_channel(&sba->client, i);
if (IS_ERR(sba->mchans[i])) {
@@ -1692,6 +1692,7 @@ static int sba_probe(struct platform_device *pdev)
}
sba->mchans_count++;
}
+ atomic_set(&sba->mchans_current, 0);

/* Find-out underlying mailbox device */
ret = of_parse_phandle_with_args(pdev->dev.of_node,
--
2.7.4

2017-08-22 09:58:30

by Anup Patel

[permalink] [raw]
Subject: [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 <[email protected]>
---
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

2017-08-22 09:58:36

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 10/17] dmaengine: bcm-sba-raid: Alloc resources before registering DMA device

We should allocate DMA channel resources before registering the
DMA device in sba_probe() because we can get DMA request soon
after registering the DMA device. If DMA channel resources are
not allocated before first DMA request then SBA-RAID driver will
crash.

Signed-off-by: Anup Patel <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index f6616da..f14ed0a 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -1478,13 +1478,13 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
int i, j, ret = 0;
struct sba_request *req = NULL;

- sba->resp_base = dma_alloc_coherent(sba->dma_dev.dev,
+ sba->resp_base = dma_alloc_coherent(sba->mbox_dev,
sba->max_resp_pool_size,
&sba->resp_dma_base, GFP_KERNEL);
if (!sba->resp_base)
return -ENOMEM;

- sba->cmds_base = dma_alloc_coherent(sba->dma_dev.dev,
+ sba->cmds_base = dma_alloc_coherent(sba->mbox_dev,
sba->max_cmds_pool_size,
&sba->cmds_dma_base, GFP_KERNEL);
if (!sba->cmds_base) {
@@ -1534,11 +1534,11 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
return 0;

fail_free_cmds_pool:
- dma_free_coherent(sba->dma_dev.dev,
+ dma_free_coherent(sba->mbox_dev,
sba->max_cmds_pool_size,
sba->cmds_base, sba->cmds_dma_base);
fail_free_resp_pool:
- dma_free_coherent(sba->dma_dev.dev,
+ dma_free_coherent(sba->mbox_dev,
sba->max_resp_pool_size,
sba->resp_base, sba->resp_dma_base);
return ret;
@@ -1547,9 +1547,9 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
static void sba_freeup_channel_resources(struct sba_device *sba)
{
dmaengine_terminate_all(&sba->dma_chan);
- dma_free_coherent(sba->dma_dev.dev, sba->max_cmds_pool_size,
+ dma_free_coherent(sba->mbox_dev, sba->max_cmds_pool_size,
sba->cmds_base, sba->cmds_dma_base);
- dma_free_coherent(sba->dma_dev.dev, sba->max_resp_pool_size,
+ dma_free_coherent(sba->mbox_dev, sba->max_resp_pool_size,
sba->resp_base, sba->resp_dma_base);
sba->resp_base = NULL;
sba->resp_dma_base = 0;
@@ -1737,15 +1737,15 @@ static int sba_probe(struct platform_device *pdev)
}
}

- /* Register DMA device with linux async framework */
- ret = sba_async_register(sba);
+ /* Prealloc channel resource */
+ ret = sba_prealloc_channel_resources(sba);
if (ret)
goto fail_free_mchans;

- /* Prealloc channel resource */
- ret = sba_prealloc_channel_resources(sba);
+ /* Register DMA device with Linux async framework */
+ ret = sba_async_register(sba);
if (ret)
- goto fail_async_dev_unreg;
+ goto fail_free_resources;

/* Print device info */
dev_info(sba->dev, "%s using SBAv%d and %d mailbox channels",
@@ -1754,8 +1754,8 @@ static int sba_probe(struct platform_device *pdev)

return 0;

-fail_async_dev_unreg:
- dma_async_device_unregister(&sba->dma_dev);
+fail_free_resources:
+ sba_freeup_channel_resources(sba);
fail_free_mchans:
for (i = 0; i < sba->mchans_count; i++)
mbox_free_channel(sba->mchans[i]);
@@ -1767,10 +1767,10 @@ static int sba_remove(struct platform_device *pdev)
int i;
struct sba_device *sba = platform_get_drvdata(pdev);

- sba_freeup_channel_resources(sba);
-
dma_async_device_unregister(&sba->dma_dev);

+ sba_freeup_channel_resources(sba);
+
for (i = 0; i < sba->mchans_count; i++)
mbox_free_channel(sba->mchans[i]);

--
2.7.4

2017-08-22 09:58:43

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 11/17] dmaengine: bcm-sba-raid: Peek mbox when we have no free requests

When setting up RAID array on several NVMe disks we observed that
sba_alloc_request() start failing (due to no free requests left)
and RAID array setup becomes very slow.

To improve performance, we do mbox channel peek when we have
no free requests. This improves performance of RAID array setup
because mbox requests that were completed but not processed by
mbox completion worker will be processed immediately by mbox
channel peek.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index f14ed0a..399250e 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -200,6 +200,14 @@ static inline u32 __pure sba_cmd_pq_c_mdata(u32 d, u32 b1, u32 b0)

/* ====== General helper routines ===== */

+static void sba_peek_mchans(struct sba_device *sba)
+{
+ int mchan_idx;
+
+ for (mchan_idx = 0; mchan_idx < sba->mchans_count; mchan_idx++)
+ mbox_client_peek_data(sba->mchans[mchan_idx]);
+}
+
static struct sba_request *sba_alloc_request(struct sba_device *sba)
{
unsigned long flags;
@@ -211,8 +219,17 @@ static struct sba_request *sba_alloc_request(struct sba_device *sba)
if (req)
list_move_tail(&req->node, &sba->reqs_alloc_list);
spin_unlock_irqrestore(&sba->reqs_lock, flags);
- if (!req)
+
+ if (!req) {
+ /*
+ * We have no more free requests so, we peek
+ * mailbox channels hoping few active requests
+ * would have completed which will create more
+ * room for new requests.
+ */
+ sba_peek_mchans(sba);
return NULL;
+ }

req->flags = SBA_REQUEST_STATE_ALLOCED;
req->first = req;
@@ -560,17 +577,15 @@ static enum dma_status sba_tx_status(struct dma_chan *dchan,
dma_cookie_t cookie,
struct dma_tx_state *txstate)
{
- int mchan_idx;
enum dma_status ret;
struct sba_device *sba = to_sba_device(dchan);

- for (mchan_idx = 0; mchan_idx < sba->mchans_count; mchan_idx++)
- mbox_client_peek_data(sba->mchans[mchan_idx]);
-
ret = dma_cookie_status(dchan, cookie, txstate);
if (ret == DMA_COMPLETE)
return ret;

+ sba_peek_mchans(sba);
+
return dma_cookie_status(dchan, cookie, txstate);
}

--
2.7.4

2017-08-22 09:58:50

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 12/17] dmaengine: bcm-sba-raid: Pre-ack async tx descriptor

We should pre-ack async tx descriptor at time of
allocating sba_request (just like other RAID drivers).

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index 399250e..2f444cc 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -237,6 +237,7 @@ static struct sba_request *sba_alloc_request(struct sba_device *sba)
atomic_set(&req->next_pending_count, 1);

dma_async_tx_descriptor_init(&req->tx, &sba->dma_chan);
+ async_tx_ack(&req->tx);

return req;
}
--
2.7.4

2017-08-22 09:58:56

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 13/17] dmaengine: bcm-sba-raid: Re-factor sba_process_deferred_requests()

Currently, sba_process_deferred_requests() handles both pending
and completed sba_request which is unnecessary overhead for
sba_issue_pending() because completed sba_request handling is
not required in sba_issue_pending().

This patch breaks sba_process_deferred_requests() into two parts
sba_process_received_request() and _sba_process_pending_requests().

The sba_issue_pending() will only process pending sba_request
by calling _sba_process_pending_requests(). This will improve
sba_issue_pending().

The sba_receive_message() will only process received sba_request
by calling sba_process_received_request() for each received
sba_request. The sba_process_received_request() will also call
_sba_process_pending_requests() after handling received sba_request
because we might have pending sba_request not submitted by previous
call to sba_issue_pending().

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 109 +++++++++++++++++++--------------------------
1 file changed, 47 insertions(+), 62 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index 2f444cc..cb6b2e2 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -419,22 +419,20 @@ static int sba_send_mbox_request(struct sba_device *sba,
return 0;
}

-static void sba_process_deferred_requests(struct sba_device *sba)
+/* Note: Must be called with sba->reqs_lock held */
+static void _sba_process_pending_requests(struct sba_device *sba)
{
int ret;
u32 count;
- unsigned long flags;
struct sba_request *req;
- struct dma_async_tx_descriptor *tx;
-
- spin_lock_irqsave(&sba->reqs_lock, flags);
-
- /* Count pending requests */
- count = 0;
- list_for_each_entry(req, &sba->reqs_pending_list, node)
- count++;

- /* Process pending requests */
+ /*
+ * Process few pending requests
+ *
+ * For now, we process (<number_of_mailbox_channels> * 8)
+ * number of requests at a time.
+ */
+ count = sba->mchans_count * 8;
while (!list_empty(&sba->reqs_pending_list) && count) {
/* Get the first pending request */
req = list_first_entry(&sba->reqs_pending_list,
@@ -445,11 +443,7 @@ static void sba_process_deferred_requests(struct sba_device *sba)
break;

/* Send request to mailbox channel */
- spin_unlock_irqrestore(&sba->reqs_lock, flags);
ret = sba_send_mbox_request(sba, req);
- spin_lock_irqsave(&sba->reqs_lock, flags);
-
- /* If something went wrong then keep request pending */
if (ret < 0) {
_sba_pending_request(sba, req);
break;
@@ -457,20 +451,18 @@ static void sba_process_deferred_requests(struct sba_device *sba)

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;
+static void sba_process_received_request(struct sba_device *sba,
+ struct sba_request *req)
+{
+ unsigned long flags;
+ struct dma_async_tx_descriptor *tx;
+ struct sba_request *nreq, *first = req->first;

- spin_unlock_irqrestore(&sba->reqs_lock, flags);
+ /* Process only after all chained requests are received */
+ if (!atomic_dec_return(&first->next_pending_count)) {
+ tx = &first->tx;

WARN_ON(tx->cookie < 0);
if (tx->cookie > 0) {
@@ -485,41 +477,34 @@ static void sba_process_deferred_requests(struct sba_device *sba)

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);
+ /* Free all requests chained to first request */
+ list_for_each_entry(nreq, &first->next, next)
+ _sba_free_request(sba, nreq);
+ INIT_LIST_HEAD(&first->next);

- count--;
- }
+ /* Mark request as received */
+ _sba_received_request(sba, first);

- /* 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);
+ /* The client is allowed to attach dependent operations
+ * until 'ack' is set
+ */
+ if (!async_tx_test_ack(tx))
+ _sba_complete_request(sba, first);
+ else
+ _sba_free_request(sba, first);

- /* Mark request as received */
- _sba_received_request(sba, req);
+ /* Cleanup completed requests */
+ list_for_each_entry_safe(req, nreq,
+ &sba->reqs_completed_list, node) {
+ if (async_tx_test_ack(&req->tx))
+ _sba_free_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);
+ /* Process pending requests */
+ _sba_process_pending_requests(sba);

- spin_unlock_irqrestore(&sba->reqs_lock, flags);
+ spin_unlock_irqrestore(&sba->reqs_lock, flags);
+ }
}

/* ====== DMAENGINE callbacks ===== */
@@ -544,10 +529,13 @@ static int sba_device_terminate_all(struct dma_chan *dchan)

static void sba_issue_pending(struct dma_chan *dchan)
{
+ unsigned long flags;
struct sba_device *sba = to_sba_device(dchan);

- /* Process deferred requests */
- sba_process_deferred_requests(sba);
+ /* Process pending requests */
+ spin_lock_irqsave(&sba->reqs_lock, flags);
+ _sba_process_pending_requests(sba);
+ spin_unlock_irqrestore(&sba->reqs_lock, flags);
}

static dma_cookie_t sba_tx_submit(struct dma_async_tx_descriptor *tx)
@@ -1482,9 +1470,6 @@ static void sba_receive_message(struct mbox_client *cl, void *msg)

/* Process received request */
sba_process_received_request(sba, req);
-
- /* Process deferred requests */
- sba_process_deferred_requests(sba);
}

/* ====== Platform driver routines ===== */
--
2.7.4

2017-08-22 09:59:03

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 14/17] dmaengine: bcm-sba-raid: Remove redundant SBA_REQUEST_STATE_RECEIVED

The SBA_REQUEST_STATE_RECEIVED state is now redundant because
received sba_request are immediately freed or moved to completed
list in sba_process_received_request().

This patch removes redundant SBA_REQUEST_STATE_RECEIVED state.

Signed-off-by: Anup Patel <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 26 ++------------------------
1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index cb6b2e2..f0a0e80 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -98,9 +98,8 @@ enum sba_request_flags {
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_COMPLETED = 0x010,
+ SBA_REQUEST_STATE_ABORTED = 0x020,
SBA_REQUEST_STATE_MASK = 0x0ff,
SBA_REQUEST_FENCE = 0x100,
};
@@ -160,7 +159,6 @@ struct sba_device {
struct list_head reqs_alloc_list;
struct list_head reqs_pending_list;
struct list_head reqs_active_list;
- struct list_head reqs_received_list;
struct list_head reqs_completed_list;
struct list_head reqs_aborted_list;
struct list_head reqs_free_list;
@@ -307,18 +305,6 @@ static void _sba_complete_request(struct sba_device *sba,
sba->reqs_fence = false;
}

-/* Note: Must be called with sba->reqs_lock held */
-static void _sba_received_request(struct sba_device *sba,
- struct sba_request *req)
-{
- 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);
- if (list_empty(&sba->reqs_active_list))
- sba->reqs_fence = false;
-}
-
static void sba_free_chained_requests(struct sba_request *req)
{
unsigned long flags;
@@ -360,10 +346,6 @@ static void sba_cleanup_nonpending_requests(struct sba_device *sba)
list_for_each_entry_safe(req, req1, &sba->reqs_alloc_list, node)
_sba_free_request(sba, req);

- /* Freeup all received request */
- list_for_each_entry_safe(req, req1, &sba->reqs_received_list, node)
- _sba_free_request(sba, req);
-
/* Freeup all completed request */
list_for_each_entry_safe(req, req1, &sba->reqs_completed_list, node)
_sba_free_request(sba, req);
@@ -482,9 +464,6 @@ static void sba_process_received_request(struct sba_device *sba,
_sba_free_request(sba, nreq);
INIT_LIST_HEAD(&first->next);

- /* Mark request as received */
- _sba_received_request(sba, first);
-
/* The client is allowed to attach dependent operations
* until 'ack' is set
*/
@@ -1498,7 +1477,6 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
INIT_LIST_HEAD(&sba->reqs_alloc_list);
INIT_LIST_HEAD(&sba->reqs_pending_list);
INIT_LIST_HEAD(&sba->reqs_active_list);
- INIT_LIST_HEAD(&sba->reqs_received_list);
INIT_LIST_HEAD(&sba->reqs_completed_list);
INIT_LIST_HEAD(&sba->reqs_aborted_list);
INIT_LIST_HEAD(&sba->reqs_free_list);
--
2.7.4

2017-08-22 09:59:10

by Anup Patel

[permalink] [raw]
Subject: [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 <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
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

2017-08-22 09:59:19

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 16/17] dmaengine: bcm-sba-raid: Explicitly ACK mailbox message after sending

We should explicitly ACK mailbox message because after
sending message we can know the send status via error
attribute of brcm_message.

This will also help SBA-RAID to use "txdone_ack" method
whenever mailbox controller supports it.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index 12310aa..0c41136 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -396,13 +396,17 @@ static int sba_send_mbox_request(struct sba_device *sba,
dev_err(sba->dev, "send message failed with error %d", ret);
return ret;
}
+
+ /* Check error returned by mailbox controller */
ret = req->msg.error;
if (ret < 0) {
dev_err(sba->dev, "message error %d", ret);
- return ret;
}

- return 0;
+ /* Signal txdone for mailbox channel */
+ mbox_client_txdone(sba->mchans[mchans_idx], ret);
+
+ return ret;
}

/* Note: Must be called with sba->reqs_lock held */
@@ -1724,7 +1728,7 @@ static int sba_probe(struct platform_device *pdev)
sba->client.dev = &pdev->dev;
sba->client.rx_callback = sba_receive_message;
sba->client.tx_block = false;
- sba->client.knows_txdone = false;
+ sba->client.knows_txdone = true;
sba->client.tx_tout = 0;

/* Allocate mailbox channel array */
--
2.7.4

2017-08-22 09:59:24

by Anup Patel

[permalink] [raw]
Subject: [PATCH v3 17/17] dmaengine: bcm-sba-raid: Remove redundant SBA_REQUEST_STATE_COMPLETED

The SBA_REQUEST_STATE_COMPLETED state was added to keep track
of sba_request which got completed but cannot be freed because
underlying Async Tx descriptor was not ACKed by DMA client.

Instead of above, we can free the sba_request with non-ACKed
Async Tx descriptor and sba_alloc_request() will ensure that
it always allocates sba_request with ACKed Async Tx descriptor.
This alternate approach makes SBA_REQUEST_STATE_COMPLETED state
redundant hence this patch removes it.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
---
drivers/dma/bcm-sba-raid.c | 63 +++++++++++++---------------------------------
1 file changed, 17 insertions(+), 46 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index 0c41136..5b1595f 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -99,8 +99,7 @@ enum sba_request_flags {
SBA_REQUEST_STATE_ALLOCED = 0x002,
SBA_REQUEST_STATE_PENDING = 0x004,
SBA_REQUEST_STATE_ACTIVE = 0x008,
- SBA_REQUEST_STATE_COMPLETED = 0x010,
- SBA_REQUEST_STATE_ABORTED = 0x020,
+ SBA_REQUEST_STATE_ABORTED = 0x010,
SBA_REQUEST_STATE_MASK = 0x0ff,
SBA_REQUEST_FENCE = 0x100,
};
@@ -160,7 +159,6 @@ struct sba_device {
struct list_head reqs_alloc_list;
struct list_head reqs_pending_list;
struct list_head reqs_active_list;
- struct list_head reqs_completed_list;
struct list_head reqs_aborted_list;
struct list_head reqs_free_list;
/* DebugFS directory entries */
@@ -212,17 +210,21 @@ static void sba_peek_mchans(struct sba_device *sba)

static struct sba_request *sba_alloc_request(struct sba_device *sba)
{
+ bool found = false;
unsigned long flags;
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);
+ list_for_each_entry(req, &sba->reqs_free_list, node) {
+ if (async_tx_test_ack(&req->tx)) {
+ list_move_tail(&req->node, &sba->reqs_alloc_list);
+ found = true;
+ break;
+ }
+ }
spin_unlock_irqrestore(&sba->reqs_lock, flags);

- if (!req) {
+ if (!found) {
/*
* We have no more free requests so, we peek
* mailbox channels hoping few active requests
@@ -297,18 +299,6 @@ static void _sba_free_request(struct sba_device *sba,
sba->reqs_fence = false;
}

-/* Note: Must be called with sba->reqs_lock held */
-static void _sba_complete_request(struct sba_device *sba,
- struct sba_request *req)
-{
- 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);
- if (list_empty(&sba->reqs_active_list))
- sba->reqs_fence = false;
-}
-
static void sba_free_chained_requests(struct sba_request *req)
{
unsigned long flags;
@@ -350,10 +340,6 @@ static void sba_cleanup_nonpending_requests(struct sba_device *sba)
list_for_each_entry_safe(req, req1, &sba->reqs_alloc_list, node)
_sba_free_request(sba, req);

- /* Freeup all completed request */
- list_for_each_entry_safe(req, req1, &sba->reqs_completed_list, node)
- _sba_free_request(sba, req);
-
/* Set all active requests as aborted */
list_for_each_entry_safe(req, req1, &sba->reqs_active_list, node)
_sba_abort_request(sba, req);
@@ -472,20 +458,8 @@ static void sba_process_received_request(struct sba_device *sba,
_sba_free_request(sba, nreq);
INIT_LIST_HEAD(&first->next);

- /* The client is allowed to attach dependent operations
- * until 'ack' is set
- */
- if (!async_tx_test_ack(tx))
- _sba_complete_request(sba, first);
- else
- _sba_free_request(sba, first);
-
- /* Cleanup completed requests */
- list_for_each_entry_safe(req, nreq,
- &sba->reqs_completed_list, node) {
- if (async_tx_test_ack(&req->tx))
- _sba_free_request(sba, req);
- }
+ /* Free the first request */
+ _sba_free_request(sba, first);

/* Process pending requests */
_sba_process_pending_requests(sba);
@@ -499,13 +473,14 @@ static void sba_write_stats_in_seqfile(struct sba_device *sba,
{
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;
+ u32 free_count = 0, alloced_count = 0;
+ u32 pending_count = 0, active_count = 0, aborted_count = 0;

spin_lock_irqsave(&sba->reqs_lock, flags);

list_for_each_entry(req, &sba->reqs_free_list, node)
- free_count++;
+ if (async_tx_test_ack(&req->tx))
+ free_count++;

list_for_each_entry(req, &sba->reqs_alloc_list, node)
alloced_count++;
@@ -519,9 +494,6 @@ static void sba_write_stats_in_seqfile(struct sba_device *sba,
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);
@@ -530,7 +502,6 @@ static void sba_write_stats_in_seqfile(struct sba_device *sba,
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 ===== */
@@ -1537,7 +1508,6 @@ static int sba_prealloc_channel_resources(struct sba_device *sba)
INIT_LIST_HEAD(&sba->reqs_alloc_list);
INIT_LIST_HEAD(&sba->reqs_pending_list);
INIT_LIST_HEAD(&sba->reqs_active_list);
- INIT_LIST_HEAD(&sba->reqs_completed_list);
INIT_LIST_HEAD(&sba->reqs_aborted_list);
INIT_LIST_HEAD(&sba->reqs_free_list);

@@ -1565,6 +1535,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);
+ async_tx_ack(&req->tx);
req->tx.tx_submit = sba_tx_submit;
req->tx.phys = sba->resp_dma_base + i * sba->hw_resp_size;
list_add_tail(&req->node, &sba->reqs_free_list);
--
2.7.4

2017-08-28 11:11:21

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v3 00/17] Broadcom SBA-RAID driver improvements

On Tue, Aug 22, 2017 at 03:26:49PM +0530, Anup Patel wrote:
> 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

Applied all, thanks

> 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/[email protected]/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
>
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
~Vinod