On Mon, Dec 01, 2008 at 01:18:33PM +0200, Pekka Enberg wrote:
> Hi Nick,
>
> On Mon, Dec 1, 2008 at 10:31 AM, Nick Piggin <npiggin@xxxxxxx> wrote:
> > What does everyone think about this patch?
>
> Doesn't matter all that much for SLUB which already merges the ACPI
> caches with the generic kmalloc caches. But for SLAB, it's an obvious
> wil so:
>
> Acked-by: Pekka Enberg <penberg@xxxxxxxxxxxxxx>
Actually I think it is also somewhat of a bugfix (not to mention that it
seems like a good idea to share testing code with other operating systems).
Because acpi_os_purge_cache seems to want to free all active objects in the
cache, but kmem_cache_shrink actually does nothing of the sort. So there
ends up being a memory leak.
In practice, there are warnings in some of the allocators if this ever
happens and I don't think I have seen these trigger, so perhaps the ACPI
code which calls this never actually cares. But still seems like a good
idea to use the generic code (which seems to get this right)
Anyway, thanks for the ack. Yes it should help SLAB.
>
> > ACPI subsystem creates a handful of kmem caches that are not particularly
> > appropriate. Most of them seem to be empty or nearly empty most of the time,
> > and the others don't have too many objects. In this situation, kmem caches
> > can actually have more overhead than they save.
> >
> > Just use ACPI's generic code for its acpi_cache_t type.
> > ---
> > drivers/acpi/osl.c | 85 ----------------------------------------
> > include/acpi/acmacros.h | 2
> > include/acpi/platform/aclinux.h | 9 ----
> > 3 files changed, 3 insertions(+), 93 deletions(-)
> >
> > Index: linux-2.6/include/acpi/acmacros.h
> > ===================================================================
> > --- linux-2.6.orig/include/acpi/acmacros.h
> > +++ linux-2.6/include/acpi/acmacros.h
> > @@ -670,7 +670,7 @@ struct acpi_integer_overlay {
> > #define ACPI_ALLOCATE_ZEROED(a) acpi_ut_allocate_zeroed((acpi_size)(a), ACPI_MEM_PARAMETERS)
> > #endif
> > #ifndef ACPI_FREE
> > -#define ACPI_FREE(a) acpio_os_free(a)
> > +#define ACPI_FREE(a) acpi_os_free(a)
> > #endif
> > #define ACPI_MEM_TRACKING(a)
> >
> > Index: linux-2.6/include/acpi/platform/aclinux.h
> > ===================================================================
> > --- linux-2.6.orig/include/acpi/platform/aclinux.h
> > +++ linux-2.6/include/acpi/platform/aclinux.h
> > @@ -65,7 +65,6 @@
> > /* Host-dependent types and defines */
> >
> > #define ACPI_MACHINE_WIDTH BITS_PER_LONG
> > -#define acpi_cache_t struct kmem_cache
> > #define acpi_spinlock spinlock_t *
> > #define ACPI_EXPORT_SYMBOL(symbol) EXPORT_SYMBOL(symbol);
> > #define strtoul simple_strtoul
> > @@ -73,6 +72,8 @@
> > /* Full namespace pathname length limit - arbitrary */
> > #define ACPI_PATHNAME_MAX 256
> >
> > +#define ACPI_USE_LOCAL_CACHE
> > +
> > #else /* !__KERNEL__ */
> >
> > #include <stdarg.h>
> > @@ -128,12 +129,6 @@ static inline void *acpi_os_allocate_zer
> > return kzalloc(size, irqs_disabled()? GFP_ATOMIC : GFP_KERNEL);
> > }
> >
> > -static inline void *acpi_os_acquire_object(acpi_cache_t * cache)
> > -{
> > - return kmem_cache_zalloc(cache,
> > - irqs_disabled()? GFP_ATOMIC : GFP_KERNEL);
> > -}
> > -
> > #define ACPI_ALLOCATE(a) acpi_os_allocate(a)
> > #define ACPI_ALLOCATE_ZEROED(a) acpi_os_allocate_zeroed(a)
> > #define ACPI_FREE(a) kfree(a)
> > Index: linux-2.6/drivers/acpi/osl.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/osl.c
> > +++ linux-2.6/drivers/acpi/osl.c
> > @@ -1212,91 +1212,6 @@ void acpi_os_release_lock(acpi_spinlock
> > spin_unlock_irqrestore(lockp, flags);
> > }
> >
> > -#ifndef ACPI_USE_LOCAL_CACHE
> > -
> > -/*******************************************************************************
> > - *
> > - * FUNCTION: acpi_os_create_cache
> > - *
> > - * PARAMETERS: name - Ascii name for the cache
> > - * size - Size of each cached object
> > - * depth - Maximum depth of the cache (in objects) <ignored>
> > - * cache - Where the new cache object is returned
> > - *
> > - * RETURN: status
> > - *
> > - * DESCRIPTION: Create a cache object
> > - *
> > - ******************************************************************************/
> > -
> > -acpi_status
> > -acpi_os_create_cache(char *name, u16 size, u16 depth, acpi_cache_t ** cache)
> > -{
> > - *cache = kmem_cache_create(name, size, 0, 0, NULL);
> > - if (*cache == NULL)
> > - return AE_ERROR;
> > - else
> > - return AE_OK;
> > -}
> > -
> > -/*******************************************************************************
> > - *
> > - * FUNCTION: acpi_os_purge_cache
> > - *
> > - * PARAMETERS: Cache - Handle to cache object
> > - *
> > - * RETURN: Status
> > - *
> > - * DESCRIPTION: Free all objects within the requested cache.
> > - *
> > - ******************************************************************************/
> > -
> > -acpi_status acpi_os_purge_cache(acpi_cache_t * cache)
> > -{
> > - kmem_cache_shrink(cache);
> > - return (AE_OK);
> > -}
> > -
> > -/*******************************************************************************
> > - *
> > - * FUNCTION: acpi_os_delete_cache
> > - *
> > - * PARAMETERS: Cache - Handle to cache object
> > - *
> > - * RETURN: Status
> > - *
> > - * DESCRIPTION: Free all objects within the requested cache and delete the
> > - * cache object.
> > - *
> > - ******************************************************************************/
> > -
> > -acpi_status acpi_os_delete_cache(acpi_cache_t * cache)
> > -{
> > - kmem_cache_destroy(cache);
> > - return (AE_OK);
> > -}
> > -
> > -/*******************************************************************************
> > - *
> > - * FUNCTION: acpi_os_release_object
> > - *
> > - * PARAMETERS: Cache - Handle to cache object
> > - * Object - The object to be released
> > - *
> > - * RETURN: None
> > - *
> > - * DESCRIPTION: Release an object to the specified cache. If cache is full,
> > - * the object is deleted.
> > - *
> > - ******************************************************************************/
> > -
> > -acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object)
> > -{
> > - kmem_cache_free(cache, object);
> > - return (AE_OK);
> > -}
> > -#endif
> > -
> > /**
> > * acpi_dmi_dump - dump DMI slots needed for blacklist entry
> > *
> >
> > --
> > To unsubscribe, send a message with 'unsubscribe linux-mm' in
> > the body to majordomo@xxxxxxxxxx For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
