From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH] net: dsa: mv88e6xxx: fix races between lock and irq freeing Date: Sat, 21 Jul 2018 22:44:09 -0700 (PDT) Message-ID: <20180721.224409.437815752648454744.davem@davemloft.net> References: <20180720095315.21055-1-u.kleine-koenig@pengutronix.de> Mime-Version: 1.0 Content-Type: Text/Plain; charset=iso-8859-1 Content-Transfer-Encoding: 8BIT Cc: andrew@lunn.ch, vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com, netdev@vger.kernel.org, kernel@pengutronix.de To: u.kleine-koenig@pengutronix.de Return-path: Received: from shards.monkeyblade.net ([23.128.96.9]:49884 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727830AbeGVGjj (ORCPT ); Sun, 22 Jul 2018 02:39:39 -0400 In-Reply-To: <20180720095315.21055-1-u.kleine-koenig@pengutronix.de> Sender: netdev-owner@vger.kernel.org List-ID: From: Uwe Kleine-König Date: Fri, 20 Jul 2018 11:53:15 +0200 > free_irq() waits until all handlers for this IRQ have completed. As the > relevant handler (mv88e6xxx_g1_irq_thread_fn()) takes the chip's reg_lock > it might never return if the thread calling free_irq() holds this lock. > > For the same reason kthread_cancel_delayed_work_sync() in the polling case > must not hold this lock. > > Also first free the irq (or stop the worker respectively) such that > mv88e6xxx_g1_irq_thread_work() isn't called any more before the irq > mappings are dropped in mv88e6xxx_g1_irq_free_common() to prevent the > worker thread to call handle_nested_irq(0) which results in a NULL-pointer > exception. > > Signed-off-by: Uwe Kleine-König Looks good. Note than the IRQ domain unmapping will do a synchronize_irq() which should cause the same deadlock as free_irq() will with the reg_lock held. Note also that g2 IRQ freeing gets the ordering right, and doesn't need a lock because it doesn't program any registers when tearing down it's IRQ. Applied and queued up for -stable, thanks.