From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-02.galae.net (smtpout-02.galae.net [185.246.84.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A6C5B3D3321; Mon, 16 Mar 2026 18:11:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.84.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773684701; cv=none; b=bM55y3L/Yr57PuFXuHSv4apb5yfZN2wUPxv4j5QfSxmVpsaG6WLAInj1FYI7b5ZitDl+UlQzF31aGI7FGb4Yo8HZmtG4RCXn05P5/YxgPIk1/mHVc88VG11biIhd+ss+XitmyvQK3c9ktVERVxID7iJzQr8ACQLK2sqKIYI8JS8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773684701; c=relaxed/simple; bh=6m73jU4PfOdCnf89AKcCpnsbYhuH3jWd6E6INcRIDeI=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=Edtl28HhqinePc1nDtXVwqua6QR7GCreHL3tBrq8b1lUX9wJa2L8qrigj5TeEyGuoLF6X0jVrBIQVX6wUJLXGLMuoJ+YUFGAnkmlHDia47itMtrhMOTvC/U1jnYTuWU2FagQgTXi5sA/Ffl3fk5ASX8N82CydMZMv8UUlL1xnl0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=f41kVzn3; arc=none smtp.client-ip=185.246.84.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="f41kVzn3" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-02.galae.net (Postfix) with ESMTPS id 2B3841A2DC3; Mon, 16 Mar 2026 18:11:38 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id E9E775FC4A; Mon, 16 Mar 2026 18:11:37 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 40BBC10450463; Mon, 16 Mar 2026 19:11:35 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1773684697; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=ThhfLgqlr7YbjbLdrOOaAW5n9JSkqrHDYXd5hwsDx8g=; b=f41kVzn32iqLCGmegXZOD0HcLCss7l+66+LBM9QtsOIGWOATyNnaBi9Wl9vsescv5hdX8H SZ8Ys67fEFqlwMv9DY5ekFZPgRCR4oJfHcIINOL+Xh5GyuntR8mnQ+BOfbpb/VTmXW93TI WvliyXVEigW9cEWZQgZjMd/7jmB7fieVDg76gqyIZATeweXRrsZmGDiXt9RFkhd1tXiJfJ NOtYnhN4w2OXSdS8x5CwKffiig90avQPBslLloP9963NjE5e3GXePNaLrT2XwB8sA7xDyp 1shhJiAOI0Ovub9yoghXungDFONns3nJU9HYQbM0meK++hTk6BMFiTu7EoUJog== Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 16 Mar 2026 19:11:34 +0100 Message-Id: Subject: Re: [PATCH net 1/2] net: macb: Move devm_{free,request}_irq() out of spin lock area Cc: "Nicolas Ferre" , "Claudiu Beznea" , "Andrew Lunn" , "David S. Miller" , "Eric Dumazet" , "Jakub Kicinski" , "Paolo Abeni" , "Vineeth Karumanchi" , "Harini Katakam" , To: "Kevin Hao" , From: =?utf-8?q?Th=C3=A9o_Lebrun?= X-Mailer: aerc 0.21.0-0-g5549850facc2 References: <20260315-macb-irq-v1-0-0154104cbf61@gmail.com> <20260315-macb-irq-v1-1-0154104cbf61@gmail.com> In-Reply-To: <20260315-macb-irq-v1-1-0154104cbf61@gmail.com> X-Last-TLS-Session-Version: TLSv1.3 On Sun Mar 15, 2026 at 12:44 PM CET, Kevin Hao wrote: > @@ -5962,6 +5962,7 @@ static int __maybe_unused macb_suspend(struct devic= e *dev) > /* write IP address into register */ > tmp |=3D MACB_BFEXT(IP, be32_to_cpu(ifa->ifa_local)); > } > + spin_unlock_irqrestore(&bp->lock, flags); > =20 > /* Change interrupt handler and > * Enable WoL IRQ on queue 0 > @@ -5974,11 +5975,12 @@ static int __maybe_unused macb_suspend(struct dev= ice *dev) > dev_err(dev, > "Unable to request IRQ %d (error %d)\n", > bp->queues[0].irq, err); > - spin_unlock_irqrestore(&bp->lock, flags); > return err; > } > + spin_lock_irqsave(&bp->lock, flags); > queue_writel(bp->queues, IER, GEM_BIT(WOL)); > gem_writel(bp, WOL, tmp); > + spin_unlock_irqrestore(&bp->lock, flags); > } else { > err =3D devm_request_irq(dev, bp->queues[0].irq, macb_wol_interrupt, > IRQF_SHARED, netdev->name, bp->queues); > @@ -5986,13 +5988,13 @@ static int __maybe_unused macb_suspend(struct dev= ice *dev) > dev_err(dev, > "Unable to request IRQ %d (error %d)\n", > bp->queues[0].irq, err); > - spin_unlock_irqrestore(&bp->lock, flags); > return err; > } > + spin_lock_irqsave(&bp->lock, flags); > queue_writel(bp->queues, IER, MACB_BIT(WOL)); > macb_writel(bp, WOL, tmp); > + spin_unlock_irqrestore(&bp->lock, flags); > } > - spin_unlock_irqrestore(&bp->lock, flags); > =20 > enable_irq_wake(bp->queues[0].irq); > } So it used to be that approximatively the whole macb_suspend() function was ran under the bp->lock spinlock. Now you split it in two to avoid calling IRQ functions in atomic context: - (1) the disable queues & silence IRQs part and, - (2) the enable WOL part (IER and WOL reg writes). Why do you need to grab bp->lock for the 2nd part? All queues are disabled anyway and IRQs masked. BH features like our work queues are disabled during the dev_pm_ops.suspend() calls anyway. Maybe I am forgetting? Or this was just out of caution? Thanks, -- Th=C3=A9o Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com