Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762190AbXJRG3L (ORCPT ); Thu, 18 Oct 2007 02:29:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756394AbXJRG2x (ORCPT ); Thu, 18 Oct 2007 02:28:53 -0400 Received: from mx10.go2.pl ([193.17.41.74]:35508 "EHLO poczta.o2.pl" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756364AbXJRG2w (ORCPT ); Thu, 18 Oct 2007 02:28:52 -0400 Date: Thu, 18 Oct 2007 08:31:57 +0200 From: Jarek Poplawski To: "Maciej W\. Rozycki" Cc: Andy Fleming , Andrew Morton , Jeff Garzik , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] PHYLIB: IRQ event workqueue handling fixes Message-ID: <20071018063157.GA1694@ff.dom.local> References: <20071015125301.GC3015@ff.dom.local> <20071016062108.GB1000@ff.dom.local> <20071017085809.GA1658@ff.dom.local> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20071017085809.GA1658@ff.dom.local> User-Agent: Mutt/1.4.2.2i Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1359 Lines: 40 On Wed, Oct 17, 2007 at 10:58:09AM +0200, Jarek Poplawski wrote: ... > 8) phy_stop_interrupts(): I'm not sure this additional call from > DEBUG_SHIRQ should be so dangerous, eg.: > > /* > * status == PHY_HALTED && > * interrupts are stopped after phy_stop() > */ > if (cancel_work_sync(...)) > enable_irq(); > > free_irq(...); > /* > * possible schedule_work() from DEBUG_SHIRQ only, > * but proper check for PHY_HALTED is done; > * so, let's flush after this too: > */ > cancel_work_sync(); After rethinking, it looks like this last cancel should be useless. So, if phy_interrupt() schedules only if !PHY_HALTED and phy_change() does enable_irq() with no exeptions, it seems phy_interrupt() even without lock must see PHY_HALTED state before this free_irq() with possible DEBUG_SHIRQ call, then maybe only this safety: WARN_ON(work_pending(&phydev->phy_queue)); Btw, I've read this was considered and not liked, but IMHO, if this really has to be like this, creating phy's own workqueue seems to be resonable, at the very least to reduce latencies to other users of this irq. 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/