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;
Date:   Fri, 26 May 2023 08:45:31 +0200
From:   Horatiu Vultur <horatiu.vultur@microchip.com>
To:     Haiyang Zhang <haiyangz@microsoft.com>
CC:     "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        Dexuan Cui <decui@microsoft.com>,
        KY Srinivasan <kys@microsoft.com>,
        Paul Rosswurm <paulros@microsoft.com>,
        "olaf@aepfle.de" <olaf@aepfle.de>,
        "vkuznets@redhat.com" <vkuznets@redhat.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "wei.liu@kernel.org" <wei.liu@kernel.org>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "leon@kernel.org" <leon@kernel.org>,
        Long Li <longli@microsoft.com>,
        "ssengar@linux.microsoft.com" <ssengar@linux.microsoft.com>,
        "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
        "daniel@iogearbox.net" <daniel@iogearbox.net>,
        "john.fastabend@gmail.com" <john.fastabend@gmail.com>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
        "ast@kernel.org" <ast@kernel.org>,
        Ajay Sharma <sharmaajay@microsoft.com>,
        "hawk@kernel.org" <hawk@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH net] net: mana: Fix perf regression: remove rx_cqes,
 tx_cqes counters
Message-ID: <20230526064531.zohcgjbaraq7c2ui@soft-dev3-1>
References: <1684963320-25282-1-git-send-email-haiyangz@microsoft.com>
 <20230525064849.ca5p6npej7p2luw2@soft-dev3-1>
 <PH7PR21MB31161F3291FF951877355DA9CA46A@PH7PR21MB3116.namprd21.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <PH7PR21MB31161F3291FF951877355DA9CA46A@PH7PR21MB3116.namprd21.prod.outlook.com>
Precedence: bulk

The 05/25/2023 14:34, Haiyang Zhang wrote:
> 
> > -----Original Message-----
> > From: Horatiu Vultur <horatiu.vultur@microchip.com>
> > Sent: Thursday, May 25, 2023 2:49 AM
> > To: Haiyang Zhang <haiyangz@microsoft.com>
> > Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; Dexuan Cui
> > <decui@microsoft.com>; KY Srinivasan <kys@microsoft.com>; Paul Rosswurm
> > <paulros@microsoft.com>; olaf@aepfle.de; vkuznets@redhat.com;
> > davem@davemloft.net; wei.liu@kernel.org; edumazet@google.com;
> > kuba@kernel.org; pabeni@redhat.com; leon@kernel.org; Long Li
> > <longli@microsoft.com>; ssengar@linux.microsoft.com; linux-
> > rdma@vger.kernel.org; daniel@iogearbox.net; john.fastabend@gmail.com;
> > bpf@vger.kernel.org; ast@kernel.org; Ajay Sharma
> > <sharmaajay@microsoft.com>; hawk@kernel.org; linux-
> > kernel@vger.kernel.org; stable@vger.kernel.org
> > Subject: Re: [PATCH net] net: mana: Fix perf regression: remove rx_cqes,
> > tx_cqes counters
> >
> > [Some people who received this message don't often get email from
> > horatiu.vultur@microchip.com. Learn why this is important at
> > https://aka.ms/LearnAboutSenderIdentification ]
> >
> > The 05/24/2023 14:22, Haiyang Zhang wrote:
> >
> > Hi Haiyang,
> >
> > >
> > > The apc->eth_stats.rx_cqes is one per NIC (vport), and it's on the
> > > frequent and parallel code path of all queues. So, r/w into this
> > > single shared variable by many threads on different CPUs creates a
> > > lot caching and memory overhead, hence perf regression. And, it's
> > > not accurate due to the high volume concurrent r/w.
> >
> > Do you have any numbers to show the improvement of this change?
> 
> The numbers are not published. The perf regression of the previous
> patch is very significant, and this patch eliminates the regression.
> 
> >
> > >
> > > Since the error path of mana_poll_rx_cq() already has warnings, so
> > > keeping the counter and convert it to a per-queue variable is not
> > > necessary. So, just remove this counter from this high frequency
> > > code path.
> > >
> > > Also, remove the tx_cqes counter for the same reason. We have
> > > warnings & other counters for errors on that path, and don't need
> > > to count every normal cqe processing.
> >
> > Will you not have problems with the counter 'apc->eth_stats.tx_cqe_err'?
> > It is not in the hot path but you will have concurrent access to it.
> 
> Yes, but that error happens rarely, so a shared variable is good enough. So, I
> don't change it in this patch.

OK, I understand.
Maybe this can be fixed in a different patch at a later point. Thanks.

Reviwed-by: Horatiu Vultur <horatiu.vultur@microchip.com>

> 
> Thanks,
> - Haiyang
> 

-- 
/Horatiu