linux-kernel - Re: [PATCH V5 net-next] net: mana: Assigning IRQ affinity on HT cores

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231212060342.GA16802@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date:   Mon, 11 Dec 2023 22:03:42 -0800
From:   Souradeep Chakrabarti <schakrabarti@...ux.microsoft.com>
To:     Yury Norov <yury.norov@...il.com>
Cc:     kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
        decui@...rosoft.com, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, longli@...rosoft.com,
        leon@...nel.org, cai.huoqing@...ux.dev,
        ssengar@...ux.microsoft.com, vkuznets@...hat.com,
        tglx@...utronix.de, linux-hyperv@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-rdma@...r.kernel.org, schakrabarti@...rosoft.com,
        paulros@...rosoft.com
Subject: Re: [PATCH V5 net-next] net: mana: Assigning IRQ affinity on HT cores

On Mon, Dec 11, 2023 at 06:00:22AM -0800, Yury Norov wrote:
> On Sun, Dec 10, 2023 at 10:53:23PM -0800, Souradeep Chakrabarti wrote:
> > On Fri, Dec 08, 2023 at 01:53:51PM -0800, Yury Norov wrote:
> > > Few more nits
> > > 
> > > On Fri, Dec 08, 2023 at 06:03:40AM -0800, Yury Norov wrote:
> > > > On Fri, Dec 08, 2023 at 02:02:34AM -0800, Souradeep Chakrabarti wrote:
> > > > > Existing MANA design assigns IRQ to every CPU, including sibling
> > > > > hyper-threads. This may cause multiple IRQs to be active simultaneously
> > > > > in the same core and may reduce the network performance with RSS.
> > > > 
> > > > Can you add an IRQ distribution diagram to compare before/after
> > > > behavior, similarly to what I did in the other email?
> > > > 
> > > > > Improve the performance by assigning IRQ to non sibling CPUs in local
> > > > > NUMA node. The performance improvement we are getting using ntttcp with
> > > > > following patch is around 15 percent with existing design and approximately
> > > > > 11 percent, when trying to assign one IRQ in each core across NUMA nodes,
> > > > > if enough cores are present.
> > > > 
> > > > How did you measure it? In the other email you said you used perf, can
> > > > you show your procedure in details?
> > > > 
> > > > > Suggested-by: Yury Norov <yury.norov@...li.com>
> > > > > Signed-off-by: Souradeep Chakrabarti <schakrabarti@...ux.microsoft.com>
> > > > > ---
> > > > 
> > > > [...]
> > > > 
> > > > >  .../net/ethernet/microsoft/mana/gdma_main.c   | 92 +++++++++++++++++--
> > > > >  1 file changed, 83 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > > > > index 6367de0c2c2e..18e8908c5d29 100644
> > > > > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > > > > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > > > > @@ -1243,15 +1243,56 @@ void mana_gd_free_res_map(struct gdma_resource *r)
> > > > >  	r->size = 0;
> > > > >  }
> > > > >  
> > > > > +static int irq_setup(int *irqs, int nvec, int start_numa_node)
> > > > > +{
> > > > > +	int w, cnt, cpu, err = 0, i = 0;
> > > > > +	int next_node = start_numa_node;
> > > > 
> > > > What for this?
> > > > 
> > > > > +	const struct cpumask *next, *prev = cpu_none_mask;
> > > > > +	cpumask_var_t curr, cpus;
> > > > > +
> > > > > +	if (!zalloc_cpumask_var(&curr, GFP_KERNEL)) {
> > > 
> > > alloc_cpumask_var() here and below, because you initialize them by
> > > copying
> > I have used zalloc here as prev gets initialized after the first hop, before that
> > it may contain unwanted values, which may impact cpumask_andnot(curr, next, prev).
> > Regarding curr I will change it to alloc_cpumask_var().
> > Please let me know if that sounds right.
> 
> What? prev is initialized at declaration:
Yes, I will remove the zalloc and will change it to alloc in V6.
Thanks for pointing.
>         
>         const struct cpumask *next, *prev = cpu_none_mask;