From patchwork Sat Apr 27 10:26:57 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhi Li X-Patchwork-Id: 240100 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 EDDB42C0097 for ; Sat, 27 Apr 2013 20:27:04 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754985Ab3D0K1A (ORCPT ); Sat, 27 Apr 2013 06:27:00 -0400 Received: from mail-wi0-f176.google.com ([209.85.212.176]:60273 "EHLO mail-wi0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752319Ab3D0K07 (ORCPT ); Sat, 27 Apr 2013 06:26:59 -0400 Received: by mail-wi0-f176.google.com with SMTP id hj19so1379792wib.3 for ; Sat, 27 Apr 2013 03:26:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=MopJ3Zaxfbs2U7+2LOKY5C38/r5XEa23YY8IUfw2uVk=; b=VQW2LgkVccrEYDZzxu9iJybMJVyxFDDhjrKrUTQMBwU7hBRSgtyzP4SokYsp+eRudY zXslRT+tQoFXbMVZuhFgSS9qycmvltdbo5kdTwRXqonJT2t6dhf7AGQ4JVVw+RgiaPN4 NAAFwvBvd6OX4FGEjwLtriMhrUly2uQ39eYgXffSZl/G+KCnrcUFx3zB/eHfdxLvLukt IvDcGVMZtHNzO2HUNXXUopJppp+F9FNOUAMBJD0jbLKjZf8Flr41hlPMp8mfYymNXwUw eoqG0WyxoWCkokEro3Cim64dkN7iJ7dvdaF+WeN9v7FFPLB3JFyuAqTOUZzYRYoeAaKs Ig6g== MIME-Version: 1.0 X-Received: by 10.180.88.33 with SMTP id bd1mr8511806wib.18.1367058418078; Sat, 27 Apr 2013 03:26:58 -0700 (PDT) Received: by 10.227.4.75 with HTTP; Sat, 27 Apr 2013 03:26:57 -0700 (PDT) In-Reply-To: <20130427090545.GY1366@pengutronix.de> References: <1366966330-5181-1-git-send-email-l.stach@pengutronix.de> <20130426134415.GS1366@pengutronix.de> <20130426.143309.1011152996477262852.davem@davemloft.net> <20130427090545.GY1366@pengutronix.de> Date: Sat, 27 Apr 2013 18:26:57 +0800 Message-ID: Subject: Re: [PATCH resend 1/3] Revert "net: fec: fix missing napi_disable call" From: Frank Li To: Robert Schwebel Cc: David Miller , Lucas Stach , "netdev@vger.kernel.org" , "Frank.Li@freescale.com" , Fabio Estevam , Shawn Guo Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 2013/4/27 Robert Schwebel : > On Fri, Apr 26, 2013 at 02:33:09PM -0400, David Miller wrote: >> From: Robert Schwebel >> Date: Fri, 26 Apr 2013 15:44:15 +0200 >> >> > Seriously - it's friday, and 3.9 is expected to come out this >> > weekend. >> >> Seriously, it took you how long to notice the breakage and report >> it in sufficient detail for the author to make an attempt at a fix? >> >> I thnk Frank's request is reasonable given the circumstances, please >> work closely with him on the fix. > > The FEC driver has worked fine in 3.8.x. > > Frank's patches for the 3.9 cycle... > > - remove locking in a way that memory is freed which is in use The purpose of remove lock is that fix dead lock. It is true I miss a execute path to reset BD descript. But the possibility is very low. You report this problem without detail reproduce steps. > - break the driver, up to a point where the kernel oopses when the link > goes away run script while [ 1 ]; do ethtool -s eth0 autoneg off;sleep 3;ethtool -s eth0 autoneg on; sleep 4; done; I just capture one oops during 1 hours. I am try to fix this issue without add back lock. I plan use below way to fix this issue. I am running my stress test. if you have time, you can run it at your sit. > - mix up different changes (queue handling) and should have been split up > into separate patches > > Unfortunately, the breakage happens only on multicore (MX6Q) and if you > change the link status; that's probably the reason why it hasn't been > noticed earlier. > > We have really tried to find a "quick fix which does it right", but it > has turned out that this isn't possible in such a short time, because it > is more complex than just re-adding locks. We feel that the results of > last week's activities are not good enough that they could be merged > without further breakage. > > Please consider to merge the reverts. Otherwhise FEC will be broken in 3.9. > > Of course, we can help with a real solution, but please after 3.9.final. > > Thanks, > Robert > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fe index 73195f6..c945bb7 100644 --- a/drivers/net/ethernet/freescale/fec.c +++ b/drivers/net/ethernet/freescale/fec.c @@ -403,6 +403,11 @@ fec_restart(struct net_device *ndev, int duplex) const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); int i; + if (netif_running(ndev)) { + napi_disable(&fep->napi); + netif_stop_queue(ndev); + } + u32 temp_mac[2]; u32 rcntl = OPT_FRAME_SIZE | 0x04; u32 ecntl = 0x2; /* ETHEREN */ @@ -559,6 +564,11 @@ fec_restart(struct net_device *ndev, int duplex) /* Enable interrupts we wish to service */ writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK); + + if (netif_running(ndev)) { + napi_enable(&fep->napi); + netif_wake_queue(ndev); + } }