Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933052Ab3JKULm (ORCPT ); Fri, 11 Oct 2013 16:11:42 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:46853 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932134Ab3JKULk (ORCPT ); Fri, 11 Oct 2013 16:11:40 -0400 Date: Fri, 11 Oct 2013 13:11:38 -0700 From: Andrew Morton To: Joe Perches Cc: Tejun Heo , Greg KH , LKML , Sangjung Woo Subject: Re: [RFC PATCH] device: Add kernel standard devm_k.alloc functions Message-Id: <20131011131138.3bc5b2acf60df3a5d79d0d24@linux-foundation.org> In-Reply-To: <1381296747.2040.17.camel@joe-AO722> References: <1381296747.2040.17.camel@joe-AO722> X-Mailer: Sylpheed 3.2.0beta5 (GTK+ 2.24.10; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6936 Lines: 213 On Tue, 08 Oct 2013 22:32:27 -0700 Joe Perches wrote: > Currently, devm_ managed memory only supports kzalloc. > > Convert the devm_kzalloc implementation to devm_kmalloc > and remove the complete memset to 0 but still set the > initial struct devres header and whatever padding before > data to 0. > > Add the other normal alloc variants as static inlines with > __GFP_ZERO added to the gfp flag where appropriate: > > devm_kzalloc > devm_kcalloc > devm_kmalloc_array > > Add gfp.h to device.h for the newly added static inlines. > > ... > > --- a/drivers/base/devres.c > +++ b/drivers/base/devres.c > @@ -91,7 +91,8 @@ static __always_inline struct devres * alloc_dr(dr_release_t release, > if (unlikely(!dr)) > return NULL; > > - memset(dr, 0, tot_size); > + memset(dr, 0, offsetof(struct devres, data)); Well, this does make some assumptions about devres layout. It would have been cleaner to do memset(&dr.node, 0, sizeof(dr.node)); but whatever. I made some changelog changes. I agree that including devm_kmalloc_array() might be going a bit far (it's the lack of devm_kmalloc which matters most). But devm_kmalloc_array() is inlined and is hence basically cost-free until someone actually uses it. From: Joe Perches Subject: devres: add kernel standard devm_k.alloc functions Currently, devm_ managed memory only supports kzalloc. Convert the devm_kzalloc implementation to devm_kmalloc and remove the complete memset to 0 but still set the initial struct devres header and whatever padding before data to 0. Add the other normal alloc variants as static inlines with __GFP_ZERO added to the gfp flag where appropriate: devm_kzalloc devm_kcalloc devm_kmalloc_array Add gfp.h to device.h for the newly added static inlines. akpm: the current API forces us to replace kmalloc() with kzalloc() when performing devm_ conversions. This adds a relatively minor overhead. More significantly, it will defeat kmemcheck used-uninitialized checking, and for a particular driver, losing used-uninitialised checking for their core controlling data structures will significantly degrade kmemcheck usefulness. Signed-off-by: Joe Perches Cc: Tejun Heo Cc: Greg KH Cc: Sangjung Woo Signed-off-by: Andrew Morton --- drivers/base/devres.c | 27 ++++++++++++++++----------- include/linux/device.h | 21 +++++++++++++++++++-- 2 files changed, 35 insertions(+), 13 deletions(-) diff -puN drivers/base/devres.c~device-add-kernel-standard-devm_kalloc-functions drivers/base/devres.c --- a/drivers/base/devres.c~device-add-kernel-standard-devm_kalloc-functions +++ a/drivers/base/devres.c @@ -91,7 +91,8 @@ static __always_inline struct devres * a if (unlikely(!dr)) return NULL; - memset(dr, 0, tot_size); + memset(dr, 0, offsetof(struct devres, data)); + INIT_LIST_HEAD(&dr->node.entry); dr->node.release = release; return dr; @@ -745,58 +746,62 @@ void devm_remove_action(struct device *d EXPORT_SYMBOL_GPL(devm_remove_action); /* - * Managed kzalloc/kfree + * Managed kmalloc/kfree */ -static void devm_kzalloc_release(struct device *dev, void *res) +static void devm_kmalloc_release(struct device *dev, void *res) { /* noop */ } -static int devm_kzalloc_match(struct device *dev, void *res, void *data) +static int devm_kmalloc_match(struct device *dev, void *res, void *data) { return res == data; } /** - * devm_kzalloc - Resource-managed kzalloc + * devm_kmalloc - Resource-managed kmalloc * @dev: Device to allocate memory for * @size: Allocation size * @gfp: Allocation gfp flags * - * Managed kzalloc. Memory allocated with this function is + * Managed kmalloc. Memory allocated with this function is * automatically freed on driver detach. Like all other devres * resources, guaranteed alignment is unsigned long long. * * RETURNS: * Pointer to allocated memory on success, NULL on failure. */ -void * devm_kzalloc(struct device *dev, size_t size, gfp_t gfp) +void * devm_kmalloc(struct device *dev, size_t size, gfp_t gfp) { struct devres *dr; /* use raw alloc_dr for kmalloc caller tracing */ - dr = alloc_dr(devm_kzalloc_release, size, gfp); + dr = alloc_dr(devm_kmalloc_release, size, gfp); if (unlikely(!dr)) return NULL; + /* + * This is named devm_kzalloc_release for historical reasons + * The initial implementation did not support kmalloc, only kzalloc + */ set_node_dbginfo(&dr->node, "devm_kzalloc_release", size); devres_add(dev, dr->data); return dr->data; } -EXPORT_SYMBOL_GPL(devm_kzalloc); +EXPORT_SYMBOL_GPL(devm_kmalloc); /** * devm_kfree - Resource-managed kfree * @dev: Device this memory belongs to * @p: Memory to free * - * Free memory allocated with devm_kzalloc(). + * Free memory allocated with devm_kmalloc(). */ void devm_kfree(struct device *dev, void *p) { int rc; - rc = devres_destroy(dev, devm_kzalloc_release, devm_kzalloc_match, p); + rc = devres_destroy(dev, devm_kmalloc_release, devm_kmalloc_match, p); WARN_ON(rc); } EXPORT_SYMBOL_GPL(devm_kfree); diff -puN include/linux/device.h~device-add-kernel-standard-devm_kalloc-functions include/linux/device.h --- a/include/linux/device.h~device-add-kernel-standard-devm_kalloc-functions +++ a/include/linux/device.h @@ -26,6 +26,7 @@ #include #include #include +#include #include struct device; @@ -602,8 +603,24 @@ extern void devres_close_group(struct de extern void devres_remove_group(struct device *dev, void *id); extern int devres_release_group(struct device *dev, void *id); -/* managed kzalloc/kfree for device drivers, no kmalloc, always use kzalloc */ -extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp); +/* managed devm_k.alloc/kfree for device drivers */ +extern void *devm_kmalloc(struct device *dev, size_t size, gfp_t gfp); +static inline void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp) +{ + return devm_kmalloc(dev, size, gfp | __GFP_ZERO); +} +static inline void *devm_kmalloc_array(struct device *dev, + size_t n, size_t size, gfp_t flags) +{ + if (size != 0 && n > SIZE_MAX / size) + return NULL; + return devm_kmalloc(dev, n * size, flags); +} +static inline void *devm_kcalloc(struct device *dev, + size_t n, size_t size, gfp_t flags) +{ + return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO); +} extern void devm_kfree(struct device *dev, void *p); void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res); _ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/