2008-06-02 10:41:37

by Jeff Layton

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

On Mon, 02 Jun 2008 00:51:50 -0500
Greg Banks <gnb-cP1dWloDopni96+mSzHFpQC/[email protected]> wrote:

> Jeff Layton wrote:
> >
> > 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?
> > [...]
> >
> > - rqstp = kzalloc(sizeof(*rqstp), GFP_KERNEL);
> > + rqstp = kmalloc_node(sizeof(*rqstp), GFP_KERNEL | __GFP_ZERO, node);
> > if (!rqstp)
> > goto out_enomem;
> >
> I'm not an expert, but from looking at the slab code I don't think
> __GFP_ZERO works the way you're expecting on the kmalloc() family, only
> on the alloc_pages() family. In particular it's not used when the
> object is allocated on the slab fastpath. kzalloc() seems to be a
> kmalloc() followed by a memset(). Perhaps you'd be better off either
> adding a kzalloc_node() or doing kmalloc_node() plus memset().
>

I'm certainly no VM expert either, but where are you seeing the
kmalloc + memset? The only definition I see is the inlined function
in slab.h which is:

static inline void *kzalloc(size_t size, gfp_t flags)
{
return kmalloc(size, flags | __GFP_ZERO);
}

Still though, you're probably right. A kzalloc_node() might be handy for
other uses. I'll consider it once I unravel what kzalloc actually
does...

--
Jeff Layton <[email protected]>