[PATCH][v2] xfrm: replace NR_CPU with nr_cpu_ids

[PATCH][v2] xfrm: replace NR_CPU with nr_cpu_ids

[Li RongQing]

The default NR_CPUS can be very large, but actual possible nr_cpu_ids
usually is very small. For some x86 distribution, the NR_CPUS is 8192
and nr_cpu_ids is 4, so replace NR_CPU to save some memory

Signed-off-by: Li RongQing <lirongqing@baidu.com>
Signed-off-by: Wang Li <wangli39@baidu.com>
---
 net/xfrm/xfrm_policy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 40b54cc64243..f8188685c1e9 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2989,11 +2989,11 @@ void __init xfrm_init(void)
 {
 	int i;
 
-	xfrm_pcpu_work = kmalloc_array(NR_CPUS, sizeof(*xfrm_pcpu_work),
+	xfrm_pcpu_work = kmalloc_array(nr_cpu_ids, sizeof(*xfrm_pcpu_work),
 				       GFP_KERNEL);
 	BUG_ON(!xfrm_pcpu_work);
 
-	for (i = 0; i < NR_CPUS; i++)
+	for (i = 0; i < nr_cpu_ids; i++)
 		INIT_WORK(&xfrm_pcpu_work[i], xfrm_pcpu_work_fn);
 
 	register_pernet_subsys(&xfrm_net_ops);
-- 
2.16.2

Re: [PATCH][v2] xfrm: replace NR_CPU with nr_cpu_ids

[Yunsheng Lin]

On 2018/6/19 15:11, Li RongQing wrote:
> The default NR_CPUS can be very large, but actual possible nr_cpu_ids
> usually is very small. For some x86 distribution, the NR_CPUS is 8192
> and nr_cpu_ids is 4, so replace NR_CPU to save some memory
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> Signed-off-by: Wang Li <wangli39@baidu.com>
> ---
>  net/xfrm/xfrm_policy.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 40b54cc64243..f8188685c1e9 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -2989,11 +2989,11 @@ void __init xfrm_init(void)
>  {
>  	int i;
>  
> -	xfrm_pcpu_work = kmalloc_array(NR_CPUS, sizeof(*xfrm_pcpu_work),
> +	xfrm_pcpu_work = kmalloc_array(nr_cpu_ids, sizeof(*xfrm_pcpu_work),
>  				       GFP_KERNEL);

It seems that xfrm_pcpu_work is not used anymore, maybe it can be deleted to
save more memory.


>  	BUG_ON(!xfrm_pcpu_work);
>  
> -	for (i = 0; i < NR_CPUS; i++)
> +	for (i = 0; i < nr_cpu_ids; i++)
>  		INIT_WORK(&xfrm_pcpu_work[i], xfrm_pcpu_work_fn);
>  
>  	register_pernet_subsys(&xfrm_net_ops);
> 

Re: [PATCH][v2] xfrm: replace NR_CPU with nr_cpu_ids

[Florian Westphal]

Li RongQing <lirongqing@baidu.com> wrote:
> The default NR_CPUS can be very large, but actual possible nr_cpu_ids
> usually is very small. For some x86 distribution, the NR_CPUS is 8192
> and nr_cpu_ids is 4, so replace NR_CPU to save some memory

Steffen,

I will soon submit a patch to remove the percpu cache; removal
improved performance for at least one user (and by quite a sizeable
amount).

Would you consider such removal for ipsec or ipsec-next?
If -next, consider applying this patch for ipsec.git.

Otherwise consider not applying this, as the code will go away soon.


Thanks,
Florian

Re: [PATCH][v2] xfrm: replace NR_CPU with nr_cpu_ids

[Steffen Klassert]

On Tue, Jun 19, 2018 at 09:53:49AM +0200, Florian Westphal wrote:
> Li RongQing <lirongqing@baidu.com> wrote:
> > The default NR_CPUS can be very large, but actual possible nr_cpu_ids
> > usually is very small. For some x86 distribution, the NR_CPUS is 8192
> > and nr_cpu_ids is 4, so replace NR_CPU to save some memory
> 
> Steffen,
> 
> I will soon submit a patch to remove the percpu cache; removal
> improved performance for at least one user (and by quite a sizeable
> amount).
> 
> Would you consider such removal for ipsec or ipsec-next?

I think this removel would better fit to ipsec-next.

> If -next, consider applying this patch for ipsec.git.
> 
> Otherwise consider not applying this, as the code will go away soon.

This patch more an optimization than a fix, so I
considered to apply it to ipsec-next. If you plan
to remove it, I'll wait for that.

Re: [PATCH][v2] xfrm: replace NR_CPU with nr_cpu_ids

[Florian Westphal]

Steffen Klassert <steffen.klassert@secunet.com> wrote:
> On Tue, Jun 19, 2018 at 09:53:49AM +0200, Florian Westphal wrote:
> > Li RongQing <lirongqing@baidu.com> wrote:
> > > The default NR_CPUS can be very large, but actual possible nr_cpu_ids
> > > usually is very small. For some x86 distribution, the NR_CPUS is 8192
> > > and nr_cpu_ids is 4, so replace NR_CPU to save some memory
> > 
> > Steffen,
> > 
> > I will soon submit a patch to remove the percpu cache; removal
> > improved performance for at least one user (and by quite a sizeable
> > amount).
> > 
> > Would you consider such removal for ipsec or ipsec-next?
> 
> I think this removel would better fit to ipsec-next.

Agree, it slows things down further for me in my tests.
Problem is that I get quite good re-use of pcpu cache due to
unidirectional flows and only one tunnel.

I suspect that even with tunnel the removal is a win in practice
though, netperf is quite artifical, so I rather trust Kristians results
(real world) than my own.

> considered to apply it to ipsec-next. If you plan
> to remove it, I'll wait for that.

I'll submit once net-next opens.