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 69B66C433FE for ; Tue, 10 May 2022 08:19:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237935AbiEJIXB (ORCPT ); Tue, 10 May 2022 04:23:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59782 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237466AbiEJIW7 (ORCPT ); Tue, 10 May 2022 04:22:59 -0400 Received: from bmailout3.hostsharing.net (bmailout3.hostsharing.net [IPv6:2a01:4f8:150:2161:1:b009:f23e:0]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9118E28E4FE; Tue, 10 May 2022 01:19:01 -0700 (PDT) Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL TLS DV RSA Mixed SHA256 2020 CA-1" (verified OK)) by bmailout3.hostsharing.net (Postfix) with ESMTPS id CCFF610029C26; Tue, 10 May 2022 10:18:58 +0200 (CEST) Received: by h08.hostsharing.net (Postfix, from userid 100393) id A91422E6707; Tue, 10 May 2022 10:18:58 +0200 (CEST) Date: Tue, 10 May 2022 10:18:58 +0200 From: Lukas Wunner To: Marc Zyngier Cc: Mark Rutland , Jakub Kicinski , Thomas Gleixner , "David S. Miller" , Paolo Abeni , netdev@vger.kernel.org, linux-usb@vger.kernel.org, Steve Glendinning , UNGLinuxDriver@microchip.com, Oliver Neukum , Andre Edich , Oleksij Rempel , Martyn Welch , Gabriel Hojda , Christoph Fritz , Lino Sanfilippo , Philipp Rosenberger , Heiner Kallweit , Andrew Lunn , Russell King , Ferry Toth Subject: Re: [PATCH net-next v2 5/7] usbnet: smsc95xx: Forward PHY interrupts to PHY driver to avoid polling Message-ID: <20220510081858.GA13058@wunner.de> References: <20220505113207.487861b2@kernel.org> <20220505185328.GA14123@wunner.de> <87tua36i70.wl-maz@kernel.org> <20220506201647.GA30860@wunner.de> <87ilqf6qjs.wl-maz@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ilqf6qjs.wl-maz@kernel.org> User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, May 09, 2022 at 09:47:19AM +0100, Marc Zyngier wrote: > On Fri, 06 May 2022 21:16:47 +0100, Lukas Wunner wrote: > > On Fri, May 06, 2022 at 11:58:43AM +0100, Marc Zyngier wrote: > > > On Thu, 05 May 2022 19:53:28 +0100, Lukas Wunner wrote: > > > > generic_handle_domain_irq() warns unconditionally on !in_irq(), > > > > unlike handle_irq_desc(), which constrains the warning to > > > > handle_enforce_irqctx() (i.e. x86 APIC, arm GIC/GICv3). > > > > Perhaps that's an oversight in generic_handle_domain_irq(), > > > > unless __irq_resolve_mapping() becomes unsafe outside in_irq() > > > > for some reason... > > > > > > > > In any case the unconditional in_irq() necessitates __irq_enter_raw() > > > > here. > > > > > > Please don't directly use __irq_enter_raw() and similar things > > > directly in driver code (it doesn't do anything related to RCU, for > > > example, which could be problematic if used in arbitrary contexts). > > > > As I've pointed out above, it seems like an oversight that Mark > > didn't make the WARN_ON_ONCE() conditional on handle_enforce_irqctx() > > (as handle_irq_desc() does). Sadly you did not respond to that > > observation. > > When did you make that observation? I can only see an email from you > being sent *after* the one I am replying to. I was referring to the above-quoted sentence: "generic_handle_domain_irq() warns unconditionally on !in_irq(), unlike handle_irq_desc(), which constrains the warning to handle_enforce_irqctx() (i.e. x86 APIC, arm GIC/GICv3). Perhaps that's an oversight in generic_handle_domain_irq(), unless __irq_resolve_mapping() becomes unsafe outside in_irq() for some reason..." Never mind, let's focus on the problem at hand. It's secondary who said what when. > > Please clarify whether that is indeed erroneous. > > Once handle_enforce_irqctx() is added to generic_handle_domain_irq(), > > there's no need for me to call __irq_enter_raw(). Problem solved. > > I don't see it as an oversight. Drivers shouldn't rely on > architectural quirks, and it is much clearer to simply forbid > something that cannot be guaranteed across the board, specially for > something that is as generic as USB. Whether a warning is warranted is not dependent on the architecture, but on the irqchip from which an interrupt normally originates: * Interrupt normally originates from x86 APIC or arm GIC/GICv3, but is synthesized in non-hardirq context: Warning is warranted. * Interrupt normally originates from any other top-level irqchip, such as irq-bcm2836.c, but is synthesized in non-hardirq context: Warning is a false positive! * Interrupt is always synthesized in non-hardirq context by a USB irqchip: Warning is a false positive, regardless whether the top-level irqchip is x86 APIC, arm GIC/GICv3 or anything else! > > Should there be a valid reason for the missing handle_enforce_irqctx(), > > then I propose adding a generic_handle_domain_irq_safe() function which > > calls __irq_enter_raw() (or probably __irq_enter() to get accounting), > > thereby resolving your objection to calling __irq_enter_raw() from a > > driver. > > Feel free to submit a patch. Done: https://lore.kernel.org/lkml/c3caf60bfa78e5fdbdf483096b7174da65d1813a.1652168866.git.lukas@wunner.de/ I'm focussing on eliminating the false-positive warning for now. Introducing a generic_handle_domain_irq_safe() wrapper which alleviates drivers from calling local_irq_save() can be done in a separate step. Thanks, Lukas