linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH v6 0/4] devres: provide and use devm_kstrdup_const()
@ 2018-10-14 15:20 Bartosz Golaszewski
  2018-10-14 15:20 ` [RESEND PATCH v6 1/4] devres: constify p in devm_kfree() Bartosz Golaszewski
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-10-14 15:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Bartosz Golaszewski

Hi Greg,

this has been reviewed a lot and three first patches can be picked
up. Please take it through your tree.

This series implements devm_kstrdup_const() together with some
prerequisite changes and uses it in tegra-hsp 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

v5 -> v6:
- fixed the commit message in patch 2/4 (s/devm_kfree_const/devm_kfree/)
- collected even more tags

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(-)

-- 
2.19.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RESEND PATCH v6 1/4] devres: constify p in devm_kfree()
  2018-10-14 15:20 [RESEND PATCH v6 0/4] devres: provide and use devm_kstrdup_const() Bartosz Golaszewski
@ 2018-10-14 15:20 ` Bartosz Golaszewski
  2018-10-14 15:20 ` [RESEND PATCH v6 2/4] mm: move is_kernel_rodata() to asm-generic/sections.h Bartosz Golaszewski
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-10-14 15:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Bartosz Golaszewski

Make devm_kfree() signature uniform with that of kfree(). To avoid
compiler warnings: cast p to (void *) when calling devres_destroy().

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/devres.c  | 5 +++--
 include/linux/device.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index f98a097e73f2..438c91a43508 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -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);
diff --git a/include/linux/device.h b/include/linux/device.h
index 8f882549edee..33f7cb271fbb 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -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);
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RESEND PATCH v6 2/4] mm: move is_kernel_rodata() to asm-generic/sections.h
  2018-10-14 15:20 [RESEND PATCH v6 0/4] devres: provide and use devm_kstrdup_const() Bartosz Golaszewski
  2018-10-14 15:20 ` [RESEND PATCH v6 1/4] devres: constify p in devm_kfree() Bartosz Golaszewski
@ 2018-10-14 15:20 ` Bartosz Golaszewski
  2018-10-16 10:53   ` Greg Kroah-Hartman
  2018-10-14 15:20 ` [RESEND PATCH v6 3/4] devres: provide devm_kstrdup_const() Bartosz Golaszewski
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-10-14 15:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Bartosz Golaszewski

Export this routine so that we can use it later in devm_kstrdup_const()
and devm_kfree().

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/asm-generic/sections.h | 14 ++++++++++++++
 mm/util.c                      |  7 -------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 849cd8eb5ca0..d79abca81a52 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -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_ */
diff --git a/mm/util.c b/mm/util.c
index 9e3ebd2ef65f..470f5cd80b64 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -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
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RESEND PATCH v6 3/4] devres: provide devm_kstrdup_const()
  2018-10-14 15:20 [RESEND PATCH v6 0/4] devres: provide and use devm_kstrdup_const() Bartosz Golaszewski
  2018-10-14 15:20 ` [RESEND PATCH v6 1/4] devres: constify p in devm_kfree() Bartosz Golaszewski
  2018-10-14 15:20 ` [RESEND PATCH v6 2/4] mm: move is_kernel_rodata() to asm-generic/sections.h Bartosz Golaszewski
@ 2018-10-14 15:20 ` Bartosz Golaszewski
  2018-10-16 10:50   ` Greg Kroah-Hartman
  2018-10-14 15:20 ` [RESEND PATCH v6 4/4] mailbox: tegra-hsp: use devm_kstrdup_const() Bartosz Golaszewski
  2018-10-14 15:21 ` [RESEND PATCH v6 0/4] devres: provide and " Bartosz Golaszewski
  4 siblings, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-10-14 15:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Bartosz Golaszewski

Provide 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.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/devres.c  | 31 +++++++++++++++++++++++++++++++
 include/linux/device.h |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 438c91a43508..00c70f0fcdcd 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -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);
diff --git a/include/linux/device.h b/include/linux/device.h
index 33f7cb271fbb..e626acb93ef5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -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);
 
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RESEND PATCH v6 4/4] mailbox: tegra-hsp: use devm_kstrdup_const()
  2018-10-14 15:20 [RESEND PATCH v6 0/4] devres: provide and use devm_kstrdup_const() Bartosz Golaszewski
                   ` (2 preceding siblings ...)
  2018-10-14 15:20 ` [RESEND PATCH v6 3/4] devres: provide devm_kstrdup_const() Bartosz Golaszewski
@ 2018-10-14 15:20 ` Bartosz Golaszewski
  2018-10-14 15:21 ` [RESEND PATCH v6 0/4] devres: provide and " Bartosz Golaszewski
  4 siblings, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-10-14 15:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Bartosz Golaszewski

Use 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.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/mailbox/tegra-hsp.c | 41 ++++++++-----------------------------
 1 file changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index 0cde356c11ab..106c94dedbf1 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -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;
 }
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RESEND PATCH v6 0/4] devres: provide and use devm_kstrdup_const()
  2018-10-14 15:20 [RESEND PATCH v6 0/4] devres: provide and use devm_kstrdup_const() Bartosz Golaszewski
                   ` (3 preceding siblings ...)
  2018-10-14 15:20 ` [RESEND PATCH v6 4/4] mailbox: tegra-hsp: use devm_kstrdup_const() Bartosz Golaszewski
@ 2018-10-14 15:21 ` Bartosz Golaszewski
  2018-10-16 11:33   ` Greg Kroah-Hartman
  4 siblings, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-10-14 15:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Linux Kernel Mailing List

niedz., 14 paź 2018 o 17:20 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> Hi Greg,
>
> this has been reviewed a lot and three first patches can be picked
> up. Please take it through your tree.
>

Except for patch 4/4 of course - it needs Thierry's Ack, but that can
wait until the first three are merged.

Bart

> This series implements devm_kstrdup_const() together with some
> prerequisite changes and uses it in tegra-hsp 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
>
> v5 -> v6:
> - fixed the commit message in patch 2/4 (s/devm_kfree_const/devm_kfree/)
> - collected even more tags
>
> 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(-)
>
> --
> 2.19.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RESEND PATCH v6 3/4] devres: provide devm_kstrdup_const()
  2018-10-14 15:20 ` [RESEND PATCH v6 3/4] devres: provide devm_kstrdup_const() Bartosz Golaszewski
@ 2018-10-16 10:50   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2018-10-16 10:50 UTC (permalink / raw)
  To: Bartosz Golaszewski; +Cc: linux-kernel

On Sun, Oct 14, 2018 at 05:20:09PM +0200, Bartosz Golaszewski wrote:
> Provide 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.
> 
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Acked-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/base/devres.c  | 31 +++++++++++++++++++++++++++++++
>  include/linux/device.h |  2 ++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index 438c91a43508..00c70f0fcdcd 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -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);

This should be EXPORT_SYMBOL_GPL() to match the rest of the file.  I'll
go fix that up when I apply it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RESEND PATCH v6 2/4] mm: move is_kernel_rodata() to asm-generic/sections.h
  2018-10-14 15:20 ` [RESEND PATCH v6 2/4] mm: move is_kernel_rodata() to asm-generic/sections.h Bartosz Golaszewski
@ 2018-10-16 10:53   ` Greg Kroah-Hartman
  2018-10-19 14:42     ` Bartosz Golaszewski
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2018-10-16 10:53 UTC (permalink / raw)
  To: Bartosz Golaszewski; +Cc: linux-kernel

On Sun, Oct 14, 2018 at 05:20:08PM +0200, Bartosz Golaszewski wrote:
> Export this routine so that we can use it later in devm_kstrdup_const()
> and devm_kfree().
> 
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Acked-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/asm-generic/sections.h | 14 ++++++++++++++
>  mm/util.c                      |  7 -------
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index 849cd8eb5ca0..d79abca81a52 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -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;

Those symbols are not exported, are you sure this is going to work for a
kernel module that ends up using this function?

I'll take it, but be aware of the future complications that might happen
here...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RESEND PATCH v6 0/4] devres: provide and use devm_kstrdup_const()
  2018-10-14 15:21 ` [RESEND PATCH v6 0/4] devres: provide and " Bartosz Golaszewski
@ 2018-10-16 11:33   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2018-10-16 11:33 UTC (permalink / raw)
  To: Bartosz Golaszewski; +Cc: Linux Kernel Mailing List

On Sun, Oct 14, 2018 at 05:21:16PM +0200, Bartosz Golaszewski wrote:
> niedz., 14 paź 2018 o 17:20 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
> >
> > Hi Greg,
> >
> > this has been reviewed a lot and three first patches can be picked
> > up. Please take it through your tree.
> >
> 
> Except for patch 4/4 of course - it needs Thierry's Ack, but that can
> wait until the first three are merged.

first 3 are merged now.

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RESEND PATCH v6 2/4] mm: move is_kernel_rodata() to asm-generic/sections.h
  2018-10-16 10:53   ` Greg Kroah-Hartman
@ 2018-10-19 14:42     ` Bartosz Golaszewski
  0 siblings, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2018-10-19 14:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Linux Kernel Mailing List

wt., 16 paź 2018 o 12:53 Greg Kroah-Hartman
<gregkh@linuxfoundation.org> napisał(a):
>
> On Sun, Oct 14, 2018 at 05:20:08PM +0200, Bartosz Golaszewski wrote:
> > Export this routine so that we can use it later in devm_kstrdup_const()
> > and devm_kfree().
> >
> > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Acked-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > Acked-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  include/asm-generic/sections.h | 14 ++++++++++++++
> >  mm/util.c                      |  7 -------
> >  2 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> > index 849cd8eb5ca0..d79abca81a52 100644
> > --- a/include/asm-generic/sections.h
> > +++ b/include/asm-generic/sections.h
> > @@ -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;
>
> Those symbols are not exported, are you sure this is going to work for a
> kernel module that ends up using this function?
>

I tested it with a loaded module, yes.

> I'll take it, but be aware of the future complications that might happen
> here...
>

These symbols are built-in for all architectures. Do they even need
exporting? On the off-chance that something fails, we can move this
function back to mm/util.c as it was in the first version of this
series.

Best regards,
Bartosz Golaszewski

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-10-19 14:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-14 15:20 [RESEND PATCH v6 0/4] devres: provide and use devm_kstrdup_const() Bartosz Golaszewski
2018-10-14 15:20 ` [RESEND PATCH v6 1/4] devres: constify p in devm_kfree() Bartosz Golaszewski
2018-10-14 15:20 ` [RESEND PATCH v6 2/4] mm: move is_kernel_rodata() to asm-generic/sections.h Bartosz Golaszewski
2018-10-16 10:53   ` Greg Kroah-Hartman
2018-10-19 14:42     ` Bartosz Golaszewski
2018-10-14 15:20 ` [RESEND PATCH v6 3/4] devres: provide devm_kstrdup_const() Bartosz Golaszewski
2018-10-16 10:50   ` Greg Kroah-Hartman
2018-10-14 15:20 ` [RESEND PATCH v6 4/4] mailbox: tegra-hsp: use devm_kstrdup_const() Bartosz Golaszewski
2018-10-14 15:21 ` [RESEND PATCH v6 0/4] devres: provide and " Bartosz Golaszewski
2018-10-16 11:33   ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).