Patchwork [resend,1/3] Revert "net: fec: fix missing napi_disable call"

login
register
mail settings
Submitter Lucas Stach
Date April 26, 2013, 8:52 a.m.
Message ID <1366966330-5181-1-git-send-email-l.stach@pengutronix.de>
Download mbox | patch
Permalink /patch/239764/
State Rejected
Delegated to: David Miller
Headers show

Comments

Lucas Stach - April 26, 2013, 8:52 a.m.
This reverts commit 3f104c38259dcb3e5443c246f0805bc04d887cc3.

In preparaton to revert the other NAPI related changes.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/net/ethernet/freescale/fec.c | 1 -
 1 file changed, 1 deletion(-)
Frank Li - April 26, 2013, 9:32 a.m.
2013/4/26 Lucas Stach <l.stach@pengutronix.de>:
> This reverts commit 3f104c38259dcb3e5443c246f0805bc04d887cc3.
>
> In preparaton to revert the other NAPI related changes.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

David:

Please hold apply this patch for some days.
Let me found a good way to fix the issue that lucas met.

This patch may cause spin_lock dead lock when using 1588 ptp.

best regards
Frank Li

> ---
>  drivers/net/ethernet/freescale/fec.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
> index 73195f6..afb52eb 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -1490,7 +1490,6 @@ fec_enet_close(struct net_device *ndev)
>         struct fec_enet_private *fep = netdev_priv(ndev);
>
>         /* Don't know what to do yet. */
> -       napi_disable(&fep->napi);
>         fep->opened = 0;
>         netif_stop_queue(ndev);
>         fec_stop(ndev);
> --
> 1.8.2.rc2
>
> --
> 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
--
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
Robert Schwebel - April 26, 2013, 1:44 p.m.
Frank,

On Fri, Apr 26, 2013 at 05:32:02PM +0800, Frank Li wrote:
> 2013/4/26 Lucas Stach <l.stach@pengutronix.de>:
> > This reverts commit 3f104c38259dcb3e5443c246f0805bc04d887cc3.
> >
> > In preparaton to revert the other NAPI related changes.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
>
> David:
>
> Please hold apply this patch for some days.
> Let me found a good way to fix the issue that lucas met.
>
> This patch may cause spin_lock dead lock when using 1588 ptp.

Seriously - it's friday, and 3.9 is expected to come out this weekend.
You broke the driver in mainline with your patches, and the current
state is that it isn't fixable before the release.

So in order not to break the FEC in 3.9-final, please let's go with the
reverts for now; if 1588 PTP is broken, it was broken in 3.8 as well,
correct? After the release, there will be a lot of time to make proper
patches that fix the issues.

rsc
David Miller - April 26, 2013, 6:33 p.m.
From: Robert Schwebel <r.schwebel@pengutronix.de>
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.

Thanks.

--
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
Robert Schwebel - April 27, 2013, 9:05 a.m.
On Fri, Apr 26, 2013 at 02:33:09PM -0400, David Miller wrote:
> From: Robert Schwebel <r.schwebel@pengutronix.de>
> 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
- break the driver, up to a point where the kernel oopses when the link
  goes away
- 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
David Miller - April 28, 2013, 5:11 a.m.
From: Robert Schwebel <r.schwebel@pengutronix.de>
Date: Sat, 27 Apr 2013 11:05:45 +0200

> Please consider to merge the reverts. Otherwhise FEC will be broken
> in 3.9.

The amount of effort you put into this email, and complaining,
far exceed that necessary to help Frank write a fix.

I've already told you how I want to see this resolved, and you
haven't given me any new information, so my point of view has
not changed.
--
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
Robert Schwebel - April 28, 2013, 5:27 p.m.
On Sun, Apr 28, 2013 at 01:11:44AM -0400, David Miller wrote:
> > Please consider to merge the reverts. Otherwhise FEC will be broken
> > in 3.9.
>
> The amount of effort you put into this email, and complaining,
> far exceed that necessary to help Frank write a fix.
>
> I've already told you how I want to see this resolved, and you
> haven't given me any new information, so my point of view has
> not changed.

Ok, so 3.9 will be broken - it's your decision. We'll have a lot of time
to fix things properly then; Lucas will test & comment the new patches
next week then.

rsc

Patch

diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 73195f6..afb52eb 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1490,7 +1490,6 @@  fec_enet_close(struct net_device *ndev)
 	struct fec_enet_private *fep = netdev_priv(ndev);
 
 	/* Don't know what to do yet. */
-	napi_disable(&fep->napi);
 	fep->opened = 0;
 	netif_stop_queue(ndev);
 	fec_stop(ndev);