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 9BED8C433FE for ; Thu, 3 Feb 2022 12:41:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1350566AbiBCMlP (ORCPT ); Thu, 3 Feb 2022 07:41:15 -0500 Received: from mail.toke.dk ([45.145.95.12]:60029 "EHLO mail.toke.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230213AbiBCMlP (ORCPT ); Thu, 3 Feb 2022 07:41:15 -0500 From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1643892073; bh=zr/1jJJt+Aahk+VsboUVd75mq76qnpf+aAXAJsfCyh8=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=JsEdTy78l9T8WVG5JOODe0KJdItQyCmk7SloiR4DiH03gEvE8jiHzr4od6Islk56S 22cs3BdcTjXI+smtRP+n+o7bZ+VmOAsWSejcIey/6kCDg064NJFa3iHFbOeqEhTGPX QHm314i2KLChaU+CVpIrDer/roHKkEzL/VPh/KbMke4rJHK+7/FtpLh5bTiCMPwsV5 nTzh73xwkM3ZW4BMys5L6b0aYudp1cwQdVcMq6nA7RXuUX7Z1x8cLQkTSd2yAC2SRN 3JsTdJwazW6yHNkMYU45TB/0knlpoq9SoG2Cv88I8dKiHqG5RKRNcA7jHj7g26IAPq TZy+0MYj2EDhA== To: Sebastian Andrzej Siewior Cc: Eric Dumazet , bpf , netdev , "David S. Miller" , Alexei Starovoitov , Daniel Borkmann , Jakub Kicinski , Jesper Dangaard Brouer , John Fastabend , Thomas Gleixner Subject: Re: [PATCH net-next 1/4] net: dev: Remove the preempt_disable() in netif_rx_internal(). In-Reply-To: References: <20220202122848.647635-1-bigeasy@linutronix.de> <20220202122848.647635-2-bigeasy@linutronix.de> <87v8xwb1o9.fsf@toke.dk> Date: Thu, 03 Feb 2022 13:41:13 +0100 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87leysazrq.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Sebastian Andrzej Siewior writes: > On 2022-02-03 13:00:06 [+0100], Toke H=C3=B8iland-J=C3=B8rgensen wrote: >> > Here is the code in larger context: >> > >> > #ifdef CONFIG_RPS >> > if (static_branch_unlikely(&rps_needed)) { >> > struct rps_dev_flow voidflow, *rflow =3D &voidflow; >> > int cpu; >> > >> > preempt_disable(); >> > rcu_read_lock(); >> > >> > cpu =3D get_rps_cpu(skb->dev, skb, &rflow); >> > if (cpu < 0) >> > cpu =3D smp_processor_id(); >> > >> > ret =3D enqueue_to_backlog(skb, cpu, &rflow->last_qtail); >> > >> > rcu_read_unlock(); >> > preempt_enable(); >> > } else >> > #endif >> > >> > This code needs the preempt_disable(). >>=20 >> This is mostly so that the CPU ID stays the same throughout that section >> of code, though, right? So wouldn't it work to replace the >> preempt_disable() with a migrate_disable()? That should keep _RT happy, >> no? > > It would but as mentioned previously: BH is disabled and > smp_processor_id() is stable. Ah, right, because of the change in loopback to use netif_rx_ni()? But that bit of the analysis only comes later in your series, so at the very least you should be explaining this in the commit message here. Or you could potentially squash patches 1 and 2 and do both changes at once, since it's changing two bits of the same function and both need the same analysis... However, if we're going with Eric's suggestion of an internal __netif_rx() for loopback that *doesn't* do local_bh_disable() then this code would end up being called without BH disable, so we'd need the migrate_disable() anyway, no? -Toke