Re: [PATCH 0/3] [RFC] knfsd: convert to kthread API and remove signaling for shutdown

["J. Bruce Fields" <bfields@fieldses.org>]

On Fri, May 30, 2008 at 12:25:17PM -0400, Jeff Layton wrote:
> On Tue, 20 May 2008 20:29:52 -0700
> Greg Banks <gnb-cP1dWloDopni96+mSzHFpQC/G2K4zDHf@public.gmane.org> wrote:
> 
> > Jeff Layton wrote:
> > >   
> > > [...] Rather than delaying the allocation like this,
> > > I wonder if we can just figure out the node from the cpumask and then
> > > do a kmalloc_node()?
> > >
> > >   
> > Sure.
> > 
> 
> I started to do this patch as part of the kthread conversion, but it's
> actually pretty independent. We can probably treat it separately.
> This hasn't been tested on an actual NUMA machine yet, but any thoughts
> on the following patch?
> 
> -----------------[snip]-------------------
> 
> >From 46432a021fe1931f4a44587124ac9442e83c4731 Mon Sep 17 00:00:00 2001
> From: Jeff Layton <jlayton@redhat.com>
> Date: Fri, 30 May 2008 11:57:18 -0400
> Subject: [PATCH] sunrpc: have pooled services make NUMA-friendly allocations
> 
> Currently, svc_prepare_thread allocates memory using plain kmalloc()
> and alloc_page() calls, even for threads that are destined to run on
> different CPUs or NUMA nodes than the current one. Add a function to
> translate a poolid into a NUMA node, and have svc_prepare_thread and
> svc_init_buffer allocate memory on those nodes instead.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  net/sunrpc/svc.c |   46 ++++++++++++++++++++++++++++++++++++++++------
>  1 files changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 01c7e31..3985fbc 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -332,6 +332,32 @@ svc_pool_map_set_cpumask(unsigned int pidx, cpumask_t *oldmask)
>  }
>  
>  /*
> + * for a given poolid, return the NUMA memory node. This allows us to
> + * allocate memory close to the CPU's where the task will be running
> + */
> +static inline unsigned int
> +svc_pool_to_node(unsigned int pidx)
> +{
> +	struct svc_pool_map *m = &svc_pool_map;
> +	unsigned int poolnode = m->pool_to[pidx];
> +
> +	/*
> +	 * The caller checks for sv_nrpools > 1, which
> +	 * implies that we've been initialized.
> +	 */
> +	BUG_ON(m->count == 0);
> +
> +	switch (m->mode) {
> +	case SVC_POOL_PERNODE:
> +		return poolnode;
> +	case SVC_POOL_PERCPU:
> +		return cpu_to_node(poolnode);
> +	}
> +
> +	return numa_node_id();
> +}
> +
> +/*
>   * Use the mapping mode to choose a pool for a given CPU.
>   * Used when enqueueing an incoming RPC.  Always returns
>   * a non-NULL pool pointer.
> @@ -507,7 +533,7 @@ EXPORT_SYMBOL(svc_destroy);
>   * We allocate pages and place them in rq_argpages.
>   */
>  static int
> -svc_init_buffer(struct svc_rqst *rqstp, unsigned int size)
> +svc_init_buffer(struct svc_rqst *rqstp, unsigned int size, unsigned int node)
>  {
>  	unsigned int pages, arghi;
>  
> @@ -517,7 +543,7 @@ svc_init_buffer(struct svc_rqst *rqstp, unsigned int size)
>  	arghi = 0;
>  	BUG_ON(pages > RPCSVC_MAXPAGES);
>  	while (pages) {
> -		struct page *p = alloc_page(GFP_KERNEL);
> +		struct page *p = alloc_pages_node(node, GFP_KERNEL, 0);
>  		if (!p)
>  			break;
>  		rqstp->rq_pages[arghi++] = p;
> @@ -543,8 +569,14 @@ struct svc_rqst *
>  svc_prepare_thread(struct svc_serv *serv, struct svc_pool *pool)
>  {
>  	struct svc_rqst	*rqstp;
> +	unsigned int	node;
> +
> +	if (serv->sv_nrpools > 1)
> +		node = svc_pool_to_node(pool->sp_id);
> +	else
> +		node = numa_node_id();
>  
> -	rqstp = kzalloc(sizeof(*rqstp), GFP_KERNEL);
> +	rqstp = kmalloc_node(sizeof(*rqstp), GFP_KERNEL | __GFP_ZERO, node);
>  	if (!rqstp)
>  		goto out_enomem;
>  
> @@ -558,15 +590,17 @@ svc_prepare_thread(struct svc_serv *serv, struct svc_pool *pool)
>  	rqstp->rq_server = serv;
>  	rqstp->rq_pool = pool;
>  
> -	rqstp->rq_argp = kmalloc(serv->sv_xdrsize, GFP_KERNEL);
> +	rqstp->rq_argp = kmalloc_node(serv->sv_xdrsize,
> +					GFP_KERNEL | __GFP_ZERO, node);
>  	if (!rqstp->rq_argp)
>  		goto out_thread;
>  
> -	rqstp->rq_resp = kmalloc(serv->sv_xdrsize, GFP_KERNEL);
> +	rqstp->rq_resp = kmalloc_node(serv->sv_xdrsize,
> +					GFP_KERNEL | __GFP_ZERO, node);

Why the __GFP_ZERO's on these last two?

--b.

>  	if (!rqstp->rq_resp)
>  		goto out_thread;
>  
> -	if (!svc_init_buffer(rqstp, serv->sv_max_mesg))
> +	if (!svc_init_buffer(rqstp, serv->sv_max_mesg, node))
>  		goto out_thread;
>  
>  	return rqstp;
> -- 
> 1.5.3.6
> 
> _______________________________________________
> NFSv4 mailing list
> NFSv4@linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4