* [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround
@ 2015-09-25 19:00 Anjali Singhai Jain
2015-09-25 19:00 ` [Intel-wired-lan] [PATCH 2/2] i40e: Disable Check for Hang Anjali Singhai Jain
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Anjali Singhai Jain @ 2015-09-25 19:00 UTC (permalink / raw)
To: intel-wired-lan
This patch fixes the issue of forcing WB too often causing us to not
benefit from NAPI.
Without this patch we were forcing WB/arming interrupt too often taking
away the benefits of NAPI and causing a performance impact.
With this patch we disable force WB in the clean routine for X710
and XL710 adapters. X722 adapters do not enable interrupt to force
a WB and benefit from WB_ON_ITR and hence force WB is left enabled
for those adapters.
For XL710 and X710 adapters if we have less than 4 packets pending
a software Interrupt triggered from service task will force a WB.
This patch also changes the conditions for setting RS bit as described
in code comments. This optimizes when the HW does a tail bump amd when
it does a WB. It also optimizes when we do a wmb.
Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 126 ++++++++++++++++++----------
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 +
2 files changed, 86 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index d51b8ed..f75db56 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -785,15 +785,21 @@ static bool i40e_clean_tx_irq(struct i40e_ring *tx_ring, int budget)
tx_ring->q_vector->tx.total_bytes += total_bytes;
tx_ring->q_vector->tx.total_packets += total_packets;
- /* check to see if there are any non-cache aligned descriptors
- * waiting to be written back, and kick the hardware to force
- * them to be written back in case of napi polling
- */
- if (budget &&
- !((i & WB_STRIDE) == WB_STRIDE) &&
- !test_bit(__I40E_DOWN, &tx_ring->vsi->state) &&
- (I40E_DESC_UNUSED(tx_ring) != tx_ring->count))
- tx_ring->arm_wb = true;
+ if (tx_ring->flags & I40E_TXR_FLAGS_WB_ON_ITR) {
+ unsigned int j = 0;
+ /* check to see if there are < 4 descriptors
+ * waiting to be written back, then kick the hardware to force
+ * them to be written back in case we stay in NAPI.
+ * In this mode on X722 we do not enable Interrupt.
+ */
+ j = i40e_get_tx_pending(tx_ring);
+
+ if (budget &&
+ ((j / (WB_STRIDE + 1)) == 0) && (j != 0) &&
+ !test_bit(__I40E_DOWN, &tx_ring->vsi->state) &&
+ (I40E_DESC_UNUSED(tx_ring) != tx_ring->count))
+ tx_ring->arm_wb = true;
+ }
if (check_for_tx_hang(tx_ring) && i40e_check_tx_hang(tx_ring)) {
/* schedule immediate reset if we believe we hung */
@@ -2582,6 +2588,9 @@ static inline void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
u32 td_tag = 0;
dma_addr_t dma;
u16 gso_segs;
+ u16 desc_count = 0;
+ bool tail_bump = true;
+ bool do_rs = false;
if (tx_flags & I40E_TX_FLAGS_HW_VLAN) {
td_cmd |= I40E_TX_DESC_CMD_IL2TAG1;
@@ -2622,6 +2631,8 @@ static inline void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
tx_desc++;
i++;
+ desc_count++;
+
if (i == tx_ring->count) {
tx_desc = I40E_TX_DESC(tx_ring, 0);
i = 0;
@@ -2641,6 +2652,8 @@ static inline void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
tx_desc++;
i++;
+ desc_count++;
+
if (i == tx_ring->count) {
tx_desc = I40E_TX_DESC(tx_ring, 0);
i = 0;
@@ -2655,34 +2668,6 @@ static inline void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
tx_bi = &tx_ring->tx_bi[i];
}
- /* Place RS bit on last descriptor of any packet that spans across the
- * 4th descriptor (WB_STRIDE aka 0x3) in a 64B cacheline.
- */
- if (((i & WB_STRIDE) != WB_STRIDE) &&
- (first <= &tx_ring->tx_bi[i]) &&
- (first >= &tx_ring->tx_bi[i & ~WB_STRIDE])) {
- tx_desc->cmd_type_offset_bsz =
- build_ctob(td_cmd, td_offset, size, td_tag) |
- cpu_to_le64((u64)I40E_TX_DESC_CMD_EOP <<
- I40E_TXD_QW1_CMD_SHIFT);
- } else {
- tx_desc->cmd_type_offset_bsz =
- build_ctob(td_cmd, td_offset, size, td_tag) |
- cpu_to_le64((u64)I40E_TXD_CMD <<
- I40E_TXD_QW1_CMD_SHIFT);
- }
-
- netdev_tx_sent_queue(netdev_get_tx_queue(tx_ring->netdev,
- tx_ring->queue_index),
- first->bytecount);
-
- /* Force memory writes to complete before letting h/w
- * know there are new descriptors to fetch. (Only
- * applicable for weak-ordered memory model archs,
- * such as IA-64).
- */
- wmb();
-
/* set next_to_watch value indicating a packet is present */
first->next_to_watch = tx_desc;
@@ -2692,15 +2677,72 @@ static inline void i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
tx_ring->next_to_use = i;
+ netdev_tx_sent_queue(netdev_get_tx_queue(tx_ring->netdev,
+ tx_ring->queue_index),
+ first->bytecount);
i40e_maybe_stop_tx(tx_ring, DESC_NEEDED);
+
+ /* Algorithm to optimize tail and RS bit setting:
+ * if xmit_more is supported
+ * if xmit_more is true
+ * do not update tail and do not mark RS bit.
+ * if xmit_more is false and last xmit_more was false
+ * if every packet spanned less than 4 desc
+ * then set RS bit on 4th packet and update tail
+ * on every packet
+ * else
+ * update tail and set RS bit on every packet.
+ * if xmit_more is false and last_xmit_more was true
+ * update tail and set RS bit.
+ *
+ * Optimization: wmb to be issued only in case of tail update.
+ * Also optimize the Descriptor WB path for RS bit with the same
+ * algorithm.
+ *
+ * Note: If there are less than 4 packets
+ * pending and interrupts were disabled the service task will
+ * trigger a force WB.
+ */
+ if (skb->xmit_more &&
+ !netif_xmit_stopped(netdev_get_tx_queue(tx_ring->netdev,
+ tx_ring->queue_index))) {
+ tx_ring->flags |= I40E_TXR_FLAGS_LAST_XMIT_MORE_SET;
+ tail_bump = false;
+ } else if (!skb->xmit_more &&
+ !netif_xmit_stopped(netdev_get_tx_queue(tx_ring->netdev,
+ tx_ring->queue_index)) &&
+ (!(tx_ring->flags & I40E_TXR_FLAGS_LAST_XMIT_MORE_SET)) &&
+ (tx_ring->packet_stride < WB_STRIDE) &&
+ (desc_count < WB_STRIDE)) {
+ tx_ring->packet_stride++;
+ } else {
+ tx_ring->packet_stride = 0;
+ tx_ring->flags &= ~I40E_TXR_FLAGS_LAST_XMIT_MORE_SET;
+ do_rs = true;
+ }
+ if (do_rs)
+ tx_ring->packet_stride = 0;
+
+ tx_desc->cmd_type_offset_bsz =
+ build_ctob(td_cmd, td_offset, size, td_tag) |
+ cpu_to_le64((u64)(do_rs ? I40E_TXD_CMD :
+ I40E_TX_DESC_CMD_EOP) <<
+ I40E_TXD_QW1_CMD_SHIFT);
+
/* notify HW of packet */
- if (!skb->xmit_more ||
- netif_xmit_stopped(netdev_get_tx_queue(tx_ring->netdev,
- tx_ring->queue_index)))
- writel(i, tx_ring->tail);
- else
+ if (!tail_bump)
prefetchw(tx_desc + 1);
+ if (tail_bump) {
+ /* Force memory writes to complete before letting h/w
+ * know there are new descriptors to fetch. (Only
+ * applicable for weak-ordered memory model archs,
+ * such as IA-64).
+ */
+ wmb();
+ writel(i, tx_ring->tail);
+ }
+
return;
dma_error:
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index 073464e..4871809 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -266,10 +266,12 @@ struct i40e_ring {
bool ring_active; /* is ring online or not */
bool arm_wb; /* do something to arm write back */
+ u8 packet_stride;
u16 flags;
#define I40E_TXR_FLAGS_WB_ON_ITR BIT(0)
#define I40E_TXR_FLAGS_OUTER_UDP_CSUM BIT(1)
+#define I40E_TXR_FLAGS_LAST_XMIT_MORE_SET BIT(2)
/* stats structs */
struct i40e_queue_stats stats;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH 2/2] i40e: Disable Check for Hang
2015-09-25 19:00 [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround Anjali Singhai Jain
@ 2015-09-25 19:00 ` Anjali Singhai Jain
2015-09-25 23:16 ` Jeff Kirsher
2015-09-25 19:50 ` [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround Singhai, Anjali
2015-09-25 23:18 ` Jeff Kirsher
2 siblings, 1 reply; 6+ messages in thread
From: Anjali Singhai Jain @ 2015-09-25 19:00 UTC (permalink / raw)
To: intel-wired-lan
The driver doesn't need its own check for hang logic as there are no
known issues that cause HW Tx Hangs. This logic was actually
(mis)firing frequently, causing us to reset when we didn't need to.
If a true tx hang happens, just wait for the stack logic to catch the
issue and send us a tx_timeout.
Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 99 +----------------------------
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 70 --------------------
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 8 ---
3 files changed, 1 insertion(+), 176 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index f048002..7b92d1d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -758,7 +758,6 @@ static void i40e_update_link_xoff_rx(struct i40e_pf *pf)
struct i40e_hw_port_stats *nsd = &pf->stats;
struct i40e_hw *hw = &pf->hw;
u64 xoff = 0;
- u16 i, v;
if ((hw->fc.current_mode != I40E_FC_FULL) &&
(hw->fc.current_mode != I40E_FC_RX_PAUSE))
@@ -772,20 +771,6 @@ static void i40e_update_link_xoff_rx(struct i40e_pf *pf)
/* No new LFC xoff rx */
if (!(nsd->link_xoff_rx - xoff))
return;
-
- /* Clear the __I40E_HANG_CHECK_ARMED bit for all Tx rings */
- for (v = 0; v < pf->num_alloc_vsi; v++) {
- struct i40e_vsi *vsi = pf->vsi[v];
-
- if (!vsi || !vsi->tx_rings[0])
- continue;
-
- for (i = 0; i < vsi->num_queue_pairs; i++) {
- struct i40e_ring *ring = vsi->tx_rings[i];
-
- clear_bit(__I40E_HANG_CHECK_ARMED, &ring->state);
- }
- }
}
/**
@@ -801,7 +786,7 @@ static void i40e_update_prio_xoff_rx(struct i40e_pf *pf)
bool xoff[I40E_MAX_TRAFFIC_CLASS] = {false};
struct i40e_dcbx_config *dcb_cfg;
struct i40e_hw *hw = &pf->hw;
- u16 i, v;
+ u16 i;
u8 tc;
dcb_cfg = &hw->local_dcbx_config;
@@ -827,23 +812,6 @@ static void i40e_update_prio_xoff_rx(struct i40e_pf *pf)
tc = dcb_cfg->etscfg.prioritytable[i];
xoff[tc] = true;
}
-
- /* Clear the __I40E_HANG_CHECK_ARMED bit for Tx rings */
- for (v = 0; v < pf->num_alloc_vsi; v++) {
- struct i40e_vsi *vsi = pf->vsi[v];
-
- if (!vsi || !vsi->tx_rings[0])
- continue;
-
- for (i = 0; i < vsi->num_queue_pairs; i++) {
- struct i40e_ring *ring = vsi->tx_rings[i];
-
- tc = ring->dcb_tc;
- if (xoff[tc])
- clear_bit(__I40E_HANG_CHECK_ARMED,
- &ring->state);
- }
- }
}
/**
@@ -2803,8 +2771,6 @@ static int i40e_configure_tx_ring(struct i40e_ring *ring)
wr32(hw, I40E_QTX_CTL(pf_q), qtx_ctl);
i40e_flush(hw);
- clear_bit(__I40E_HANG_CHECK_ARMED, &ring->state);
-
/* cache tail off for easier writes later */
ring->tail = hw->hw_addr + I40E_QTX_TAIL(pf_q);
@@ -5949,68 +5915,6 @@ static void i40e_link_event(struct i40e_pf *pf)
}
/**
- * i40e_check_hang_subtask - Check for hung queues and dropped interrupts
- * @pf: board private structure
- *
- * Set the per-queue flags to request a check for stuck queues in the irq
- * clean functions, then force interrupts to be sure the irq clean is called.
- **/
-static void i40e_check_hang_subtask(struct i40e_pf *pf)
-{
- int i, v;
-
- /* If we're down or resetting, just bail */
- if (test_bit(__I40E_DOWN, &pf->state) ||
- test_bit(__I40E_CONFIG_BUSY, &pf->state))
- return;
-
- /* for each VSI/netdev
- * for each Tx queue
- * set the check flag
- * for each q_vector
- * force an interrupt
- */
- for (v = 0; v < pf->num_alloc_vsi; v++) {
- struct i40e_vsi *vsi = pf->vsi[v];
- int armed = 0;
-
- if (!pf->vsi[v] ||
- test_bit(__I40E_DOWN, &vsi->state) ||
- (vsi->netdev && !netif_carrier_ok(vsi->netdev)))
- continue;
-
- for (i = 0; i < vsi->num_queue_pairs; i++) {
- set_check_for_tx_hang(vsi->tx_rings[i]);
- if (test_bit(__I40E_HANG_CHECK_ARMED,
- &vsi->tx_rings[i]->state))
- armed++;
- }
-
- if (armed) {
- if (!(pf->flags & I40E_FLAG_MSIX_ENABLED)) {
- wr32(&vsi->back->hw, I40E_PFINT_DYN_CTL0,
- (I40E_PFINT_DYN_CTL0_INTENA_MASK |
- I40E_PFINT_DYN_CTL0_SWINT_TRIG_MASK |
- I40E_PFINT_DYN_CTL0_ITR_INDX_MASK |
- I40E_PFINT_DYN_CTL0_SW_ITR_INDX_ENA_MASK |
- I40E_PFINT_DYN_CTL0_SW_ITR_INDX_MASK));
- } else {
- u16 vec = vsi->base_vector - 1;
- u32 val = (I40E_PFINT_DYN_CTLN_INTENA_MASK |
- I40E_PFINT_DYN_CTLN_SWINT_TRIG_MASK |
- I40E_PFINT_DYN_CTLN_ITR_INDX_MASK |
- I40E_PFINT_DYN_CTLN_SW_ITR_INDX_ENA_MASK |
- I40E_PFINT_DYN_CTLN_SW_ITR_INDX_MASK);
- for (i = 0; i < vsi->num_q_vectors; i++, vec++)
- wr32(&vsi->back->hw,
- I40E_PFINT_DYN_CTLN(vec), val);
- }
- i40e_flush(&vsi->back->hw);
- }
- }
-}
-
-/**
* i40e_watchdog_subtask - periodic checks not using event driven response
* @pf: board private structure
**/
@@ -6029,7 +5933,6 @@ static void i40e_watchdog_subtask(struct i40e_pf *pf)
return;
pf->service_timer_previous = jiffies;
- i40e_check_hang_subtask(pf);
if (pf->flags & I40E_FLAG_LINK_POLLING_ENABLED)
i40e_link_event(pf);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index f75db56..a30e3a1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -636,50 +636,6 @@ static u32 i40e_get_tx_pending(struct i40e_ring *ring)
return 0;
}
-/**
- * i40e_check_tx_hang - Is there a hang in the Tx queue
- * @tx_ring: the ring of descriptors
- **/
-static bool i40e_check_tx_hang(struct i40e_ring *tx_ring)
-{
- u32 tx_done = tx_ring->stats.packets;
- u32 tx_done_old = tx_ring->tx_stats.tx_done_old;
- u32 tx_pending = i40e_get_tx_pending(tx_ring);
- struct i40e_pf *pf = tx_ring->vsi->back;
- bool ret = false;
-
- clear_check_for_tx_hang(tx_ring);
-
- /* Check for a hung queue, but be thorough. This verifies
- * that a transmit has been completed since the previous
- * check AND there is at least one packet pending. The
- * ARMED bit is set to indicate a potential hang. The
- * bit is cleared if a pause frame is received to remove
- * false hang detection due to PFC or 802.3x frames. By
- * requiring this to fail twice we avoid races with
- * PFC clearing the ARMED bit and conditions where we
- * run the check_tx_hang logic with a transmit completion
- * pending but without time to complete it yet.
- */
- if ((tx_done_old == tx_done) && tx_pending) {
- /* make sure it is true for two checks in a row */
- ret = test_and_set_bit(__I40E_HANG_CHECK_ARMED,
- &tx_ring->state);
- } else if (tx_done_old == tx_done &&
- (tx_pending < I40E_MIN_DESC_PENDING) && (tx_pending > 0)) {
- if (I40E_DEBUG_FLOW & pf->hw.debug_mask)
- dev_info(tx_ring->dev, "HW needs some more descs to do a cacheline flush. tx_pending %d, queue %d",
- tx_pending, tx_ring->queue_index);
- pf->tx_sluggish_count++;
- } else {
- /* update completed stats and disarm the hang check */
- tx_ring->tx_stats.tx_done_old = tx_done;
- clear_bit(__I40E_HANG_CHECK_ARMED, &tx_ring->state);
- }
-
- return ret;
-}
-
#define WB_STRIDE 0x3
/**
@@ -801,32 +757,6 @@ static bool i40e_clean_tx_irq(struct i40e_ring *tx_ring, int budget)
tx_ring->arm_wb = true;
}
- if (check_for_tx_hang(tx_ring) && i40e_check_tx_hang(tx_ring)) {
- /* schedule immediate reset if we believe we hung */
- dev_info(tx_ring->dev, "Detected Tx Unit Hang\n"
- " VSI <%d>\n"
- " Tx Queue <%d>\n"
- " next_to_use <%x>\n"
- " next_to_clean <%x>\n",
- tx_ring->vsi->seid,
- tx_ring->queue_index,
- tx_ring->next_to_use, i);
-
- netif_stop_subqueue(tx_ring->netdev, tx_ring->queue_index);
-
- dev_info(tx_ring->dev,
- "tx hang detected on queue %d, reset requested\n",
- tx_ring->queue_index);
-
- /* do not fire the reset immediately, wait for the stack to
- * decide we are truly stuck, also prevents every queue from
- * simultaneously requesting a reset
- */
-
- /* the adapter is about to reset, no point in enabling polling */
- budget = 1;
- }
-
netdev_tx_completed_queue(netdev_get_tx_queue(tx_ring->netdev,
tx_ring->queue_index),
total_packets, total_bytes);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index 4871809..809e170 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -201,8 +201,6 @@ struct i40e_rx_queue_stats {
enum i40e_ring_state_t {
__I40E_TX_FDIR_INIT_DONE,
__I40E_TX_XPS_INIT_DONE,
- __I40E_TX_DETECT_HANG,
- __I40E_HANG_CHECK_ARMED,
__I40E_RX_PS_ENABLED,
__I40E_RX_16BYTE_DESC_ENABLED,
};
@@ -213,12 +211,6 @@ enum i40e_ring_state_t {
set_bit(__I40E_RX_PS_ENABLED, &(ring)->state)
#define clear_ring_ps_enabled(ring) \
clear_bit(__I40E_RX_PS_ENABLED, &(ring)->state)
-#define check_for_tx_hang(ring) \
- test_bit(__I40E_TX_DETECT_HANG, &(ring)->state)
-#define set_check_for_tx_hang(ring) \
- set_bit(__I40E_TX_DETECT_HANG, &(ring)->state)
-#define clear_check_for_tx_hang(ring) \
- clear_bit(__I40E_TX_DETECT_HANG, &(ring)->state)
#define ring_is_16byte_desc_enabled(ring) \
test_bit(__I40E_RX_16BYTE_DESC_ENABLED, &(ring)->state)
#define set_ring_16byte_desc_enabled(ring) \
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround
2015-09-25 19:00 [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround Anjali Singhai Jain
2015-09-25 19:00 ` [Intel-wired-lan] [PATCH 2/2] i40e: Disable Check for Hang Anjali Singhai Jain
@ 2015-09-25 19:50 ` Singhai, Anjali
2015-09-25 23:18 ` Jeff Kirsher
2 siblings, 0 replies; 6+ messages in thread
From: Singhai, Anjali @ 2015-09-25 19:50 UTC (permalink / raw)
To: intel-wired-lan
The other patches needed for this patch to fix a tx_timeout issue are
https://patchwork.ozlabs.org/patch/522598/
https://patchwork.ozlabs.org/patch/522954/
Anjali
> -----Original Message-----
> From: Singhai, Anjali
> Sent: Friday, September 25, 2015 12:00 PM
> To: intel-wired-lan at lists.osuosl.org
> Cc: Singhai, Anjali
> Subject: [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB
> workaround
>
> This patch fixes the issue of forcing WB too often causing us to not benefit
> from NAPI.
>
> Without this patch we were forcing WB/arming interrupt too often taking
> away the benefits of NAPI and causing a performance impact.
>
> With this patch we disable force WB in the clean routine for X710 and XL710
> adapters. X722 adapters do not enable interrupt to force a WB and benefit
> from WB_ON_ITR and hence force WB is left enabled for those adapters.
> For XL710 and X710 adapters if we have less than 4 packets pending a
> software Interrupt triggered from service task will force a WB.
>
> This patch also changes the conditions for setting RS bit as described in code
> comments. This optimizes when the HW does a tail bump amd when it does a
> WB. It also optimizes when we do a wmb.
>
> Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 126 ++++++++++++++++++---
> -------
> drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 +
> 2 files changed, 86 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index d51b8ed..f75db56 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -785,15 +785,21 @@ static bool i40e_clean_tx_irq(struct i40e_ring
> *tx_ring, int budget)
> tx_ring->q_vector->tx.total_bytes += total_bytes;
> tx_ring->q_vector->tx.total_packets += total_packets;
>
> - /* check to see if there are any non-cache aligned descriptors
> - * waiting to be written back, and kick the hardware to force
> - * them to be written back in case of napi polling
> - */
> - if (budget &&
> - !((i & WB_STRIDE) == WB_STRIDE) &&
> - !test_bit(__I40E_DOWN, &tx_ring->vsi->state) &&
> - (I40E_DESC_UNUSED(tx_ring) != tx_ring->count))
> - tx_ring->arm_wb = true;
> + if (tx_ring->flags & I40E_TXR_FLAGS_WB_ON_ITR) {
> + unsigned int j = 0;
> + /* check to see if there are < 4 descriptors
> + * waiting to be written back, then kick the hardware to force
> + * them to be written back in case we stay in NAPI.
> + * In this mode on X722 we do not enable Interrupt.
> + */
> + j = i40e_get_tx_pending(tx_ring);
> +
> + if (budget &&
> + ((j / (WB_STRIDE + 1)) == 0) && (j != 0) &&
> + !test_bit(__I40E_DOWN, &tx_ring->vsi->state) &&
> + (I40E_DESC_UNUSED(tx_ring) != tx_ring->count))
> + tx_ring->arm_wb = true;
> + }
>
> if (check_for_tx_hang(tx_ring) && i40e_check_tx_hang(tx_ring)) {
> /* schedule immediate reset if we believe we hung */ @@ -
> 2582,6 +2588,9 @@ static inline void i40e_tx_map(struct i40e_ring *tx_ring,
> struct sk_buff *skb,
> u32 td_tag = 0;
> dma_addr_t dma;
> u16 gso_segs;
> + u16 desc_count = 0;
> + bool tail_bump = true;
> + bool do_rs = false;
>
> if (tx_flags & I40E_TX_FLAGS_HW_VLAN) {
> td_cmd |= I40E_TX_DESC_CMD_IL2TAG1;
> @@ -2622,6 +2631,8 @@ static inline void i40e_tx_map(struct i40e_ring
> *tx_ring, struct sk_buff *skb,
>
> tx_desc++;
> i++;
> + desc_count++;
> +
> if (i == tx_ring->count) {
> tx_desc = I40E_TX_DESC(tx_ring, 0);
> i = 0;
> @@ -2641,6 +2652,8 @@ static inline void i40e_tx_map(struct i40e_ring
> *tx_ring, struct sk_buff *skb,
>
> tx_desc++;
> i++;
> + desc_count++;
> +
> if (i == tx_ring->count) {
> tx_desc = I40E_TX_DESC(tx_ring, 0);
> i = 0;
> @@ -2655,34 +2668,6 @@ static inline void i40e_tx_map(struct i40e_ring
> *tx_ring, struct sk_buff *skb,
> tx_bi = &tx_ring->tx_bi[i];
> }
>
> - /* Place RS bit on last descriptor of any packet that spans across the
> - * 4th descriptor (WB_STRIDE aka 0x3) in a 64B cacheline.
> - */
> - if (((i & WB_STRIDE) != WB_STRIDE) &&
> - (first <= &tx_ring->tx_bi[i]) &&
> - (first >= &tx_ring->tx_bi[i & ~WB_STRIDE])) {
> - tx_desc->cmd_type_offset_bsz =
> - build_ctob(td_cmd, td_offset, size, td_tag) |
> - cpu_to_le64((u64)I40E_TX_DESC_CMD_EOP <<
> - I40E_TXD_QW1_CMD_SHIFT);
> - } else {
> - tx_desc->cmd_type_offset_bsz =
> - build_ctob(td_cmd, td_offset, size, td_tag) |
> - cpu_to_le64((u64)I40E_TXD_CMD <<
> - I40E_TXD_QW1_CMD_SHIFT);
> - }
> -
> - netdev_tx_sent_queue(netdev_get_tx_queue(tx_ring->netdev,
> - tx_ring->queue_index),
> - first->bytecount);
> -
> - /* Force memory writes to complete before letting h/w
> - * know there are new descriptors to fetch. (Only
> - * applicable for weak-ordered memory model archs,
> - * such as IA-64).
> - */
> - wmb();
> -
> /* set next_to_watch value indicating a packet is present */
> first->next_to_watch = tx_desc;
>
> @@ -2692,15 +2677,72 @@ static inline void i40e_tx_map(struct i40e_ring
> *tx_ring, struct sk_buff *skb,
>
> tx_ring->next_to_use = i;
>
> + netdev_tx_sent_queue(netdev_get_tx_queue(tx_ring->netdev,
> + tx_ring->queue_index),
> + first->bytecount);
> i40e_maybe_stop_tx(tx_ring, DESC_NEEDED);
> +
> + /* Algorithm to optimize tail and RS bit setting:
> + * if xmit_more is supported
> + * if xmit_more is true
> + * do not update tail and do not mark RS bit.
> + * if xmit_more is false and last xmit_more was false
> + * if every packet spanned less than 4 desc
> + * then set RS bit on 4th packet and update tail
> + * on every packet
> + * else
> + * update tail and set RS bit on every packet.
> + * if xmit_more is false and last_xmit_more was true
> + * update tail and set RS bit.
> + *
> + * Optimization: wmb to be issued only in case of tail update.
> + * Also optimize the Descriptor WB path for RS bit with the same
> + * algorithm.
> + *
> + * Note: If there are less than 4 packets
> + * pending and interrupts were disabled the service task will
> + * trigger a force WB.
> + */
> + if (skb->xmit_more &&
> + !netif_xmit_stopped(netdev_get_tx_queue(tx_ring->netdev,
> + tx_ring->queue_index))) {
> + tx_ring->flags |= I40E_TXR_FLAGS_LAST_XMIT_MORE_SET;
> + tail_bump = false;
> + } else if (!skb->xmit_more &&
> + !netif_xmit_stopped(netdev_get_tx_queue(tx_ring-
> >netdev,
> + tx_ring->queue_index))
> &&
> + (!(tx_ring->flags &
> I40E_TXR_FLAGS_LAST_XMIT_MORE_SET)) &&
> + (tx_ring->packet_stride < WB_STRIDE) &&
> + (desc_count < WB_STRIDE)) {
> + tx_ring->packet_stride++;
> + } else {
> + tx_ring->packet_stride = 0;
> + tx_ring->flags &=
> ~I40E_TXR_FLAGS_LAST_XMIT_MORE_SET;
> + do_rs = true;
> + }
> + if (do_rs)
> + tx_ring->packet_stride = 0;
> +
> + tx_desc->cmd_type_offset_bsz =
> + build_ctob(td_cmd, td_offset, size, td_tag) |
> + cpu_to_le64((u64)(do_rs ? I40E_TXD_CMD :
> + I40E_TX_DESC_CMD_EOP)
> <<
> +
> I40E_TXD_QW1_CMD_SHIFT);
> +
> /* notify HW of packet */
> - if (!skb->xmit_more ||
> - netif_xmit_stopped(netdev_get_tx_queue(tx_ring->netdev,
> - tx_ring->queue_index)))
> - writel(i, tx_ring->tail);
> - else
> + if (!tail_bump)
> prefetchw(tx_desc + 1);
>
> + if (tail_bump) {
> + /* Force memory writes to complete before letting h/w
> + * know there are new descriptors to fetch. (Only
> + * applicable for weak-ordered memory model archs,
> + * such as IA-64).
> + */
> + wmb();
> + writel(i, tx_ring->tail);
> + }
> +
> return;
>
> dma_error:
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> index 073464e..4871809 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
> @@ -266,10 +266,12 @@ struct i40e_ring {
>
> bool ring_active; /* is ring online or not */
> bool arm_wb; /* do something to arm write back */
> + u8 packet_stride;
>
> u16 flags;
> #define I40E_TXR_FLAGS_WB_ON_ITR BIT(0)
> #define I40E_TXR_FLAGS_OUTER_UDP_CSUM BIT(1)
> +#define I40E_TXR_FLAGS_LAST_XMIT_MORE_SET BIT(2)
>
> /* stats structs */
> struct i40e_queue_stats stats;
> --
> 1.8.1.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH 2/2] i40e: Disable Check for Hang
2015-09-25 19:00 ` [Intel-wired-lan] [PATCH 2/2] i40e: Disable Check for Hang Anjali Singhai Jain
@ 2015-09-25 23:16 ` Jeff Kirsher
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Kirsher @ 2015-09-25 23:16 UTC (permalink / raw)
To: intel-wired-lan
On Fri, 2015-09-25 at 12:00 -0700, Anjali Singhai Jain wrote:
> The driver doesn't need its own check for hang logic as there are no
> known issues that cause HW Tx Hangs. This logic was actually
> (mis)firing frequently, causing us to reset when we didn't need to.
> If a true tx hang happens, just wait for the stack logic to catch the
> issue and send us a tx_timeout.
>
> Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 99 +------------------
> ----------
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 70 -------------------
> -
> drivers/net/ethernet/intel/i40e/i40e_txrx.h | 8 ---
> 3 files changed, 1 insertion(+), 176 deletions(-)
Does not apply at all to my next-queue tree, dev-queue branch.
Dropping this patch.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20150925/82234a5d/attachment.asc>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround
2015-09-25 19:00 [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround Anjali Singhai Jain
2015-09-25 19:00 ` [Intel-wired-lan] [PATCH 2/2] i40e: Disable Check for Hang Anjali Singhai Jain
2015-09-25 19:50 ` [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround Singhai, Anjali
@ 2015-09-25 23:18 ` Jeff Kirsher
2015-09-26 0:00 ` Singhai, Anjali
2 siblings, 1 reply; 6+ messages in thread
From: Jeff Kirsher @ 2015-09-25 23:18 UTC (permalink / raw)
To: intel-wired-lan
On Fri, 2015-09-25 at 12:00 -0700, Anjali Singhai Jain wrote:
> This patch fixes the issue of forcing WB too often causing us to not
> benefit from NAPI.
>
> Without this patch we were forcing WB/arming interrupt too often
> taking
> away the benefits of NAPI and causing a performance impact.
>
> With this patch we disable force WB in the clean routine for X710
> and XL710 adapters. X722 adapters do not enable interrupt to force
> a WB and benefit from WB_ON_ITR and hence force WB is left enabled
> for those adapters.
> For XL710 and X710 adapters if we have less than 4 packets pending
> a software Interrupt triggered from service task will force a WB.
>
> This patch also changes the conditions for setting RS bit as
> described
> in code comments. This optimizes when the HW does a tail bump amd
> when
> it does a WB. It also optimizes when we do a wmb.
>
> Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 126 ++++++++++++++++++
> ----------
> drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 +
> 2 files changed, 86 insertions(+), 42 deletions(-)
This applied with a little massaging, but seeing that the second patch
does not apply at all. I am dropping this patch.
Please resend an updated patch against my next-queue tree, dev-queue
branch.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20150925/6a834ff5/attachment-0001.asc>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround
2015-09-25 23:18 ` Jeff Kirsher
@ 2015-09-26 0:00 ` Singhai, Anjali
0 siblings, 0 replies; 6+ messages in thread
From: Singhai, Anjali @ 2015-09-26 0:00 UTC (permalink / raw)
To: intel-wired-lan
Jeff, I realized later that disable check for hang patch got squished to another pach from Catherine so you could drop that one. But since that one already got taken care off by Catherine, just go ahead and apply this one.
Thanks
Anjali
Sent from my iPhone
> On Sep 25, 2015, at 4:18 PM, Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com> wrote:
>
>> On Fri, 2015-09-25 at 12:00 -0700, Anjali Singhai Jain wrote:
>> This patch fixes the issue of forcing WB too often causing us to not
>> benefit from NAPI.
>>
>> Without this patch we were forcing WB/arming interrupt too often
>> taking
>> away the benefits of NAPI and causing a performance impact.
>>
>> With this patch we disable force WB in the clean routine for X710
>> and XL710 adapters. X722 adapters do not enable interrupt to force
>> a WB and benefit from WB_ON_ITR and hence force WB is left enabled
>> for those adapters.
>> For XL710 and X710 adapters if we have less than 4 packets pending
>> a software Interrupt triggered from service task will force a WB.
>>
>> This patch also changes the conditions for setting RS bit as
>> described
>> in code comments. This optimizes when the HW does a tail bump amd
>> when
>> it does a WB. It also optimizes when we do a wmb.
>>
>> Signed-off-by: Anjali Singhai Jain <anjali.singhai@intel.com>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 126 ++++++++++++++++++
>> ----------
>> drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 +
>> 2 files changed, 86 insertions(+), 42 deletions(-)
>
> This applied with a little massaging, but seeing that the second patch
> does not apply at all. I am dropping this patch.
>
> Please resend an updated patch against my next-queue tree, dev-queue
> branch.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-26 0:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-25 19:00 [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround Anjali Singhai Jain
2015-09-25 19:00 ` [Intel-wired-lan] [PATCH 2/2] i40e: Disable Check for Hang Anjali Singhai Jain
2015-09-25 23:16 ` Jeff Kirsher
2015-09-25 19:50 ` [Intel-wired-lan] [PATCH 1/2] i40e: Fix RS bit update in Tx path and disable force WB workaround Singhai, Anjali
2015-09-25 23:18 ` Jeff Kirsher
2015-09-26 0:00 ` Singhai, Anjali
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.