lkml.org 
[lkml]   [2007]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] PHYLIB: IRQ event workqueue handling fixes
    On Fri, Oct 19, 2007 at 12:38:29PM +0100, Maciej W. Rozycki wrote:
    > On Thu, 18 Oct 2007, Maciej W. Rozycki wrote:
    >
    > > > 1) phy_change() checks PHY_HALTED flag without lock; I think it's
    > > > racy: eg. if it's done during phy_stop() it can check just before
    > > > the flag is set and reenable interrupts just after phy_stop() ends.
    > >
    > > I remember having a look into it, but it was long ago and I cannot
    > > immediately recall the conclusion. Which means it is either broken or
    > > deserves a comment as non-obvious. I will have a look into it again, but
    > > I am resource-starved a little at the moment, sorry.
    >
    > Well, I have now recalled what the issue is -- we just plainly and simply
    > want to avoid a hardirq spinlock for the very reason we do not do all the
    > processing in the hardirq handler. The thing is we make accesses to the
    > MDIO bus with the phydev lock held and it may take ages until these
    > accesses will have completed. And we cannot afford keeping interrupts
    > disabled for so long.
    >
    > So the only way is to make the check for the HALTED state lockless and
    > make sure any race condition is handled gracefully and does not lead to
    > inconsistent behaviour. Which I think as of what we have in the
    > net-2.6.24 tree is the case, but there are never too many eyes to look at
    > a piece of code, so if anybody feels like proving me wrong, then just go
    > ahead!

    Actually I'm not convinced with this explanation. It seems to me that
    since there are such serious locking problems (especially with rntl),
    there could be once more considered a private workqueue. You've
    written earlier about being a lonely user of this code. But, since
    Benjamin offered his help with changing to mutexes, which looks like
    very reasonable idea to me (probably I miss most of the points...),
    maybe it's very good opportunity to both: make this code better and
    double the user base! I'm interested in looking for such solution
    if Benjamin thinks there could be too few problems for him... So,
    let somebody tell us what could be wrong with this idea?

    Cheers (till Monday),
    Jarek P.
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-10-19 16:39    [W:3.886 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site