Re: [PATCH net-next] net/smc: Allocate pages of SMC-R on ibdev NUMA node

[Leon Romanovsky <leon@kernel.org>]

On Mon, Jan 31, 2022 at 03:03:00AM +0800, Tony Lu wrote:
> Currently, pages are allocated in the process context, for its NUMA node
> isn't equal to ibdev's, which is not the best policy for performance.
> 
> Applications will generally perform best when the processes are
> accessing memory on the same NUMA node. When numa_balancing enabled
> (which is enabled by most of OS distributions), it moves tasks closer to
> the memory of sndbuf or rmb and ibdev, meanwhile, the IRQs of ibdev bind
> to the same node usually. This reduces the latency when accessing remote
> memory.

It is very subjective per-specific test. I would expect that
application will control NUMA memory policies (set_mempolicy(), ...)
by itself without kernel setting NUMA node.

Various *_alloc_node() APIs are applicable for in-kernel allocations
where user can't control memory policy.

I don't know SMC-R enough, but if I judge from your description, this
allocation is controlled by the application.

Thanks

> 
> According to our tests in different scenarios, there has up to 15.30%
> performance drop (Redis benchmark) when accessing remote memory.
> 
> Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
> ---
>  net/smc/smc_core.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 8935ef4811b0..2a28b045edfa 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -2065,9 +2065,10 @@ int smcr_buf_reg_lgr(struct smc_link *lnk)
>  	return rc;
>  }
>  
> -static struct smc_buf_desc *smcr_new_buf_create(struct smc_link_group *lgr,
> +static struct smc_buf_desc *smcr_new_buf_create(struct smc_connection *conn,
>  						bool is_rmb, int bufsize)
>  {
> +	int node = ibdev_to_node(conn->lnk->smcibdev->ibdev);
>  	struct smc_buf_desc *buf_desc;
>  
>  	/* try to alloc a new buffer */
> @@ -2076,10 +2077,10 @@ static struct smc_buf_desc *smcr_new_buf_create(struct smc_link_group *lgr,
>  		return ERR_PTR(-ENOMEM);
>  
>  	buf_desc->order = get_order(bufsize);
> -	buf_desc->pages = alloc_pages(GFP_KERNEL | __GFP_NOWARN |
> -				      __GFP_NOMEMALLOC | __GFP_COMP |
> -				      __GFP_NORETRY | __GFP_ZERO,
> -				      buf_desc->order);
> +	buf_desc->pages = alloc_pages_node(node, GFP_KERNEL | __GFP_NOWARN |
> +					   __GFP_NOMEMALLOC | __GFP_COMP |
> +					   __GFP_NORETRY | __GFP_ZERO,
> +					   buf_desc->order);
>  	if (!buf_desc->pages) {
>  		kfree(buf_desc);
>  		return ERR_PTR(-EAGAIN);
> @@ -2190,7 +2191,7 @@ static int __smc_buf_create(struct smc_sock *smc, bool is_smcd, bool is_rmb)
>  		if (is_smcd)
>  			buf_desc = smcd_new_buf_create(lgr, is_rmb, bufsize);
>  		else
> -			buf_desc = smcr_new_buf_create(lgr, is_rmb, bufsize);
> +			buf_desc = smcr_new_buf_create(conn, is_rmb, bufsize);
>  
>  		if (PTR_ERR(buf_desc) == -ENOMEM)
>  			break;
> -- 
> 2.32.0.3.g01195cf9f
>