devres: provide and use devm_kstrdup_const()
Subsystems (6)
v5 cover devres: provide and use devm_kstrdup_const()
This series implements devm_kstrdup_const() together with some prerequisite changes and uses it in pmc-atom driver.
v1 -> v2:
- fixed the changelog in the patch implementing devm_kstrdup_const()
- fixed the kernel doc
- moved is_kernel_rodata() to asm-generic/sections.h
- fixed constness
v2 -> v3:
- rebased on top of 4.19-rc5 as there were some conflicts in the pmc-atom driver
- collected Reviewed-by tags
v3 -> v4:
- Andy NAK'ed patch 4/4 so I added a different example
- collected more tags
v4 -> v5:
- instead of providing devm_kfree_const(), make devm_kfree() check if given pointer is not in .rodata and act accordingly
Bartosz Golaszewski (4):
devres: constify p in devm_kfree()
mm: move is_kernel_rodata() to asm-generic/sections.h
devres: provide devm_kstrdup_const()
mailbox: tegra-hsp: use devm_kstrdup_const()
drivers/base/devres.c | 36 +++++++++++++++++++++++++++-- drivers/mailbox/tegra-hsp.c | 41 ++++++++-------------------------- include/asm-generic/sections.h | 14 ++++++++++++ include/linux/device.h | 4 +++- mm/util.c | 7 ------ 5 files changed, 60 insertions(+), 42 deletions(-)
Signature · 1 line
2.18.0
PATCH v5 1/4 devres: constify p in devm_kfree()
Make devm_kfree() signature uniform with that of kfree(). To avoid compiler warnings: cast p to (void *) when calling devres_destroy().
--- drivers/base/devres.c | 5 +++-- include/linux/device.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-)
drivers/base/devres.c +3 -2
@@ -885,11 +885,12 @@ EXPORT_SYMBOL_GPL(devm_kasprintf);* * Free memory allocated with devm_kmalloc(). */ -void devm_kfree (struct device *dev,void *p) +void devm_kfree (struct device *dev,const void *p) {int rc; - rc =devres_destroy (dev, devm_kmalloc_release, devm_kmalloc_match, p); + rc =devres_destroy (dev, devm_kmalloc_release, + devm_kmalloc_match, (void *)p);WARN_ON (rc); }EXPORT_SYMBOL_GPL (devm_kfree);
include/linux/device.h +1 -1
@@ -692,7 +692,7 @@ static inline void *devm_kcalloc(struct device *dev, {return devm_kmalloc_array (dev, n, size, flags | __GFP_ZERO); } -extern void devm_kfree (struct device *dev,void *p); +extern void devm_kfree (struct device *dev,const void *p);extern char *devm_kstrdup (struct device *dev,const char *s,gfp_t gfp) __malloc;extern void *devm_kmemdup (struct device *dev,const void *src,size_t len,gfp_t gfp);
Signature · 1 line
2.18.0
Re: [PATCH v5 1/4] devres: constify p in devm_kfree()
Quoted 2 lines · Bartosz Golaszewski
Make
devm_kfree()signature uniform with that ofkfree(). To avoid compiler warnings: cast p to (void *) when callingdevres_destroy().
Hmm, devres_destroy(), devres_remove(), and find_dr() really should take const void pointers. But that requires changing dr_match_t, and fixing up all users...
Quoted 2 lines
Signed-off-by Bartosz Golaszewski <[email protected]>Reviewed-by Bjorn Andersson <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH v5 2/4 mm: move is_kernel_rodata() to asm-generic/sections.h
In reply to Bartosz GolaszewskiExport this routine so that we can use it later in devm_kstrdup_const() and devm_kfree_const().
--- include/asm-generic/sections.h | 14 ++++++++++++++ mm/util.c | 7 ------- 2 files changed, 14 insertions(+), 7 deletions(-)
include/asm-generic/sections.h +14 -0
@@ -141,4 +141,18 @@ static inline bool init_section_intersects(void *virt, size_t size) return memory_intersects(__init_begin, __init_end, virt, size); } +/** +* is_kernel_rodata - checks if the pointer address is located in the +* .rodata section +* +* @addr: address to check +* +* Returns: true if the address is located in .rodata, false otherwise. +*/ +static inline bool is_kernel_rodata (unsigned long addr) +{ +return addr >= (unsigned long )__start_rodata && + addr < (unsigned long )__end_rodata; +} +#endif /* _ASM_GENERIC_SECTIONS_H_ */
mm/util.c +0 -7
@@ -15,17 +15,10 @@#include <linux/vmalloc.h> #include <linux/userfaultfd_k.h> -#include <asm/sections.h> #include <linux/uaccess.h> #include "internal.h" -static inline int is_kernel_rodata (unsigned long addr) -{ -return addr >= (unsigned long )__start_rodata && - addr < (unsigned long )__end_rodata; -} -/** * kfree_const - conditionally free memory * @x: pointer to the memory
Signature · 1 line
2.18.0
Re: [PATCH v5 2/4] mm: move is_kernel_rodata() to asm-generic/sections.h
Quoted 2 lines · Bartosz Golaszewski
Export this routine so that we can use it later in
devm_kstrdup_const()anddevm_kfree_const().
s/devm_kfree_const/devm_kfree/.
Apart from that nit, feel free to add my ack to 1,2,3.
Thanks,
Rasmus
Re: [PATCH v5 2/4] mm: move is_kernel_rodata() to asm-generic/sections.h
In reply to Bartosz GolaszewskiQuoted 6 lines · Bartosz Golaszewski
Export this routine so that we can use it later in
devm_kstrdup_const()anddevm_kfree_const().Signed-off-by Bartosz Golaszewski <[email protected]>Reviewed-by Bjorn Andersson <[email protected]>Acked-by Mike Rapoport <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH v5 3/4 devres: provide devm_kstrdup_const()
In reply to Bartosz GolaszewskiProvide a resource managed version of kstrdup_const(). This variant internally calls devm_kstrdup() on pointers that are outside of .rodata section and returns the string as is otherwise.
Make devm_kfree() check if the passed pointer doesn't point to .rodata and if so - don't actually destroy the resource.
--- drivers/base/devres.c | 31 +++++++++++++++++++++++++++++++ include/linux/device.h | 2 ++ 2 files changed, 33 insertions(+)
drivers/base/devres.c +31 -0
@@ -11,6 +11,8 @@#include <linux/slab.h> #include <linux/percpu.h> +#include <asm/sections.h> +#include "base.h" struct devres_node {
@@ -822,6 +824,28 @@ char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp) }EXPORT_SYMBOL_GPL (devm_kstrdup); +/** +* devm_kstrdup_const - resource managed conditional string duplication +* @dev: device for which to duplicate the string +* @s: the string to duplicate +* @gfp: the GFP mask used in the kmalloc() call when allocating memory +* +* Strings allocated by devm_kstrdup_const will be automatically freed when +* the associated device is detached. +* +* RETURNS: +* Source string if it is in .rodata section otherwise it falls back to +* devm_kstrdup. +*/ +const char *devm_kstrdup_const (struct device *dev,const char *s,gfp_t gfp) +{ +if (is_kernel_rodata ((unsigned long )s)) +return s; + +return devm_kstrdup (dev, s, gfp); +} +EXPORT_SYMBOL (devm_kstrdup_const); +/** * devm_kvasprintf - Allocate resource managed space and format a string * into that.
@@ -889,6 +913,13 @@ void devm_kfree(struct device *dev, const void *p) {int rc; +/* +* Special case: pointer to a string in .rodata returned by +* devm_kstrdup_const(). +*/ +if (unlikely (is_kernel_rodata ((unsigned long )p))) +return ; + rc =devres_destroy (dev, devm_kmalloc_release, devm_kmalloc_match, (void *)p);WARN_ON (rc);
include/linux/device.h +2 -0
@@ -694,6 +694,8 @@ static inline void *devm_kcalloc(struct device *dev, }extern void devm_kfree (struct device *dev,const void *p);extern char *devm_kstrdup (struct device *dev,const char *s,gfp_t gfp) __malloc; +extern const char *devm_kstrdup_const (struct device *dev, +const char *s,gfp_t gfp);extern void *devm_kmemdup (struct device *dev,const void *src,size_t len,gfp_t gfp);
Signature · 1 line
2.18.0
Re: [PATCH v5 3/4] devres: provide devm_kstrdup_const()
Quoted 10 lines · Bartosz Golaszewski
Provide a resource managed version of
kstrdup_const(). This variant internally callsdevm_kstrdup()on pointers that are outside of .rodata section and returns the string as is otherwise.Make
devm_kfree()check if the passed pointer doesn't point to .rodata and if so - don't actually destroy the resource.Signed-off-by Bartosz Golaszewski <[email protected]>Reviewed-by Bjorn Andersson <[email protected]>Acked-by Mike Rapoport <[email protected]>
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
PATCH v5 4/4 mailbox: tegra-hsp: use devm_kstrdup_const()
In reply to Bartosz GolaszewskiUse devm_kstrdup_const() in the tegra-hsp driver. This mostly serves as an example of how to use this new routine to shrink driver code.
Also use devm_kzalloc() instead of regular kzalloc() to get shrink the driver even more.
Doorbell objects are only removed in the driver's remove callback so it's safe to convert all memory allocations to devres.
--- drivers/mailbox/tegra-hsp.c | 41 ++++++++----------------------------- 1 file changed, 9 insertions(+), 32 deletions(-)
drivers/mailbox/tegra-hsp.c +9 -32
@@ -183,14 +183,15 @@ static irqreturn_t tegra_hsp_doorbell_irq(int irq, void *data) }static struct tegra_hsp_channel * -tegra_hsp_doorbell_create (struct tegra_hsp *hsp,const char *name, -unsigned int master,unsigned int index) +tegra_hsp_doorbell_create (struct device *dev,struct tegra_hsp *hsp, +const char *name,unsigned int master, +unsigned int index) {struct tegra_hsp_doorbell *db;unsigned int offset;unsigned long flags; - db =kzalloc (sizeof (*db),GFP_KERNEL ); + db =devm_kzalloc (dev,sizeof (*db),GFP_KERNEL );if (!db)return ERR_PTR (-ENOMEM );
@@ -200,7 +201,7 @@ tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name, db->channel.regs = hsp->regs + offset; db->channel.hsp = hsp; - db->name =kstrdup_const (name,GFP_KERNEL ); + db->name =devm_kstrdup_const (dev, name,GFP_KERNEL ); db->master = master; db->index = index;
@@ -211,13 +212,6 @@ tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,return &db->channel; } -static void __tegra_hsp_doorbell_destroy (struct tegra_hsp_doorbell *db) -{ -list_del (&db->list); -kfree_const (db->name); -kfree (db); -} -static int tegra_hsp_doorbell_send_data (struct mbox_chan *chan,void *data) {struct tegra_hsp_doorbell *db = chan->con_priv;
@@ -332,31 +326,16 @@ static struct mbox_chan *of_tegra_hsp_xlate(struct mbox_controller *mbox,return chan ?:ERR_PTR (-EBUSY ); } -static void tegra_hsp_remove_doorbells (struct tegra_hsp *hsp) -{ -struct tegra_hsp_doorbell *db, *tmp; -unsigned long flags; - -spin_lock_irqsave (&hsp->lock, flags); - -list_for_each_entry_safe (db, tmp, &hsp->doorbells, list) -__tegra_hsp_doorbell_destroy (db); - -spin_unlock_irqrestore (&hsp->lock, flags); -} - -static int tegra_hsp_add_doorbells (struct tegra_hsp *hsp) +static int tegra_hsp_add_doorbells (struct device *dev,struct tegra_hsp *hsp) {const struct tegra_hsp_db_map *map = hsp->soc->map;struct tegra_hsp_channel *channel;while (map->name) { - channel =tegra_hsp_doorbell_create (hsp, map->name, + channel =tegra_hsp_doorbell_create (dev, hsp, map->name, map->master, map->index); -if (IS_ERR (channel)) { -tegra_hsp_remove_doorbells (hsp); +if (IS_ERR (channel))return PTR_ERR (channel); - } map++; }
@@ -412,7 +391,7 @@ static int tegra_hsp_probe(struct platform_device *pdev)if (!hsp->mbox.chans)return -ENOMEM ; - err =tegra_hsp_add_doorbells (hsp); + err =tegra_hsp_add_doorbells (&pdev->dev, hsp);if (err <0 ) {dev_err (&pdev->dev,"failed to add doorbells: %d\n" , err);return err;
@@ -423,7 +402,6 @@ static int tegra_hsp_probe(struct platform_device *pdev) err =mbox_controller_register (&hsp->mbox);if (err) {dev_err (&pdev->dev,"failed to register mailbox: %d\n" , err); -tegra_hsp_remove_doorbells (hsp);return err; }
@@ -443,7 +421,6 @@ static int tegra_hsp_remove(struct platform_device *pdev)struct tegra_hsp *hsp =platform_get_drvdata (pdev);mbox_controller_unregister (&hsp->mbox); -tegra_hsp_remove_doorbells (hsp);return 0 ; }
Signature · 1 line
2.18.0