| Message ID | 20200629065008.27620-6-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | devres: provide and use devm_krealloc() | expand |
On Mon, Jun 29, 2020 at 08:50:07AM +0200, Bartosz Golaszewski 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(). > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > .../driver-api/driver-model/devres.rst | 1 + > drivers/base/devres.c | 50 +++++++++++++++++++ > include/linux/device.h | 2 + > 3 files changed, 53 insertions(+) > > diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst > index e0b58c392e4f..0a2572c3813c 100644 > --- a/Documentation/driver-api/driver-model/devres.rst > +++ b/Documentation/driver-api/driver-model/devres.rst > @@ -352,6 +352,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); That's a lot of logic that does not seem to match up with the krealloc() logic in mm/slab_common.c, are you sure we need to do all of that? Who wants this? thanks, greg k-h
On Thu, Jul 2, 2020 at 2:42 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Mon, Jun 29, 2020 at 08:50:07AM +0200, Bartosz Golaszewski 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(). > > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> [snip!] > > That's a lot of logic that does not seem to match up with the krealloc() > logic in mm/slab_common.c, are you sure we need to do all of that? > What are you referring to exactly? The check for rodata? It's because devm_kfree() handles this case, while regular kfree() (or krealloc()) doesn't - there's kfree_const() but no devm_kfree_const(). > Who wants this? The hwmon commit I mentioned in my response to patch 6/6 explicitly mentions the lack of this helper. Bartosz
On Thu, Jul 2, 2020 at 3:11 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Thu, Jul 2, 2020 at 2:42 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Mon, Jun 29, 2020 at 08:50:07AM +0200, Bartosz Golaszewski 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(). > > > > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > [snip!] > > > > > That's a lot of logic that does not seem to match up with the krealloc() > > logic in mm/slab_common.c, are you sure we need to do all of that? > > > > What are you referring to exactly? The check for rodata? It's because > devm_kfree() handles this case, while regular kfree() (or krealloc()) > doesn't - there's kfree_const() but no devm_kfree_const(). > > > Who wants this? > > The hwmon commit I mentioned in my response to patch 6/6 explicitly > mentions the lack of this helper. > > Bartosz Hi Greg, As we've established in the discussion under the iio patch that there will in fact be more users of this - can this now be merged too? Bart
On Mon, Jul 06, 2020 at 06:38:10PM +0200, Bartosz Golaszewski wrote: > On Thu, Jul 2, 2020 at 3:11 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > On Thu, Jul 2, 2020 at 2:42 PM Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > > > > > > On Mon, Jun 29, 2020 at 08:50:07AM +0200, Bartosz Golaszewski 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(). > > > > > > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > [snip!] > > > > > > > > That's a lot of logic that does not seem to match up with the krealloc() > > > logic in mm/slab_common.c, are you sure we need to do all of that? > > > > > > > What are you referring to exactly? The check for rodata? It's because > > devm_kfree() handles this case, while regular kfree() (or krealloc()) > > doesn't - there's kfree_const() but no devm_kfree_const(). > > > > > Who wants this? > > > > The hwmon commit I mentioned in my response to patch 6/6 explicitly > > mentions the lack of this helper. > > > > Bartosz > > Hi Greg, > > As we've established in the discussion under the iio patch that there > will in fact be more users of this - can this now be merged too? Sorry, it's still in my queue, along with 500+ other patches :) it's not lost... thanks, greg k-h
On Mon, Jul 06, 2020 at 06:38:10PM +0200, Bartosz Golaszewski wrote: > On Thu, Jul 2, 2020 at 3:11 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > On Thu, Jul 2, 2020 at 2:42 PM Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > > > > > > On Mon, Jun 29, 2020 at 08:50:07AM +0200, Bartosz Golaszewski 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(). > > > > > > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > [snip!] > > > > > > > > That's a lot of logic that does not seem to match up with the krealloc() > > > logic in mm/slab_common.c, are you sure we need to do all of that? > > > > > > > What are you referring to exactly? The check for rodata? It's because > > devm_kfree() handles this case, while regular kfree() (or krealloc()) > > doesn't - there's kfree_const() but no devm_kfree_const(). > > > > > Who wants this? > > > > The hwmon commit I mentioned in my response to patch 6/6 explicitly > > mentions the lack of this helper. > > > > Bartosz > > Hi Greg, > > As we've established in the discussion under the iio patch that there > will in fact be more users of this - can this now be merged too? Can you resend the remaining patches, along with the users, and I'll review it? I just worry about having to duplicate all of that mm code in there that it will grow stale over time... thanks, greg k-h
diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index e0b58c392e4f..0a2572c3813c 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -352,6 +352,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 9cadb647cacc..228063c6392c 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -201,6 +201,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,