Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230328142112.12493-1-kerneljasonxing@gmail.com>
In-Reply-To: <20230328142112.12493-1-kerneljasonxing@gmail.com>
From:   Eric Dumazet <edumazet@google.com>
Date:   Tue, 28 Mar 2023 16:33:06 +0200
Message-ID: <CANn89iLXA198djT-+NGSgTqTYA=cJtcvFmcoFbY59VewrqT0BA@mail.gmail.com>
Subject: Re: [PATCH v2 net] net: rps: avoid raising a softirq on the current
 cpu when scheduling napi
To:     Jason Xing <kerneljasonxing@gmail.com>
Cc:     davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Jason Xing <kernelxing@tencent.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Mar 28, 2023 at 4:21=E2=80=AFPM Jason Xing <kerneljasonxing@gmail.c=
om> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> When we are scheduling napi and then RPS decides to put the skb into
> a backlog queue of another cpu, we shouldn't raise the softirq for
> the current cpu. When to raise a softirq is based on whether we have
> more data left to process later. But apparently, as to the current
> cpu, there is no indication of more data enqueued, so we do not need
> this action. After enqueuing to another cpu, net_rx_action() or
> process_backlog() will call ipi and then another cpu will raise the
> softirq as expected.
>
> Also, raising more softirqs which set the corresponding bit field
> can make the IRQ mechanism think we probably need to start ksoftirqd
> on the current cpu. Actually it shouldn't happen.
>
> Here are some codes to clarify how it can trigger ksoftirqd:
> __do_softirq()
>   [1] net_rx_action() -> enqueue_to_backlog() -> raise an IRQ
>   [2] check if pending is set again -> wakeup_softirqd
>
> Comments on above:
> [1] when RPS chooses another cpu to enqueue skb
> [2] in __do_softirq() it will wait a little bit of time around 2 jiffies
>
> In this patch, raising an IRQ can be avoided when RPS enqueues the skb
> into another backlog queue not the current one.
>
> I captured some data when starting one iperf3 process and found out
> we can reduces around ~1500 times/sec at least calling
> __raise_softirq_irqoff().
>
> Fixes: 0a9627f2649a ("rps: Receive Packet Steering")

No Fixes: tag, when you are trying to optimize things, and so far fail at t=
his.

> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> v2:
> 1) change the title and add more details.
> 2) add one parameter to recognise whether it is napi or non-napi case
> suggested by Eric.
> Link: https://lore.kernel.org/lkml/20230325152417.5403-1-kerneljasonxing@=
gmail.com/
> ---

Wrong again.

I think I will send a series, instead of you trying so hard to break the st=
ack.

You have not considered busy polling, and that netif_receive_skb() contract
does not enforce it to be called from net_rx_action().