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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <BE651D76-77D9-4CC0-85E1-F586C24F9012@freescale.com>
Date:	Mon, 4 Dec 2006 13:54:45 -0600
From:	Andy Fleming <afleming@...escale.com>
To:	"Maciej W. Rozycki" <macro@...ux-mips.org>
Cc:	Andrew Morton <akpm@...l.org>, Jeff Garzik <jgarzik@...ox.com>,
	Ralf Baechle <ralf@...ux-mips.org>, netdev@...r.kernel.org,
	linux-mips@...ux-mips.org
Subject: Re: [patch 3/6] 2.6.18: sb1250-mac: Phylib IRQ handling fixes


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


-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html