mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] tlsf: Add tracking of added tlsf memory pools
@ 2025-03-29  3:13 David Dgien via B4 Relay
  2025-04-02  9:07 ` Sascha Hauer
  0 siblings, 1 reply; 3+ messages in thread
From: David Dgien via B4 Relay @ 2025-03-29  3:13 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: David Dgien, Tyler Reece

From: David Dgien <dgienda125@gmail.com>

When configured to use the TLSF allocator, the malloc_stats function
only walks the initial memory pool, as that is the only pool it is aware
of, and TLSF doesn't link pools together in a way that the walker can
follow.

Add wrapper functions around tlsf_add_pool and tlsf_remove_pool to add
additional tracking of added pools, so that they can be walked by the
malloc_stats function and meminfo can report accurate heap utilization.

Signed-off-by: David Dgien <dgienda125@gmail.com>
Reviewed-by: Tyler Reece <mtreece@acm.org>
---
For this change I put the wrapper function prototypes in malloc.h, gated
by CONFIG_MALLOC_TLSF. I was split between there and memory.h, but I'm
not entirely convinced malloc.h was the right decision. I also didn't
provide stubs for when CONFIG_MALLOC_TLSF isn't enabled, as I did not
see any upstream code that utilizes tlsf_add_pool. If the project has
different preferences I can make those changes.
---
 common/tlsf_malloc.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/malloc.h     |  5 ++++
 2 files changed, 69 insertions(+)

diff --git a/common/tlsf_malloc.c b/common/tlsf_malloc.c
index 6a90ee47199fa9e8223f843cc95f52eebfec2aee..0e756a13d5a2665eccf8931b19dfa4e56cad15eb 100644
--- a/common/tlsf_malloc.c
+++ b/common/tlsf_malloc.c
@@ -12,8 +12,18 @@
 #include <module.h>
 #include <tlsf.h>
 
+#include <linux/kasan.h>
+#include <linux/list.h>
+
 extern tlsf_t tlsf_mem_pool;
 
+struct pool_entry {
+	pool_t pool;
+	struct list_head list;
+};
+
+static LIST_HEAD(mem_pool_list);
+
 void *malloc(size_t bytes)
 {
 	void *mem;
@@ -75,6 +85,7 @@ static void malloc_walker(void* ptr, size_t size, int used, void *user)
 
 void malloc_stats(void)
 {
+	struct pool_entry *cur_pool;
 	struct malloc_stats s;
 
 	s.used = 0;
@@ -82,5 +93,58 @@ void malloc_stats(void)
 
 	tlsf_walk_pool(tlsf_get_pool(tlsf_mem_pool), malloc_walker, &s);
 
+	list_for_each_entry(cur_pool, &mem_pool_list, list)
+		tlsf_walk_pool(cur_pool->pool, malloc_walker, &s);
+
 	printf("used: %zu\nfree: %zu\n", s.used, s.free);
 }
+
+void *malloc_add_pool(void *mem, size_t bytes)
+{
+	pool_t new_pool;
+	struct pool_entry *new_pool_entry;
+
+	if (!mem)
+		return NULL;
+
+	new_pool_entry = malloc(sizeof(struct pool_entry));
+	if (!new_pool_entry)
+		return NULL;
+
+	new_pool = tlsf_add_pool(tlsf_mem_pool, mem, bytes);
+	if (!new_pool) {
+		free(new_pool_entry);
+		return NULL;
+	}
+
+	kasan_poison_shadow(mem, bytes, KASAN_TAG_INVALID);
+
+	new_pool_entry->pool = new_pool;
+	list_add(&new_pool_entry->list, &mem_pool_list);
+
+	return (void *)new_pool;
+}
+
+int malloc_remove_pool(void *pool)
+{
+	struct pool_entry *cur_pool;
+	struct malloc_stats s;
+
+	s.used = 0;
+	s.free = 0;
+
+	list_for_each_entry(cur_pool, &mem_pool_list, list) {
+		if (cur_pool->pool == (pool_t)pool) {
+			tlsf_walk_pool(cur_pool->pool, malloc_walker, &s);
+			if (s.used)
+				return -EBUSY;
+
+			list_del(&(cur_pool->list));
+			free(cur_pool);
+			tlsf_remove_pool(tlsf_mem_pool, pool);
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
diff --git a/include/malloc.h b/include/malloc.h
index 0b74746360c08a95e593c9afe485382d02cd5c12..a3684105981eb4a27594c7cb9a71191ca9dceff3 100644
--- a/include/malloc.h
+++ b/include/malloc.h
@@ -21,6 +21,11 @@
 #define ZERO_OR_NULL_PTR(x) ((unsigned long)(x) <= \
 				(unsigned long)ZERO_SIZE_PTR)
 
+#ifdef CONFIG_MALLOC_TLSF
+void *malloc_add_pool(void *mem, size_t bytes);
+int malloc_remove_pool(void *pool);
+#endif
+
 #if IN_PROPER
 void *malloc(size_t) __alloc_size(1);
 size_t malloc_usable_size(void *);

---
base-commit: d4b5f3d3d1919189c2a8ebbc6ecc531c5459acf8
change-id: 20250131-tlsf-addpool-5f42029b582c

Best regards,
-- 
David Dgien <dgienda125@gmail.com>





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

* Re: [PATCH] tlsf: Add tracking of added tlsf memory pools
  2025-03-29  3:13 [PATCH] tlsf: Add tracking of added tlsf memory pools David Dgien via B4 Relay
@ 2025-04-02  9:07 ` Sascha Hauer
  2025-04-03  2:23   ` David Dgien
  0 siblings, 1 reply; 3+ messages in thread
From: Sascha Hauer @ 2025-04-02  9:07 UTC (permalink / raw)
  To: David Dgien via B4 Relay; +Cc: BAREBOX, David Dgien, Tyler Reece

Hi David,

On Fri, Mar 28, 2025 at 11:13:40PM -0400, David Dgien via B4 Relay wrote:
> From: David Dgien <dgienda125@gmail.com>
> 
> When configured to use the TLSF allocator, the malloc_stats function
> only walks the initial memory pool, as that is the only pool it is aware
> of, and TLSF doesn't link pools together in a way that the walker can
> follow.
> 
> Add wrapper functions around tlsf_add_pool and tlsf_remove_pool to add
> additional tracking of added pools, so that they can be walked by the
> malloc_stats function and meminfo can report accurate heap utilization.
> 
> Signed-off-by: David Dgien <dgienda125@gmail.com>
> Reviewed-by: Tyler Reece <mtreece@acm.org>
> ---
> For this change I put the wrapper function prototypes in malloc.h, gated
> by CONFIG_MALLOC_TLSF. I was split between there and memory.h, but I'm
> not entirely convinced malloc.h was the right decision. I also didn't
> provide stubs for when CONFIG_MALLOC_TLSF isn't enabled, as I did not
> see any upstream code that utilizes tlsf_add_pool. If the project has
> different preferences I can make those changes.

I think it's fine the way you did it for now. We have plans to remove
dlmalloc support in the near future which gives us an opportunity to
rework this and make malloc_add_pool() generally available.

Currently barebox only uses the initial memory. With dlmalloc removed we
could make use of the remaining memory banks using malloc_add_pool().

> ---
>  common/tlsf_malloc.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/malloc.h     |  5 ++++
>  2 files changed, 69 insertions(+)
> 
> diff --git a/common/tlsf_malloc.c b/common/tlsf_malloc.c
> index 6a90ee47199fa9e8223f843cc95f52eebfec2aee..0e756a13d5a2665eccf8931b19dfa4e56cad15eb 100644
> --- a/common/tlsf_malloc.c
> +++ b/common/tlsf_malloc.c
> @@ -12,8 +12,18 @@
>  #include <module.h>
>  #include <tlsf.h>
>  
> +#include <linux/kasan.h>
> +#include <linux/list.h>
> +
>  extern tlsf_t tlsf_mem_pool;
>  
> +struct pool_entry {
> +	pool_t pool;
> +	struct list_head list;
> +};
> +
> +static LIST_HEAD(mem_pool_list);
> +
>  void *malloc(size_t bytes)
>  {
>  	void *mem;
> @@ -75,6 +85,7 @@ static void malloc_walker(void* ptr, size_t size, int used, void *user)
>  
>  void malloc_stats(void)
>  {
> +	struct pool_entry *cur_pool;
>  	struct malloc_stats s;
>  
>  	s.used = 0;
> @@ -82,5 +93,58 @@ void malloc_stats(void)
>  
>  	tlsf_walk_pool(tlsf_get_pool(tlsf_mem_pool), malloc_walker, &s);
>  
> +	list_for_each_entry(cur_pool, &mem_pool_list, list)
> +		tlsf_walk_pool(cur_pool->pool, malloc_walker, &s);

Can you create a pool_entry for the initial malloc area as well so that
the list contains all pools?

> +
>  	printf("used: %zu\nfree: %zu\n", s.used, s.free);
>  }
> +
> +void *malloc_add_pool(void *mem, size_t bytes)
> +{
> +	pool_t new_pool;
> +	struct pool_entry *new_pool_entry;
> +
> +	if (!mem)
> +		return NULL;
> +
> +	new_pool_entry = malloc(sizeof(struct pool_entry));
> +	if (!new_pool_entry)
> +		return NULL;
> +
> +	new_pool = tlsf_add_pool(tlsf_mem_pool, mem, bytes);
> +	if (!new_pool) {
> +		free(new_pool_entry);
> +		return NULL;
> +	}
> +
> +	kasan_poison_shadow(mem, bytes, KASAN_TAG_INVALID);
> +
> +	new_pool_entry->pool = new_pool;
> +	list_add(&new_pool_entry->list, &mem_pool_list);
> +
> +	return (void *)new_pool;
> +}
> +
> +int malloc_remove_pool(void *pool)
> +{
> +	struct pool_entry *cur_pool;
> +	struct malloc_stats s;
> +
> +	s.used = 0;
> +	s.free = 0;
> +
> +	list_for_each_entry(cur_pool, &mem_pool_list, list) {
> +		if (cur_pool->pool == (pool_t)pool) {
> +			tlsf_walk_pool(cur_pool->pool, malloc_walker, &s);
> +			if (s.used)
> +				return -EBUSY;

Do we need this function at all? We have no way to move allocations away
from the pool being removed, so there's no way to make this function
work reliably. What's the point in having it?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

* Re: [PATCH] tlsf: Add tracking of added tlsf memory pools
  2025-04-02  9:07 ` Sascha Hauer
@ 2025-04-03  2:23   ` David Dgien
  0 siblings, 0 replies; 3+ messages in thread
From: David Dgien @ 2025-04-03  2:23 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: David Dgien via B4 Relay, BAREBOX, Tyler Reece

On Wed, Apr 2, 2025 at 5:07 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> Hi David,
>
> On Fri, Mar 28, 2025 at 11:13:40PM -0400, David Dgien via B4 Relay wrote:
> > From: David Dgien <dgienda125@gmail.com>
> >
> > When configured to use the TLSF allocator, the malloc_stats function
> > only walks the initial memory pool, as that is the only pool it is aware
> > of, and TLSF doesn't link pools together in a way that the walker can
> > follow.
> >
> > Add wrapper functions around tlsf_add_pool and tlsf_remove_pool to add
> > additional tracking of added pools, so that they can be walked by the
> > malloc_stats function and meminfo can report accurate heap utilization.
> >
> > Signed-off-by: David Dgien <dgienda125@gmail.com>
> > Reviewed-by: Tyler Reece <mtreece@acm.org>
> > ---
> > For this change I put the wrapper function prototypes in malloc.h, gated
> > by CONFIG_MALLOC_TLSF. I was split between there and memory.h, but I'm
> > not entirely convinced malloc.h was the right decision. I also didn't
> > provide stubs for when CONFIG_MALLOC_TLSF isn't enabled, as I did not
> > see any upstream code that utilizes tlsf_add_pool. If the project has
> > different preferences I can make those changes.
>
> I think it's fine the way you did it for now. We have plans to remove
> dlmalloc support in the near future which gives us an opportunity to
> rework this and make malloc_add_pool() generally available.
>
> Currently barebox only uses the initial memory. With dlmalloc removed we
> could make use of the remaining memory banks using malloc_add_pool().
>
> > ---
> >  common/tlsf_malloc.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/malloc.h     |  5 ++++
> >  2 files changed, 69 insertions(+)
> >
> > diff --git a/common/tlsf_malloc.c b/common/tlsf_malloc.c
> > index 6a90ee47199fa9e8223f843cc95f52eebfec2aee..0e756a13d5a2665eccf8931b19dfa4e56cad15eb 100644
> > --- a/common/tlsf_malloc.c
> > +++ b/common/tlsf_malloc.c
> > @@ -12,8 +12,18 @@
> >  #include <module.h>
> >  #include <tlsf.h>
> >
> > +#include <linux/kasan.h>
> > +#include <linux/list.h>
> > +
> >  extern tlsf_t tlsf_mem_pool;
> >
> > +struct pool_entry {
> > +     pool_t pool;
> > +     struct list_head list;
> > +};
> > +
> > +static LIST_HEAD(mem_pool_list);
> > +
> >  void *malloc(size_t bytes)
> >  {
> >       void *mem;
> > @@ -75,6 +85,7 @@ static void malloc_walker(void* ptr, size_t size, int used, void *user)
> >
> >  void malloc_stats(void)
> >  {
> > +     struct pool_entry *cur_pool;
> >       struct malloc_stats s;
> >
> >       s.used = 0;
> > @@ -82,5 +93,58 @@ void malloc_stats(void)
> >
> >       tlsf_walk_pool(tlsf_get_pool(tlsf_mem_pool), malloc_walker, &s);
> >
> > +     list_for_each_entry(cur_pool, &mem_pool_list, list)
> > +             tlsf_walk_pool(cur_pool->pool, malloc_walker, &s);
>
> Can you create a pool_entry for the initial malloc area as well so that
> the list contains all pools?

I should be able to do that.

Since the initial memory pool would never be able to be removed, I
thought it would be better to not put it in the list at all. But if
I'm removing the ability to remove pools, then I can add the initial
malloc area to the list and simplify things.

>
> > +
> >       printf("used: %zu\nfree: %zu\n", s.used, s.free);
> >  }
> > +
> > +void *malloc_add_pool(void *mem, size_t bytes)
> > +{
> > +     pool_t new_pool;
> > +     struct pool_entry *new_pool_entry;
> > +
> > +     if (!mem)
> > +             return NULL;
> > +
> > +     new_pool_entry = malloc(sizeof(struct pool_entry));
> > +     if (!new_pool_entry)
> > +             return NULL;
> > +
> > +     new_pool = tlsf_add_pool(tlsf_mem_pool, mem, bytes);
> > +     if (!new_pool) {
> > +             free(new_pool_entry);
> > +             return NULL;
> > +     }
> > +
> > +     kasan_poison_shadow(mem, bytes, KASAN_TAG_INVALID);
> > +
> > +     new_pool_entry->pool = new_pool;
> > +     list_add(&new_pool_entry->list, &mem_pool_list);
> > +
> > +     return (void *)new_pool;
> > +}
> > +
> > +int malloc_remove_pool(void *pool)
> > +{
> > +     struct pool_entry *cur_pool;
> > +     struct malloc_stats s;
> > +
> > +     s.used = 0;
> > +     s.free = 0;
> > +
> > +     list_for_each_entry(cur_pool, &mem_pool_list, list) {
> > +             if (cur_pool->pool == (pool_t)pool) {
> > +                     tlsf_walk_pool(cur_pool->pool, malloc_walker, &s);
> > +                     if (s.used)
> > +                             return -EBUSY;
>
> Do we need this function at all? We have no way to move allocations away
> from the pool being removed, so there's no way to make this function
> work reliably. What's the point in having it?

The intent was to match the symmetry of the TLSF add/remove functions.
I will remove it from the next version, assuming removing unused pools
is not a useful feature.

-- David Dgien

>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

end of thread, other threads:[~2025-04-03  2:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-29  3:13 [PATCH] tlsf: Add tracking of added tlsf memory pools David Dgien via B4 Relay
2025-04-02  9:07 ` Sascha Hauer
2025-04-03  2:23   ` David Dgien

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox