From patchwork Sun Feb 28 09:50:20 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladislav Zolotarov X-Patchwork-Id: 46486 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 180BCB7DEF for ; Sun, 28 Feb 2010 21:00:26 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031719Ab0B1KAS (ORCPT ); Sun, 28 Feb 2010 05:00:18 -0500 Received: from mms1.broadcom.com ([216.31.210.17]:3060 "EHLO mms1.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031648Ab0B1KAR (ORCPT ); Sun, 28 Feb 2010 05:00:17 -0500 Received: from [10.9.200.131] by mms1.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.3.2)); Sun, 28 Feb 2010 02:00:04 -0800 X-Server-Uuid: 02CED230-5797-4B57-9875-D5D2FEE4708A Received: from mail-irva-12.broadcom.com (10.11.16.101) by IRVEXCHHUB01.corp.ad.broadcom.com (10.9.200.131) with Microsoft SMTP Server id 8.2.213.0; Sun, 28 Feb 2010 02:00:03 -0800 Received: from [10.185.6.94] (lb-tlvb-vladz.il.broadcom.com [10.185.6.94]) by mail-irva-12.broadcom.com (Postfix) with ESMTP id BDE4169CAB; Sun, 28 Feb 2010 02:00:00 -0800 (PST) Subject: [PATCH 1/1] Tx barriers and locks From: "Vladislav Zolotarov" To: netdev@vger.kernel.org, davem@davemloft.net cc: eilong@broadcom.com, sgruszka@redhat.com Date: Sun, 28 Feb 2010 11:50:20 +0200 Message-ID: <1267350620.2952.33.camel@lb-tlvb-vladz> MIME-Version: 1.0 X-Mailer: Evolution 2.28.1 X-WSS-ID: 6794E12E20S44134290-01-01 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org This patch is based on the RFC of Stanislaw Gruszka. More specifically it fixes two possible races: - One, described by Stanislaw, may lead to permanent disabling of the Tx queue. This is fixed by adding the smp_wmb() to propagate the BD consumer change towards the memory. - Second may lead to bnx2x_start_xmit() returning NETDEV_TX_BUSY. This is fixed by taking a tx_lock() before rechecking the number of available Tx BDs. thanks, vlad Signed-off-by: Stanislaw Gruszka Signed-off-by: Vladislav Zolotarov Signed-off-by: Eilon Greenstein --- drivers/net/bnx2x_main.c | 31 ++++++++++++++++++++++--------- 1 files changed, 22 insertions(+), 9 deletions(-) diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c index 5adf2a0..ed785a3 100644 --- a/drivers/net/bnx2x_main.c +++ b/drivers/net/bnx2x_main.c @@ -57,8 +57,8 @@ #include "bnx2x_init_ops.h" #include "bnx2x_dump.h" -#define DRV_MODULE_VERSION "1.52.1-6" -#define DRV_MODULE_RELDATE "2010/02/16" +#define DRV_MODULE_VERSION "1.52.1-7" +#define DRV_MODULE_RELDATE "2010/02/28" #define BNX2X_BC_VER 0x040200 #include @@ -957,21 +957,34 @@ static int bnx2x_tx_int(struct bnx2x_fastpath *fp) fp->tx_pkt_cons = sw_cons; fp->tx_bd_cons = bd_cons; + /* Need to make the tx_bd_cons update visible to start_xmit() + * before checking for netif_tx_queue_stopped(). Without the + * memory barrier, there is a small possibility that + * start_xmit() will miss it and cause the queue to be stopped + * forever. + */ + smp_wmb(); + /* TBD need a thresh? */ if (unlikely(netif_tx_queue_stopped(txq))) { - - /* Need to make the tx_bd_cons update visible to start_xmit() - * before checking for netif_tx_queue_stopped(). Without the - * memory barrier, there is a small possibility that - * start_xmit() will miss it and cause the queue to be stopped - * forever. + /* Taking tx_lock() is needed to prevent reenabling the queue + * while it's empty. This could have happen if rx_action() gets + * suspended in bnx2x_tx_int() after the condition before + * netif_tx_wake_queue(), while tx_action (bnx2x_start_xmit()): + * + * stops the queue->sees fresh tx_bd_cons->releases the queue-> + * sends some packets consuming the whole queue again-> + * stops the queue */ - smp_mb(); + + __netif_tx_lock(txq, smp_processor_id()); if ((netif_tx_queue_stopped(txq)) && (bp->state == BNX2X_STATE_OPEN) && (bnx2x_tx_avail(fp) >= MAX_SKB_FRAGS + 3)) netif_tx_wake_queue(txq); + + __netif_tx_unlock(txq); } return 0; }