lkml.org 
[lkml]   [2012]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3] mm: Restructure kmem_cache_create() to move debug cache integrity checks into a new function
    From
    Date
    Acked-by: Christoph Lameter <cl@linux.com>



    On Aug 12, 2012, at 11:40, Shuah Khan <shuah.khan@hp.com> wrote:

    > kmem_cache_create() does cache integrity checks when CONFIG_DEBUG_VM
    > is defined. These checks interspersed with the regular code path has
    > lead to compile time warnings when compiled without CONFIG_DEBUG_VM
    > defined. Restructuring the code to move the integrity checks in to a new
    > function would eliminate the current compile warning problem and also
    > will allow for future changes to the debug only code to evolve without
    > introducing new warnings in the regular path. This restructuring work
    > is based on the discussion in the following thread:
    >
    > https://lkml.org/lkml/2012/7/13/424
    >
    > Signed-off-by: Shuah Khan <shuah.khan@hp.com>
    > ---
    > mm/slab_common.c | 90 +++++++++++++++++++++++++++++-------------------------
    > 1 file changed, 48 insertions(+), 42 deletions(-)
    >
    > diff --git a/mm/slab_common.c b/mm/slab_common.c
    > index 12637ce..44facdf 100644
    > --- a/mm/slab_common.c
    > +++ b/mm/slab_common.c
    > @@ -23,6 +23,52 @@ enum slab_state slab_state;
    > LIST_HEAD(slab_caches);
    > DEFINE_MUTEX(slab_mutex);
    >
    > +#ifdef CONFIG_DEBUG_VM
    > +static int kmem_cache_sanity_check(const char *name, size_t size)
    > +{
    > + struct kmem_cache *s = NULL;
    > +
    > + if (!name || in_interrupt() || size < sizeof(void *) ||
    > + size > KMALLOC_MAX_SIZE) {
    > + pr_err("kmem_cache_create(%s) integrity check failed\n", name);
    > + return -EINVAL;
    > + }
    > +
    > + list_for_each_entry(s, &slab_caches, list) {
    > + char tmp;
    > + int res;
    > +
    > + /*
    > + * This happens when the module gets unloaded and doesn't
    > + * destroy its slab cache and no-one else reuses the vmalloc
    > + * area of the module. Print a warning.
    > + */
    > + res = probe_kernel_address(s->name, tmp);
    > + if (res) {
    > + pr_err("Slab cache with size %d has lost its name\n",
    > + s->object_size);
    > + continue;
    > + }
    > +
    > + if (!strcmp(s->name, name)) {
    > + pr_err("%s (%s): Cache name already exists.\n",
    > + __func__, name);
    > + dump_stack();
    > + s = NULL;
    > + return -EINVAL;
    > + }
    > + }
    > +
    > + WARN_ON(strchr(name, ' ')); /* It confuses parsers */
    > + return 0;
    > +}
    > +#else
    > +static inline int kmem_cache_sanity_check(const char *name, size_t size)
    > +{
    > + return 0;
    > +}
    > +#endif
    > +
    > /*
    > * kmem_cache_create - Create a cache.
    > * @name: A string which is used in /proc/slabinfo to identify this cache.
    > @@ -53,48 +99,11 @@ struct kmem_cache *kmem_cache_create(const char *name, size_t size, size_t align
    > {
    > struct kmem_cache *s = NULL;
    >
    > -#ifdef CONFIG_DEBUG_VM
    > - if (!name || in_interrupt() || size < sizeof(void *) ||
    > - size > KMALLOC_MAX_SIZE) {
    > - printk(KERN_ERR "kmem_cache_create(%s) integrity check"
    > - " failed\n", name);
    > - goto out;
    > - }
    > -#endif
    > -
    > get_online_cpus();
    > mutex_lock(&slab_mutex);
    >
    > -#ifdef CONFIG_DEBUG_VM
    > - list_for_each_entry(s, &slab_caches, list) {
    > - char tmp;
    > - int res;
    > -
    > - /*
    > - * This happens when the module gets unloaded and doesn't
    > - * destroy its slab cache and no-one else reuses the vmalloc
    > - * area of the module. Print a warning.
    > - */
    > - res = probe_kernel_address(s->name, tmp);
    > - if (res) {
    > - printk(KERN_ERR
    > - "Slab cache with size %d has lost its name\n",
    > - s->object_size);
    > - continue;
    > - }
    > -
    > - if (!strcmp(s->name, name)) {
    > - printk(KERN_ERR "kmem_cache_create(%s): Cache name"
    > - " already exists.\n",
    > - name);
    > - dump_stack();
    > - s = NULL;
    > - goto oops;
    > - }
    > - }
    > -
    > - WARN_ON(strchr(name, ' ')); /* It confuses parsers */
    > -#endif
    > + if (kmem_cache_sanity_check(name, size))
    > + goto oops;
    >
    > s = __kmem_cache_create(name, size, align, flags, ctor);
    >
    > @@ -102,9 +111,6 @@ oops:
    > mutex_unlock(&slab_mutex);
    > put_online_cpus();
    >
    > -#ifdef CONFIG_DEBUG_VM
    > -out:
    > -#endif
    > if (!s && (flags & SLAB_PANIC))
    > panic("kmem_cache_create: Failed to create slab '%s'\n", name);
    >
    > --
    > 1.7.9.5
    >
    >


    \
     
     \ /
      Last update: 2012-08-12 20:22    [W:3.148 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site