linux-mips
[Top] [All Lists]

Re: [patch 3/6] 2.6.18: sb1250-mac: Phylib IRQ handling fixes

To: "Maciej W. Rozycki" <macro@linux-mips.org>
Subject: Re: [patch 3/6] 2.6.18: sb1250-mac: Phylib IRQ handling fixes
From: Andy Fleming <afleming@freescale.com>
Date: Mon, 4 Dec 2006 13:54:45 -0600
Cc: Andrew Morton <akpm@osdl.org>, Jeff Garzik <jgarzik@pobox.com>, Ralf Baechle <ralf@linux-mips.org>, netdev@vger.kernel.org, linux-mips@linux-mips.org
In-reply-to: <Pine.LNX.4.64N.0611301757200.1757@blysk.ds.pg.gda.pl>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <Pine.LNX.4.64N.0610031509380.4642@blysk.ds.pg.gda.pl> <E2ACBAE3-B0E1-4D90-BF25-6981543090C4@freescale.com> <Pine.LNX.4.64N.0610231752440.4426@blysk.ds.pg.gda.pl> <Pine.LNX.4.64N.0611301757200.1757@blysk.ds.pg.gda.pl>
Sender: linux-mips-bounce@linux-mips.org

On Nov 30, 2006, at 12:07, Maciej W. Rozycki wrote:

On Mon, 23 Oct 2006, Maciej W. Rozycki wrote:

I'm not too enthusiastic about requiring the ethernet drivers to call phy_disconnect in a separate thread after "close" is called. Assuming there's not some sort of "squash work queue" function that can be invoked with rtnl_lock held, I think phy_disconnect should schedule itself to flush the queue. This would also require that mdiobus_unregister hold off on freeing phydevs if any of the phys were still waiting for pending flush_pending calls to finish. Which would, in turn, require mdiobus_unregister to schedule
cleaning up memory for some later time.

 This could work, indeed.

I'm not enthusiastic about that implementation, either, but it maintains the abstractions I consider important for this code. The ethernet driver should not need to know what structures the PHY lib uses to implement its interrupt
handling, and how to work around their failings, IMHO.

 Agreed.

 So what's the plan?

 Here's a new version of the patch that addresses your other concerns.


So I think the problem is we still don't understand the problem, and the solution to the problem, except that it's causing your driver to lock up. Most of the changes below are fine with me. The confusing one is still the check for current_is_keventd(). This is related in some way to why the driver code invokes phy_disconnect from a work_queue. I admit, though, I'm not familiar enough with the work queue infrastructure to understand the problem. But I'm very certain that creating a work queue for the sole purpose of disconnecting from the PHY is crufty.

Can you try again to convey how this solves your problem, so we can try to figure out if there's a better way?

Andy



<Prev in Thread] Current Thread [Next in Thread>
  • Re: [patch 3/6] 2.6.18: sb1250-mac: Phylib IRQ handling fixes, Andy Fleming <=