| Message ID | 20200710161516.11625-2-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | devres: provide and use devm_krealloc() | expand |
On Fri, Jul 10, 2020 at 7:17 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Implement the managed variant of krealloc(). This function works with > all memory allocated by devm_kmalloc() (or devres functions using it > implicitly like devm_kmemdup(), devm_kstrdup() etc.). > > Managed realloc'ed chunks can be manually released with devm_kfree(). ... > devm_kfree() > devm_kmalloc() > devm_kmalloc_array() > + devm_krealloc() > devm_kmemdup() > devm_kstrdup() > devm_kvasprintf() Order? ... > +void *devm_krealloc(struct device *dev, void *ptr, size_t new_size, gfp_t gfp) Do we really need the 'new_' prefix in the parameter? > +{ > + struct devres *old_dr, *new_dr; > + struct list_head old_head; > + unsigned long flags; > + void *ret = NULL; > + size_t tot_size; tot -> total. > + > + if (unlikely(!new_size)) { > + devm_kfree(dev, ptr); > + return ZERO_SIZE_PTR; > + } I guess here we need a comment of the possibilities below to have ZERO_SIZE_PTR as input. > + if (unlikely(ZERO_OR_NULL_PTR(ptr))) > + return devm_kmalloc(dev, new_size, gfp); > + > + if (WARN_ON(is_kernel_rodata((unsigned long)ptr))) > + /* > + * We cannot reliably realloc a const string returned by > + * devm_kstrdup_const(). > + */ > + return NULL; > + > + if (!check_dr_size(new_size, &tot_size)) > + return NULL; > + > + spin_lock_irqsave(&dev->devres_lock, flags); > + > + old_dr = find_dr(dev, devm_kmalloc_release, devm_kmalloc_match, ptr); > + if (WARN_ON(!old_dr)) Under spin lock? I would rather see spin unlock followed by WARN. > + /* Memory chunk not managed or managed by a different device. */ > + goto out; > + > + old_head = old_dr->node.entry; > + > + new_dr = krealloc(old_dr, tot_size, gfp); > + if (!new_dr) > + goto out; > + > + if (new_dr != old_dr) > + list_replace(&old_head, &new_dr->node.entry); > + > + ret = new_dr->data; > + > +out: > + spin_unlock_irqrestore(&dev->devres_lock, flags); > + return ret; > +}
On Sat, Jul 11, 2020 at 5:08 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Fri, Jul 10, 2020 at 7:17 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > Implement the managed variant of krealloc(). This function works with > > all memory allocated by devm_kmalloc() (or devres functions using it > > implicitly like devm_kmemdup(), devm_kstrdup() etc.). > > > > Managed realloc'ed chunks can be manually released with devm_kfree(). > > ... > > > devm_kfree() > > devm_kmalloc() > > devm_kmalloc_array() > > + devm_krealloc() > > devm_kmemdup() > > devm_kstrdup() > > devm_kvasprintf() > > Order? > I didn't notice these were ordered alphabetically, I thought it was by functionality. > ... > > > +void *devm_krealloc(struct device *dev, void *ptr, size_t new_size, gfp_t gfp) > > Do we really need the 'new_' prefix in the parameter? > Yes, I think this is a good name for a function that's meant to modify the size of a memory chunk. > > +{ > > + struct devres *old_dr, *new_dr; > > + struct list_head old_head; > > + unsigned long flags; > > + void *ret = NULL; > > + size_t tot_size; > > tot -> total. > Meh, ok. > > + > > + if (unlikely(!new_size)) { > > + devm_kfree(dev, ptr); > > + return ZERO_SIZE_PTR; > > + } > > I guess here we need a comment of the possibilities below to have > ZERO_SIZE_PTR as input. > Better yet: I'll just add a kernel doc for this function. > > + if (unlikely(ZERO_OR_NULL_PTR(ptr))) > > + return devm_kmalloc(dev, new_size, gfp); > > + > > + if (WARN_ON(is_kernel_rodata((unsigned long)ptr))) > > + /* > > + * We cannot reliably realloc a const string returned by > > + * devm_kstrdup_const(). > > + */ > > + return NULL; > > + > > + if (!check_dr_size(new_size, &tot_size)) > > + return NULL; > > + > > + spin_lock_irqsave(&dev->devres_lock, flags); > > + > > + old_dr = find_dr(dev, devm_kmalloc_release, devm_kmalloc_match, ptr); > > > + if (WARN_ON(!old_dr)) > > Under spin lock? I would rather see spin unlock followed by WARN. > Yeah, can be done. > > + /* Memory chunk not managed or managed by a different device. */ > > + goto out; > > + > > + old_head = old_dr->node.entry; > > + > > + new_dr = krealloc(old_dr, tot_size, gfp); > > + if (!new_dr) > > + goto out; > > + > > + if (new_dr != old_dr) > > + list_replace(&old_head, &new_dr->node.entry); > > + > > + ret = new_dr->data; > > + > > +out: > > + spin_unlock_irqrestore(&dev->devres_lock, flags); > > + return ret; > > +} > > -- > With Best Regards, > Andy Shevchenko Bartosz
diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index eaaaafc21134..aa08f5162524 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -353,6 +353,7 @@ MEM devm_kfree() devm_kmalloc() devm_kmalloc_array() + devm_krealloc() devm_kmemdup() devm_kstrdup() devm_kvasprintf() diff --git a/drivers/base/devres.c b/drivers/base/devres.c index ed615d3b9cf1..4b8870ef6a3f 100644 --- a/drivers/base/devres.c +++ b/drivers/base/devres.c @@ -837,6 +837,56 @@ void *devm_kmalloc(struct device *dev, size_t size, gfp_t gfp) } EXPORT_SYMBOL_GPL(devm_kmalloc); +void *devm_krealloc(struct device *dev, void *ptr, size_t new_size, gfp_t gfp) +{ + struct devres *old_dr, *new_dr; + struct list_head old_head; + unsigned long flags; + void *ret = NULL; + size_t tot_size; + + if (unlikely(!new_size)) { + devm_kfree(dev, ptr); + return ZERO_SIZE_PTR; + } + + if (unlikely(ZERO_OR_NULL_PTR(ptr))) + return devm_kmalloc(dev, new_size, gfp); + + if (WARN_ON(is_kernel_rodata((unsigned long)ptr))) + /* + * We cannot reliably realloc a const string returned by + * devm_kstrdup_const(). + */ + return NULL; + + if (!check_dr_size(new_size, &tot_size)) + return NULL; + + spin_lock_irqsave(&dev->devres_lock, flags); + + old_dr = find_dr(dev, devm_kmalloc_release, devm_kmalloc_match, ptr); + if (WARN_ON(!old_dr)) + /* Memory chunk not managed or managed by a different device. */ + goto out; + + old_head = old_dr->node.entry; + + new_dr = krealloc(old_dr, tot_size, gfp); + if (!new_dr) + goto out; + + if (new_dr != old_dr) + list_replace(&old_head, &new_dr->node.entry); + + ret = new_dr->data; + +out: + spin_unlock_irqrestore(&dev->devres_lock, flags); + return ret; +} +EXPORT_SYMBOL_GPL(devm_krealloc); + /** * devm_kstrdup - Allocate resource managed space and * copy an existing string into that. diff --git a/include/linux/device.h b/include/linux/device.h index 7322c51e9c0c..f64f40843159 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -206,6 +206,8 @@ int devres_release_group(struct device *dev, void *id); /* managed devm_k.alloc/kfree for device drivers */ void *devm_kmalloc(struct device *dev, size_t size, gfp_t gfp) __malloc; +void *devm_krealloc(struct device *dev, void *ptr, size_t size, + gfp_t gfp) __must_check; __printf(3, 0) char *devm_kvasprintf(struct device *dev, gfp_t gfp, const char *fmt, va_list ap) __malloc; __printf(3, 4) char *devm_kasprintf(struct device *dev, gfp_t gfp,