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 23420C433EF for ; Fri, 6 May 2022 10:58:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1391107AbiEFLCa (ORCPT ); Fri, 6 May 2022 07:02:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33442 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1391105AbiEFLC3 (ORCPT ); Fri, 6 May 2022 07:02:29 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 517E81F614; Fri, 6 May 2022 03:58:47 -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 dfw.source.kernel.org (Postfix) with ESMTPS id DD90461BB6; Fri, 6 May 2022 10:58:46 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 38CA5C385AE; Fri, 6 May 2022 10:58:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1651834726; bh=wGkjYK+Q3/CWC/Vo7gfKw8naKn70zntYWDbUwNfSiuM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=tVZoGX5fPJjH70QTpNeAjArJAayUKcJ09Js+S7NxoctUpKgibefflt6GaBjp+GOp/ g9ahxBYP73eMsMncs4j1NA+ERq8cB61LkoyCrjZ5mm1/OBByAOb7Tafu0azvL9ArOB qoS3tsehKaQhYmSC3ueB8NSLxspqYxG3wEQGbwnouWohYHdmsQKsNXFw4a9pSpHrIa wbGgK0fsWV5RvAb8OAxb91NBV73KO+cfWhV3qIOFAhlXh29fos9atargGnr/u07nJS +c5k9WJKiHMRfH53sqdgmJ/rvhN2Kd24kfyN+ivM1LXGgn2Ufqn3GWzppF+G7Bnqbv HjX7slXKnCoNw== 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.94.2) (envelope-from ) id 1nmvfX-009RKN-IV; Fri, 06 May 2022 11:58:43 +0100 Date: Fri, 06 May 2022 11:58:43 +0100 Message-ID: <87tua36i70.wl-maz@kernel.org> From: Marc Zyngier To: Lukas Wunner Cc: 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 In-Reply-To: <20220505185328.GA14123@wunner.de> References: <20220505113207.487861b2@kernel.org> <20220505185328.GA14123@wunner.de> 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: lukas@wunner.de, kuba@kernel.org, tglx@linutronix.de, davem@davemloft.net, pabeni@redhat.com, netdev@vger.kernel.org, linux-usb@vger.kernel.org, steve.glendinning@shawell.net, UNGLinuxDriver@microchip.com, oneukum@suse.com, andre.edich@microchip.com, linux@rempel-privat.de, martyn.welch@collabora.com, ghojda@yo2urs.ro, chf.fritz@googlemail.com, LinoSanfilippo@gmx.de, p.rosenberger@kunbus.com, hkallweit1@gmail.com, andrew@lunn.ch, linux@armlinux.org.uk, fntoth@gmail.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: netdev@vger.kernel.org On Thu, 05 May 2022 19:53:28 +0100, Lukas Wunner wrote: > > On Thu, May 05, 2022 at 11:32:07AM -0700, Jakub Kicinski wrote: > > On Tue, 3 May 2022 15:15:05 +0200 Lukas Wunner wrote: > > > @@ -608,11 +618,20 @@ static void smsc95xx_status(struct usbnet *dev, struct urb *urb) > > > intdata = get_unaligned_le32(urb->transfer_buffer); > > > netif_dbg(dev, link, dev->net, "intdata: 0x%08X\n", intdata); > > > > > > + /* USB interrupts are received in softirq (tasklet) context. > > > + * Switch to hardirq context to make genirq code happy. > > > + */ > > > + local_irq_save(flags); > > > + __irq_enter_raw(); > > > + > > > if (intdata & INT_ENP_PHY_INT_) > > > - ; > > > + generic_handle_domain_irq(pdata->irqdomain, PHY_HWIRQ); > > > else > > > netdev_warn(dev->net, "unexpected interrupt, intdata=0x%08X\n", > > > intdata); > > > + > > > + __irq_exit_raw(); > > > + local_irq_restore(flags); > > > > IRQ maintainers could you cast your eyes over this? > > > > Full patch: > > > > https://lore.kernel.org/all/c6b7f4e4a17913d2f2bc4fe722df0804c2d6fea7.1651574194.git.lukas@wunner.de/ > > This is basically identical to what drivers/net/usb/lan78xx.c does > in lan78xx_status(), except I'm passing the hw irq instead of the > linux irq to genirq code, thereby avoiding the overhead of a > radix_tree_lookup(). > > 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. > > And there's no _safe variant() of generic_handle_domain_irq() > (unlike generic_handle_irq_safe() which was recently added by > 509853f9e1e7), hence the necessity of an explicit local_irq_save(). 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). Given how infrequent this interrupt is, I'd rather you use something similar to what lan78xx is doing, and be done with it. And since this is a construct that seems to be often repeated, why don't you simply make the phy interrupt handling available over a smp_call_function() interface, which would always put you in the correct context and avoid faking things up? Thanks, M. -- Without deviation from the norm, progress is not possible.