[PATCH] NTB: allocate number transport entries depending on size of ring size

10 views
Skip to first unread message

Dave Jiang

unread,
Apr 6, 2016, 5:26:18 PM4/6/16
to allen...@emc.com, jdm...@kudzu.us, linu...@googlegroups.com
Currently we only allocate a fixed default number of descriptors for the tx
and rx side. We should dynamically resize it to the number of descriptors
resides in the transport rings. We should know the number of transmit
descriptors at initializaiton. We will allocate the default number of
descriptors for receive side and allocate additional ones when we know the
actual max entries for receive.

Signed-off-by: Dave Jiang <dave....@intel.com>
---
drivers/ntb/ntb_transport.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 2ef9d913..4f7006a 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -597,9 +597,13 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
{
struct ntb_transport_qp *qp = &nt->qp_vec[qp_num];
struct ntb_transport_mw *mw;
+ struct ntb_dev *ndev = nt->ndev;
+ struct ntb_queue_entry *entry;
unsigned int rx_size, num_qps_mw;
unsigned int mw_num, mw_count, qp_count;
unsigned int i;
+ unsigned int rx_entries = 0;
+ int node;

mw_count = nt->mw_count;
qp_count = nt->qp_count;
@@ -626,6 +630,20 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
qp->rx_max_entry = rx_size / qp->rx_max_frame;
qp->rx_index = 0;

+ if (qp->rx_max_entry > NTB_QP_DEF_NUM_ENTRIES)
+ rx_entries = qp->rx_max_entry - NTB_QP_DEF_NUM_ENTRIES;
+
+ node = dev_to_node(&ndev->dev);
+ for (i = 0; i < rx_entries; i++) {
+ entry = kzalloc_node(sizeof(*entry), GFP_ATOMIC, node);
+ if (!entry)
+ return -ENOMEM;
+
+ entry->qp = qp;
+ ntb_list_add(&qp->ntb_rx_q_lock, &entry->entry,
+ &qp->rx_free_q);
+ }
+
qp->remote_rx_info->entry = qp->rx_max_entry - 1;

/* setup the hdr offsets with 0's */
@@ -1723,7 +1741,7 @@ ntb_transport_create_queue(void *data, struct device *client_dev,
&qp->rx_free_q);
}

- for (i = 0; i < NTB_QP_DEF_NUM_ENTRIES; i++) {
+ for (i = 0; i < qp->tx_max_entry; i++) {
entry = kzalloc_node(sizeof(*entry), GFP_ATOMIC, node);
if (!entry)
goto err2;

Jon Mason

unread,
Apr 7, 2016, 12:41:01 PM4/7/16
to Dave Jiang, Hubbe, Allen, linu...@googlegroups.com
I would be good to have a comment here describing what/why this is
being done here. After thinking about it, I understood why, but who
wants to think when they can read a comment :)

Aside from this, it looks good to me.

Thanks,
Jon

Dave Jiang

unread,
Apr 7, 2016, 2:01:01 PM4/7/16
to allen...@emc.com, jdm...@kudzu.us, linu...@googlegroups.com
Currently we only allocate a fixed default number of descriptors for the tx
and rx side. We should dynamically resize it to the number of descriptors
resides in the transport rings. We should know the number of transmit
descriptors at initializaiton. We will allocate the default number of
descriptors for receive side and allocate additional ones when we know the
actual max entries for receive.

Signed-off-by: Dave Jiang <dave....@intel.com>
---
drivers/ntb/ntb_transport.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 2ef9d913..a943bb1 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -597,9 +597,13 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
{
struct ntb_transport_qp *qp = &nt->qp_vec[qp_num];
struct ntb_transport_mw *mw;
+ struct ntb_dev *ndev = nt->ndev;
+ struct ntb_queue_entry *entry;
unsigned int rx_size, num_qps_mw;
unsigned int mw_num, mw_count, qp_count;
unsigned int i;
+ unsigned int rx_entries = 0;
+ int node;

mw_count = nt->mw_count;
qp_count = nt->qp_count;
@@ -626,6 +630,25 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
qp->rx_max_entry = rx_size / qp->rx_max_frame;
qp->rx_index = 0;

+ /*
+ * Checking to see if we have more entries than the default.
+ * We should add additional entries if that is the case so we
+ * can be in sync with the transport frames.
+ */
+ if (qp->rx_max_entry > NTB_QP_DEF_NUM_ENTRIES)
+ rx_entries = qp->rx_max_entry - NTB_QP_DEF_NUM_ENTRIES;
+
+ node = dev_to_node(&ndev->dev);
+ for (i = 0; i < rx_entries; i++) {
+ entry = kzalloc_node(sizeof(*entry), GFP_ATOMIC, node);
+ if (!entry)
+ return -ENOMEM;
+
+ entry->qp = qp;
+ ntb_list_add(&qp->ntb_rx_q_lock, &entry->entry,
+ &qp->rx_free_q);
+ }
+
qp->remote_rx_info->entry = qp->rx_max_entry - 1;

/* setup the hdr offsets with 0's */
@@ -1723,7 +1746,7 @@ ntb_transport_create_queue(void *data, struct device *client_dev,

Allen Hubbe

unread,
Apr 7, 2016, 2:43:26 PM4/7/16
to Dave Jiang, jdm...@kudzu.us, linu...@googlegroups.com
> From: Dave Jiang
If the link drops and then returns, it looks like this will we allocate
another (rx_max_entries - DEFAULT) each time we establish the link.
Do we have an actual count of the number of entries that we could use
here, instead of NTB_QP_DEF_NUM_ENTRIES?

If we don't already track the actual number of entries allocated,
different from the maximum number of entries that fit in the ring
buffer, maybe we should add it. Something like qp->rx_num_entry?

It is possible, if the memory window is small or mtu is large, we have
allocated more entries than max. The term max is deceptive, oh well.

> +
> + node = dev_to_node(&ndev->dev);
> + for (i = 0; i < rx_entries; i++) {

Instead of:
if (num_entry < max_entry)
calc the difference;
for (i = 0 .. the difference) {
/* do alloc */
}

What about:

for (i = num_entry; i < max_entry; ++i) {
/* do alloc */
}
current_entry = max_entry;

or

while (current_entry < max_entry) {
/* do alloc */
++current_entry;

Dave Jiang

unread,
Apr 8, 2016, 1:49:19 PM4/8/16
to allen...@emc.com, jdm...@kudzu.us, linu...@googlegroups.com
Currently we only allocate a fixed default number of descriptors for the tx
and rx side. We should dynamically resize it to the number of descriptors
resides in the transport rings. We should know the number of transmit
descriptors at initializaiton. We will allocate the default number of
descriptors for receive side and allocate additional ones when we know the
actual max entries for receive.

Signed-off-by: Dave Jiang <dave....@intel.com>
---
drivers/ntb/ntb_transport.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 2ef9d913..6db8c85 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -153,6 +153,7 @@ struct ntb_transport_qp {
unsigned int rx_index;
unsigned int rx_max_entry;
unsigned int rx_max_frame;
+ unsigned int rx_alloc_entry;
dma_cookie_t last_cookie;
struct tasklet_struct rxc_db_work;

@@ -480,7 +481,9 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
out_offset += snprintf(buf + out_offset, out_count - out_offset,
"rx_index - \t%u\n", qp->rx_index);
out_offset += snprintf(buf + out_offset, out_count - out_offset,
- "rx_max_entry - \t%u\n\n", qp->rx_max_entry);
+ "rx_max_entry - \t%u\n", qp->rx_max_entry);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "rx_alloc_entry - \t%u\n\n", qp->rx_alloc_entry);

out_offset += snprintf(buf + out_offset, out_count - out_offset,
"tx_bytes - \t%llu\n", qp->tx_bytes);
@@ -597,9 +600,12 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
{
struct ntb_transport_qp *qp = &nt->qp_vec[qp_num];
struct ntb_transport_mw *mw;
+ struct ntb_dev *ndev = nt->ndev;
+ struct ntb_queue_entry *entry;
unsigned int rx_size, num_qps_mw;
unsigned int mw_num, mw_count, qp_count;
unsigned int i;
+ int node;

mw_count = nt->mw_count;
qp_count = nt->qp_count;
@@ -626,6 +632,23 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
qp->rx_max_entry = rx_size / qp->rx_max_frame;
qp->rx_index = 0;

+ /*
+ * Checking to see if we have more entries than the default.
+ * We should add additional entries if that is the case so we
+ * can be in sync with the transport frames.
+ */
+ node = dev_to_node(&ndev->dev);
+ for (i = qp->rx_alloc_entry; i < qp->rx_max_entry; i++) {
+ entry = kzalloc_node(sizeof(*entry), GFP_ATOMIC, node);
+ if (!entry)
+ return -ENOMEM;
+
+ entry->qp = qp;
+ ntb_list_add(&qp->ntb_rx_q_lock, &entry->entry,
+ &qp->rx_free_q);
+ qp->rx_alloc_entry++;
+ }
+
qp->remote_rx_info->entry = qp->rx_max_entry - 1;

/* setup the hdr offsets with 0's */
@@ -1722,8 +1745,9 @@ ntb_transport_create_queue(void *data, struct device *client_dev,
ntb_list_add(&qp->ntb_rx_q_lock, &entry->entry,
&qp->rx_free_q);
}
+ qp->rx_alloc_entry = NTB_QP_DEF_NUM_ENTRIES;

- for (i = 0; i < NTB_QP_DEF_NUM_ENTRIES; i++) {
+ for (i = 0; i < qp->tx_max_entry; i++) {
entry = kzalloc_node(sizeof(*entry), GFP_ATOMIC, node);
if (!entry)
goto err2;
@@ -1744,6 +1768,7 @@ err2:
while ((entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
kfree(entry);
err1:
+ qp->rx_alloc_entry = 0;
while ((entry = ntb_list_rm(&qp->ntb_rx_q_lock, &qp->rx_free_q)))
kfree(entry);
if (qp->tx_dma_chan)

Allen Hubbe

unread,
Apr 8, 2016, 1:57:29 PM4/8/16
to Dave Jiang, jdm...@kudzu.us, linu...@googlegroups.com
> From: Dave Jiang
>
> Currently we only allocate a fixed default number of descriptors for the tx
> and rx side. We should dynamically resize it to the number of descriptors
> resides in the transport rings. We should know the number of transmit
> descriptors at initializaiton. We will allocate the default number of
> descriptors for receive side and allocate additional ones when we know the
> actual max entries for receive.
>
> Signed-off-by: Dave Jiang <dave....@intel.com>

Acked-by: Allen Hubbe <allen...@emc.com>

Jon Mason

unread,
Apr 21, 2016, 10:54:31 AM4/21/16
to Allen Hubbe, Dave Jiang, linu...@googlegroups.com
On Fri, Apr 8, 2016 at 1:57 PM, Allen Hubbe <Allen...@emc.com> wrote:
>> From: Dave Jiang
>>
>> Currently we only allocate a fixed default number of descriptors for the tx
>> and rx side. We should dynamically resize it to the number of descriptors
>> resides in the transport rings. We should know the number of transmit
>> descriptors at initializaiton. We will allocate the default number of
>> descriptors for receive side and allocate additional ones when we know the
>> actual max entries for receive.
>>
>> Signed-off-by: Dave Jiang <dave....@intel.com>
>
> Acked-by: Allen Hubbe <allen...@emc.com>

Applied to my ntb-next git branch
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+...@googlegroups.com.
> To post to this group, send email to linu...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/000001d191c0%240ea3cfe0%242beb6fa0%24%40emc.com.
> For more options, visit https://groups.google.com/d/optout.
Reply all
Reply to author
Forward
0 new messages