DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/2] cxgbe: Minor fixes in cxgbe pmd
@ 2015-11-20 13:13 Rahul Lakkireddy
  2015-11-20 13:13 ` [dpdk-dev] [PATCH 1/2] cxgbe: fix queue setup failure due to strict min desc requirement Rahul Lakkireddy
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Rahul Lakkireddy @ 2015-11-20 13:13 UTC (permalink / raw)
  To: dev; +Cc: Felix Marti, Kumar Sanghvi, Nirranjan Kirubaharan

This series of patches fix some issues and recent regressions in cxgbe pmd.

Patch 1 fixes a regression where queue setup for cxgbe pmd fails with
-EINVAL because cxgbe pmd requires a min of 1024 descriptors, but most
examples and apps initialize with 128 rx descriptors and 512 tx
descriptors.

Patch 2 replaces spinning for a lock with a better trylock in
tx alarm callback.

Rahul Lakkireddy (2):
  cxgbe: fix queue setup failure due to strict min desc requirement
  cxgbe: fix unnecessary spinning for a lock with trylock instead

 drivers/net/cxgbe/base/adapter.h |  9 +++++++++
 drivers/net/cxgbe/cxgbe.h        |  2 +-
 drivers/net/cxgbe/sge.c          | 21 ++++++++++++---------
 3 files changed, 22 insertions(+), 10 deletions(-)

-- 
2.5.3

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [dpdk-dev] [PATCH 1/2] cxgbe: fix queue setup failure due to strict min desc requirement
  2015-11-20 13:13 [dpdk-dev] [PATCH 0/2] cxgbe: Minor fixes in cxgbe pmd Rahul Lakkireddy
@ 2015-11-20 13:13 ` Rahul Lakkireddy
  2015-11-20 13:13 ` [dpdk-dev] [PATCH 2/2] cxgbe: fix unnecessary spinning for a lock with trylock instead Rahul Lakkireddy
  2015-11-24 13:49 ` [dpdk-dev] [PATCH 0/2] cxgbe: Minor fixes in cxgbe pmd Thomas Monjalon
  2 siblings, 0 replies; 4+ messages in thread
From: Rahul Lakkireddy @ 2015-11-20 13:13 UTC (permalink / raw)
  To: dev; +Cc: Felix Marti, Kumar Sanghvi, Nirranjan Kirubaharan

Most dpdk examples and apps seem to initialize with a minimum of 128 rx
descriptors and 512 tx descriptors for queue setup.  However, CXGBE PMD
enforces a minimum of 1024 descriptors.  This causes the dpdk stack
to return -EINVAL because the apps do not request the minimum required.

The fix is to relax this enforcing in CXGBE PMD. A minimum of 128
descriptors seems to be a good number with the least impact on the
performance.

Fixes: 946c9ed95616 ("cxgbe: get descriptor limits")

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
---
 drivers/net/cxgbe/cxgbe.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/cxgbe/cxgbe.h b/drivers/net/cxgbe/cxgbe.h
index adc0d92..0201c99 100644
--- a/drivers/net/cxgbe/cxgbe.h
+++ b/drivers/net/cxgbe/cxgbe.h
@@ -37,7 +37,7 @@
 #include "common.h"
 #include "t4_regs.h"
 
-#define CXGBE_MIN_RING_DESC_SIZE      1024 /* Min TX/RX descriptor ring size */
+#define CXGBE_MIN_RING_DESC_SIZE      128  /* Min TX/RX descriptor ring size */
 #define CXGBE_MAX_RING_DESC_SIZE      4096 /* Max TX/RX descriptor ring size */
 
 #define CXGBE_DEFAULT_TX_DESC_SIZE    1024 /* Default TX ring size */
-- 
2.5.3

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [dpdk-dev] [PATCH 2/2] cxgbe: fix unnecessary spinning for a lock with trylock instead
  2015-11-20 13:13 [dpdk-dev] [PATCH 0/2] cxgbe: Minor fixes in cxgbe pmd Rahul Lakkireddy
  2015-11-20 13:13 ` [dpdk-dev] [PATCH 1/2] cxgbe: fix queue setup failure due to strict min desc requirement Rahul Lakkireddy
@ 2015-11-20 13:13 ` Rahul Lakkireddy
  2015-11-24 13:49 ` [dpdk-dev] [PATCH 0/2] cxgbe: Minor fixes in cxgbe pmd Thomas Monjalon
  2 siblings, 0 replies; 4+ messages in thread
From: Rahul Lakkireddy @ 2015-11-20 13:13 UTC (permalink / raw)
  To: dev; +Cc: Felix Marti, Kumar Sanghvi, Nirranjan Kirubaharan

CXGBE PMD depends on an alarm to periodically transmit any pending
coalesced packets and hence spins for a lock for each tx queue in the
alarm callback.

A better solution is to try to get a lock whenever possible, instead
of spinning for it.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
---
 drivers/net/cxgbe/base/adapter.h |  9 +++++++++
 drivers/net/cxgbe/sge.c          | 21 ++++++++++++---------
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/net/cxgbe/base/adapter.h b/drivers/net/cxgbe/base/adapter.h
index a1e8ef7..a5225c0 100644
--- a/drivers/net/cxgbe/base/adapter.h
+++ b/drivers/net/cxgbe/base/adapter.h
@@ -473,6 +473,15 @@ static inline void t4_os_unlock(rte_spinlock_t *lock)
 }
 
 /**
+ * t4_os_trylock - try to get a lock
+ * @lock: the spinlock
+ */
+static inline int t4_os_trylock(rte_spinlock_t *lock)
+{
+	return rte_spinlock_trylock(lock);
+}
+
+/**
  * t4_os_init_list_head - initialize
  * @head: head of list to initialize [to empty]
  */
diff --git a/drivers/net/cxgbe/sge.c b/drivers/net/cxgbe/sge.c
index aa0c2e5..51449e0 100644
--- a/drivers/net/cxgbe/sge.c
+++ b/drivers/net/cxgbe/sge.c
@@ -808,20 +808,23 @@ static void tx_timer_cb(void *data)
 	struct adapter *adap = (struct adapter *)data;
 	struct sge_eth_txq *txq = &adap->sge.ethtxq[0];
 	int i;
+	unsigned int coal_idx;
 
 	/* monitor any pending tx */
 	for (i = 0; i < adap->sge.max_ethqsets; i++, txq++) {
-		t4_os_lock(&txq->txq_lock);
-		if (txq->q.coalesce.idx) {
-			if (txq->q.coalesce.idx == txq->q.last_coal_idx &&
-			    txq->q.pidx == txq->q.last_pidx) {
-				ship_tx_pkt_coalesce_wr(adap, txq);
-			} else {
-				txq->q.last_coal_idx = txq->q.coalesce.idx;
-				txq->q.last_pidx = txq->q.pidx;
+		if (t4_os_trylock(&txq->txq_lock)) {
+			coal_idx = txq->q.coalesce.idx;
+			if (coal_idx) {
+				if (coal_idx == txq->q.last_coal_idx &&
+				    txq->q.pidx == txq->q.last_pidx) {
+					ship_tx_pkt_coalesce_wr(adap, txq);
+				} else {
+					txq->q.last_coal_idx = coal_idx;
+					txq->q.last_pidx = txq->q.pidx;
+				}
 			}
+			t4_os_unlock(&txq->txq_lock);
 		}
-		t4_os_unlock(&txq->txq_lock);
 	}
 	rte_eal_alarm_set(50, tx_timer_cb, (void *)adap);
 }
-- 
2.5.3

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH 0/2] cxgbe: Minor fixes in cxgbe pmd
  2015-11-20 13:13 [dpdk-dev] [PATCH 0/2] cxgbe: Minor fixes in cxgbe pmd Rahul Lakkireddy
  2015-11-20 13:13 ` [dpdk-dev] [PATCH 1/2] cxgbe: fix queue setup failure due to strict min desc requirement Rahul Lakkireddy
  2015-11-20 13:13 ` [dpdk-dev] [PATCH 2/2] cxgbe: fix unnecessary spinning for a lock with trylock instead Rahul Lakkireddy
@ 2015-11-24 13:49 ` Thomas Monjalon
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2015-11-24 13:49 UTC (permalink / raw)
  To: Rahul Lakkireddy; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi

2015-11-20 18:43, Rahul Lakkireddy:
> This series of patches fix some issues and recent regressions in cxgbe pmd.
> 
> Patch 1 fixes a regression where queue setup for cxgbe pmd fails with
> -EINVAL because cxgbe pmd requires a min of 1024 descriptors, but most
> examples and apps initialize with 128 rx descriptors and 512 tx
> descriptors.
> 
> Patch 2 replaces spinning for a lock with a better trylock in
> tx alarm callback.
> 
> Rahul Lakkireddy (2):
>   cxgbe: fix queue setup failure due to strict min desc requirement
>   cxgbe: fix unnecessary spinning for a lock with trylock instead

Applied, thanks

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-11-24 13:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-20 13:13 [dpdk-dev] [PATCH 0/2] cxgbe: Minor fixes in cxgbe pmd Rahul Lakkireddy
2015-11-20 13:13 ` [dpdk-dev] [PATCH 1/2] cxgbe: fix queue setup failure due to strict min desc requirement Rahul Lakkireddy
2015-11-20 13:13 ` [dpdk-dev] [PATCH 2/2] cxgbe: fix unnecessary spinning for a lock with trylock instead Rahul Lakkireddy
2015-11-24 13:49 ` [dpdk-dev] [PATCH 0/2] cxgbe: Minor fixes in cxgbe pmd Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).