It should improve performance in some scenarii where a lot of
these nsproxy objects are created by unsharing namespaces. This is
a typical use of virtual servers that are being created or entered.
This is also a good tool to find leaks and gather statistics on
namespace usage.
Signed-off-by: Cedric Le Goater <[email protected]>
---
kernel/nsproxy.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
Index: 2.6.22-rc4-mm2/kernel/nsproxy.c
===================================================================
--- 2.6.22-rc4-mm2.orig/kernel/nsproxy.c
+++ 2.6.22-rc4-mm2/kernel/nsproxy.c
@@ -21,6 +21,8 @@
#include <linux/utsname.h>
#include <linux/pid_namespace.h>
+static struct kmem_cache *nsproxy_cachep;
+
struct nsproxy init_nsproxy = INIT_NSPROXY(init_nsproxy);
static inline void get_nsproxy(struct nsproxy *ns)
@@ -43,9 +45,11 @@ static inline struct nsproxy *clone_nspr
{
struct nsproxy *ns;
- ns = kmemdup(orig, sizeof(struct nsproxy), GFP_KERNEL);
- if (ns)
+ ns = kmem_cache_alloc(nsproxy_cachep, GFP_KERNEL);
+ if (ns) {
+ memcpy(ns, orig, sizeof(struct nsproxy));
atomic_set(&ns->count, 1);
+ }
return ns;
}
@@ -109,7 +113,7 @@ out_uts:
if (new_nsp->mnt_ns)
put_mnt_ns(new_nsp->mnt_ns);
out_ns:
- kfree(new_nsp);
+ kmem_cache_free(nsproxy_cachep, new_nsp);
return ERR_PTR(err);
}
@@ -160,7 +164,7 @@ void free_nsproxy(struct nsproxy *ns)
put_pid_ns(ns->pid_ns);
if (ns->user_ns)
put_user_ns(ns->user_ns);
- kfree(ns);
+ kmem_cache_free(nsproxy_cachep, ns);
}
/*
@@ -191,3 +195,12 @@ int unshare_nsproxy_namespaces(unsigned
}
return err;
}
+
+static int __init nsproxy_cache_init(void)
+{
+ nsproxy_cachep = kmem_cache_create("nsproxy", sizeof(struct nsproxy),
+ 0, SLAB_PANIC, NULL, NULL);
+ return 0;
+}
+
+module_init(nsproxy_cache_init);
On Mon, 18 Jun 2007 22:53:13 +0200
Cedric Le Goater <[email protected]> wrote:
> +static int __init nsproxy_cache_init(void)
> +{
> + nsproxy_cachep = kmem_cache_create("nsproxy", sizeof(struct nsproxy),
> + 0, SLAB_PANIC, NULL, NULL);
> + return 0;
> +}
> +
Christoph added this cheesy KMEM_CACHE macro. But I don't immediately recall
the rationale so I'm a bit reluctant to ask people to use-the-cheesy-macro.
Perhaps he can remind us why it is there?
On Tue, Jun 19, 2007 at 11:35:01AM -0700, Andrew Morton wrote:
> On Mon, 18 Jun 2007 22:53:13 +0200
> Cedric Le Goater <[email protected]> wrote:
>
> > +static int __init nsproxy_cache_init(void)
> > +{
> > + nsproxy_cachep = kmem_cache_create("nsproxy", sizeof(struct nsproxy),
> > + 0, SLAB_PANIC, NULL, NULL);
> > + return 0;
> > +}
> > +
>
> Christoph added this cheesy KMEM_CACHE macro. But I don't immediately recall
> the rationale so I'm a bit reluctant to ask people to use-the-cheesy-macro.
>
> Perhaps he can remind us why it is there?
Hmm, I must have missed the macro going in. Frankly speaking I plain hate
it. It's a rather useless obsfucation.
On Tue, 19 Jun 2007, Andrew Morton wrote:
> On Mon, 18 Jun 2007 22:53:13 +0200
> Cedric Le Goater <[email protected]> wrote:
>
> > +static int __init nsproxy_cache_init(void)
> > +{
> > + nsproxy_cachep = kmem_cache_create("nsproxy", sizeof(struct nsproxy),
> > + 0, SLAB_PANIC, NULL, NULL);
> > + return 0;
> > +}
> > +
>
> Christoph added this cheesy KMEM_CACHE macro. But I don't immediately recall
> the rationale so I'm a bit reluctant to ask people to use-the-cheesy-macro.
>
> Perhaps he can remind us why it is there?
Because it simplifies the handling of slabs.
The above will could become:
nsproxy_cachep = KMEM_CACHE(nsproxy, SLAB_PANIC);
meaning create a cache for the nsproxy struct, the nsproxy name and the
nsproxy size. See include/linux/slab.h.
?
On Tue, 19 Jun 2007, Christoph Hellwig wrote:
> Hmm, I must have missed the macro going in. Frankly speaking I plain hate
> it. It's a rather useless obsfucation.
It makes the code easier to review and reduces errors by establishing a
standard way of defining a slab with minimal effort. You can
still do the old style and create the kmem_cache_create parameter
monsters that span lots of lines.
KMEM_CACHE can do it just by specifying two parameters.
Hi Christoph x 2,
On Tue, 19 Jun 2007, Christoph Hellwig wrote:
> > Hmm, I must have missed the macro going in. Frankly speaking I plain hate
> > it. It's a rather useless obsfucation.
I hate the name, but the macro is far from useless.
On 6/19/07, Christoph Lameter <[email protected]> wrote:
> It makes the code easier to review and reduces errors by establishing a
> standard way of defining a slab with minimal effort. You can
> still do the old style and create the kmem_cache_create parameter
> monsters that span lots of lines.
>
> KMEM_CACHE can do it just by specifying two parameters.
Yes and if you look at existing callers of kmem_cache_create(), you'll
notice that most of them work _exactly_ the way the macro does. Most
of the time you want to use default alignment and not define a
constructor.
Christoph Lameter wrote:
> On Tue, 19 Jun 2007, Andrew Morton wrote:
>
>> On Mon, 18 Jun 2007 22:53:13 +0200
>> Cedric Le Goater <[email protected]> wrote:
>>
>>> +static int __init nsproxy_cache_init(void)
>>> +{
>>> + nsproxy_cachep = kmem_cache_create("nsproxy", sizeof(struct nsproxy),
>>> + 0, SLAB_PANIC, NULL, NULL);
>>> + return 0;
>>> +}
>>> +
>> Christoph added this cheesy KMEM_CACHE macro. But I don't immediately recall
>> the rationale so I'm a bit reluctant to ask people to use-the-cheesy-macro.
>>
>> Perhaps he can remind us why it is there?
>
> Because it simplifies the handling of slabs.
>
> The above will could become:
>
> nsproxy_cachep = KMEM_CACHE(nsproxy, SLAB_PANIC);
>
> meaning create a cache for the nsproxy struct, the nsproxy name and the
> nsproxy size. See include/linux/slab.h.
yes, I should probably use that for the nsproxy struct.
my 2cts :
the macro sets the align parameter to "__alignof__(struct)" by default.
is that something we want to do all the time ? if so, why not change
kmem_cache_create() directly ?
Most of the complexity is in flags. I did a grep and picked what i thought
was the most aggressive. The macro would probably be more useful if we could
identify by it's name in which context it can be used.
C.
On Fri, 22 Jun 2007, Cedric Le Goater wrote:
> the macro sets the align parameter to "__alignof__(struct)" by default.
> is that something we want to do all the time ? if so, why not change
> kmem_cache_create() directly ?
Its a safety net. If there is some reason that the structure needs a
larger alignment than ARCH_KMALLOC_MINALIGN then that alignment will be
applied.
> Most of the complexity is in flags. I did a grep and picked what i thought
> was the most aggressive. The macro would probably be more useful if we could
> identify by it's name in which context it can be used.
What context are you thinking about and how would it influence
the macro?