From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4FE60C04A68 for ; Thu, 28 Jul 2022 13:14:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238100AbiG1NOd (ORCPT ); Thu, 28 Jul 2022 09:14:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52594 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236620AbiG1NOa (ORCPT ); Thu, 28 Jul 2022 09:14:30 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5ECF82CC8B for ; Thu, 28 Jul 2022 06:14:29 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 17C1DB82444 for ; Thu, 28 Jul 2022 13:14:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C123EC433C1; Thu, 28 Jul 2022 13:14:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1659014066; bh=zXjDJxQ311o3IVYcPBceCf8E6XYgNC1pAoh4HZnuioE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=T8SMEHy5uY7dVQ7QwcGeWG1wcpquH+oxSmFvuL8AmgememDdM+ytjbKYqhY1hExaF 3KbV0HGxfE/p4Nqu9YGSH0LoeI33q+r75gmwvG3gC33FNhT4joEtCpSchazW1+HiBB /x/xUvhmMKO66LOaA3gDCafvRutA/uBUprp7CjzkLG08QSZGETPZ0YTV+pa/wk6rSG Hre1WvwCqKik/qXRxbVFU6vcvcAiV8/L6luHG8JbzC8u1g5cq2QsF9lVIXqXfICLkv KPepuDBo4v84OWqVsRkIHat340RosZFO9MBFq3Lqw4gMz4crEXY4bYKO/BPKYFmwCv AD9SjQ5YUvYFg== Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1oH3LM-00Afg5-Bh; Thu, 28 Jul 2022 14:14:24 +0100 Date: Thu, 28 Jul 2022 14:14:24 +0100 Message-ID: <87v8rhwf9r.wl-maz@kernel.org> From: Marc Zyngier To: Johan Hovold Cc: Johan Hovold , Thomas Gleixner , Greg Kroah-Hartman , Rob Herring , linux-kernel@vger.kernel.org, Dmitry Torokhov , Jon Hunter Subject: Re: [PATCH] irqdomain: Fix mapping-creation race In-Reply-To: References: <20220728092710.21190-1-johan+linaro@kernel.org> <87wnbxwj94.wl-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: johan@kernel.org, johan+linaro@kernel.org, tglx@linutronix.de, gregkh@linuxfoundation.org, robh@kernel.org, linux-kernel@vger.kernel.org, dtor@chromium.org, jonathanh@nvidia.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 28 Jul 2022 13:56:41 +0100, Johan Hovold wrote: > > On Thu, Jul 28, 2022 at 12:48:23PM +0100, Marc Zyngier wrote: > > On Thu, 28 Jul 2022 10:27:10 +0100, > > Johan Hovold wrote: > > > > > > Parallel probing (e.g. due to asynchronous probing) of devices that share > > > interrupts can currently result in two mappings for the same hardware > > > interrupt to be created. > > > > And I thought nobody would be using shared interrupts anymore. Turns > > out people are still building braindead HW... :-/ > > > > > > > > Add a serialising mapping mutex so that looking for an existing mapping > > > before creating a new one is done atomically. > > > > > > Note that serialising the lookup and creation in > > > irq_create_mapping_affinity() would have been enough to prevent the > > > duplicate mapping, but that could instead cause > > > irq_create_fwspec_mapping() to fail when there is a race. > > > > > > Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers") > > > Fixes: b62b2cf5759b ("irqdomain: Fix handling of type settings for existing mappings") > > > Cc: Dmitry Torokhov > > > Cc: Jon Hunter > > > Signed-off-by: Johan Hovold > > > --- > > > kernel/irq/irqdomain.c | 46 +++++++++++++++++++++++++++++++----------- > > > 1 file changed, 34 insertions(+), 12 deletions(-) > > > > > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > > > index 8fe1da9614ee..d263a7dd4170 100644 > > > --- a/kernel/irq/irqdomain.c > > > +++ b/kernel/irq/irqdomain.c > > > @@ -22,6 +22,7 @@ > > > > > > static LIST_HEAD(irq_domain_list); > > > static DEFINE_MUTEX(irq_domain_mutex); > > > +static DEFINE_MUTEX(irq_mapping_mutex); > > > > I'd really like to avoid a global mutex. At the very least this should > > be a per-domain mutex, otherwise this will serialise a lot more than > > what is needed. > > Yeah, I considered that too, but wanted to get your comments on this > first. > > Also note that the likewise global irq_domain_mutex (and > sparse_irq_lock) are taken in some of these paths so perhaps using finer > locking won't actually matter that much as this is mostly for parallel > probing. It will be a good opportunity to make the locking suck a bit less, like in irq_domain_associate(). > > > } else { > > > /* Create mapping */ > > > - virq = irq_create_mapping(domain, hwirq); > > > + virq = __irq_create_mapping_affinity(domain, hwirq, NULL); > > > > This rechecks for the existence of the mapping. Surely we can do a bit > > better by rejigging this (admittedly bitrotting) code. > > I'm sure we can. Should I try to fix the race first with a patch like > this one that can potentially be backported, and then see what I can do > about cleaning this up? > > After all it has looked like this for the past eight years since when > this code was first merged. No, let's put the code in shape *first*, then add work on the locking, as it should make the patch simpler. Backports aren't my concern, really. Thanks, M. -- Without deviation from the norm, progress is not possible.