mm/mm_heap : realtime support for malloc/free functions performing wi… #3398

davidfather · 2019-06-15T09:49:37Z

…thin a constant time

Do memory allocation and releasing within a constant time
if the requested size is less than or equal to (16 * MM_REALTIME_SUPPORT_NUMOF_SIZES),
where MM_REALTIME_SUPPORT_NUMOF_SIZES is the number of sizes for realtime support,
starting from 16 bytes with 16 bytes step.
For example, if MM_REALTIME_SUPPORT_NUMOF_SIZES is 3,
16, 32, 48 bytes memory can be allocated within a constant time.
This number should be positive integer.

MM_REALTIME_SUPPORT_NUMS is the list of numbers each of which means the number of
a certain-sized free nodes which can be used for memory allocation within a constant time.
For example, by default, MM_REALTIME_SUPPORT_NUMS = "100 100 100" means
a hundred free nodes for each size, i.e, 16, 32, 48 bytes,
are reserved for realtime support. The total amount of memory sizes
for realtime support is calculated by (16 * N1 + 32 * N2 + ... + 16 * n * Nn) bytes,
where n is the number of sizes (i.e., MM_REALTIME_SUPPORT_NUMOF_SIZES)
and Nn is the number of free nodes for the size of 16 * n bytes.
These numbers should not be negative integers.
If n is larger than the number of these numbers, non-defined numbers of free nodes
become, by default, zero.

seinfra · 2019-06-15T09:51:14Z

Target : [006d616eb3bff6dc9380778460179f076c815d60] - Code Rule Check (C++) OK.

seinfra · 2019-06-15T09:51:18Z

Target : [006d616eb3bff6dc9380778460179f076c815d60] - Code Rule Check Result:
os/mm/mm_heap/mm_initialize.c:158: ERROR: [SPC_M_KWD] space required before the open parenthesis '('
os/mm/mm_heap/mm_initialize.c:160: ERROR: [SPC_M_KWD] space required before the open parenthesis '('
os/mm/mm_heap/mm_size2ndx.c:88: ERROR: [SPC_M_KWD] space required before the open parenthesis '('
os/mm/mm_heap/mm_size2ndx.c:130: ERROR: [SPC_M_KWD] space required before the open parenthesis '('

seinfra · 2019-06-15T10:09:16Z

Target : [eac8f6315338d20ea3a0357bcf528661c22c7163] - Code Rule Check (C++) OK.

seinfra · 2019-06-15T10:09:21Z

Target : [eac8f6315338d20ea3a0357bcf528661c22c7163] - Code Rule Check Result:
os/mm/mm_heap/mm_initialize.c:158: ERROR: [SPC_M_KWD] space required before the open parenthesis '('
os/mm/mm_heap/mm_initialize.c:160: ERROR: [SPC_M_KWD] space required before the open parenthesis '('
os/mm/mm_heap/mm_size2ndx.c:88: ERROR: [SPC_M_KWD] space required before the open parenthesis '('
os/mm/mm_heap/mm_size2ndx.c:130: ERROR: [SPC_M_KWD] space required before the open parenthesis '('

seinfra · 2019-06-15T10:36:12Z

Target : [b21610aa00c5f1d4b7ec8271c304d868f6af2145] - Code Rule Check (C++) OK.

seinfra · 2019-06-15T10:36:15Z

Target : [b21610aa00c5f1d4b7ec8271c304d868f6af2145] - Code Rule Check Result:
os/mm/mm_heap/mm_initialize.c:158: ERROR: [SPC_M_KWD] space required before the open parenthesis '('
os/mm/mm_heap/mm_initialize.c:160: ERROR: [SPC_M_KWD] space required before the open parenthesis '('

seinfra · 2019-06-15T10:39:12Z

Target : [2fe08620cc5f5a0c653eb63d991f16cb62858b77] - Code Rule Check (C++) OK.

seinfra · 2019-06-15T10:39:18Z

Target : [2fe08620cc5f5a0c653eb63d991f16cb62858b77] - Code Rule Check Result:
os/mm/mm_heap/mm_initialize.c:158: ERROR: [SPC_M_KWD] space required before the open parenthesis '('
os/mm/mm_heap/mm_initialize.c:160: ERROR: [SPC_M_KWD] space required before the open parenthesis '('

seinfra · 2019-06-15T10:45:13Z

Target : [c57311982eb4d58d28cc9123404f7f8b389b2702] - Code Rule Check OK.

seinfra · 2019-06-15T10:45:15Z

Target : [c57311982eb4d58d28cc9123404f7f8b389b2702] - Code Rule Check (C++) OK.

NilabhShant · 2019-06-17T08:57:06Z

os/include/tinyara/mm/mm.h

 #define MM_NNODES        (MM_MAX_SHIFT - MM_MIN_SHIFT + 1)
+#endif // CONFIG_MM_REALTIME_SUPPORT

 #define MM_GRAN_MASK     (MM_MIN_CHUNK-1)


Please provide space before and after '-'

This line is not modified. Let's just keep it as it is.

NilabhShant · 2019-06-17T08:58:03Z

apps/examples/heap_stat_realtime/Makefile

+
+else
+install:
+


Please remove this extra line.

This is like a template. Let's just keep it as it is.

NilabhShant · 2019-06-17T08:58:21Z

apps/examples/heap_stat_realtime/Makefile

+
+else
+context:
+


same as above.

This is like a template. Let's just keep it as it is.

NilabhShant · 2019-06-17T09:02:53Z

apps/examples/heap_stat_realtime/heap_stat_realtime.c

+{
+	char *addr;
+	int ndx;
+	uint32_t miss[MM_REALTIME_SUPPORT_NUMOF_SIZES] = {0, };


Please provide space after open brace.
like: { 0, }

seinfra · 2019-06-18T02:34:21Z

Target : [39bc5e7384e2285ac42d7bd51d0b91e29d8f8c10] - Code Rule Check (C++) OK.

seinfra · 2019-06-18T02:34:34Z

Target : [39bc5e7384e2285ac42d7bd51d0b91e29d8f8c10] - Code Rule Check OK.

Taejun-Kwon · 2019-06-19T01:45:24Z

apps/examples/heap_stat_realtime/heap_stat_realtime.c

+
+	addr = (char *)malloc(sizeof(char));
+
+	if (argc > 2) {


3 comments here.

null check after malloc needed but I think you can use unsigned with out mem alloc.

You'd better modify it to switch-case if you consider expand it.

argc > 2 is intended code? actually you already mentioned usage in else state, it means you restrict usage.
So Error case also should be there if argc is greater than 2

Here, my feedback.

Null check is necessary, but this mem alloc is intended to get heap stat.

I'll do when I need to extend this function further.

if argc <= 2, 'heapstat' just goes well.

I mean if user type heapstat reset blah blah blah it also works. is it intended? of course some os accepts it but some other doesn't accept it. I was just curious because you added usaage below..

Yes, it was intentionally allowed to keep the code simple. Thanks for your comment, though.

Taejun-Kwon · 2019-06-19T02:00:11Z

os/mm/mm_heap/mm_free.c

@@ -149,6 +149,9 @@ void mm_free(FAR struct mm_heap_s *heap, FAR void *mem)
 	/* Check if the following node is free and, if so, merge it */

 	next = (FAR struct mm_freenode_s *)((char *)node + node->size);
+#ifdef  CONFIG_MM_REALTIME_SUPPORT


Below is our style.

#ifdef CONFIG_MM_REALTIME_SUPPORT if (A) #endif { do_something(); }

Fixed. Thanks.

Taejun-Kwon · 2019-06-19T02:15:25Z

os/mm/mm_heap/mm_heapstat.c

+	struct mm_heap_s *heap;
+	int ndx;
+
+	heap = mm_get_heap(addr);


I think you'd better change type from void, and return error if mm_get_heap returns null

Fixed. Thanks.

Taejun-Kwon · 2019-06-19T02:15:48Z

os/mm/mm_heap/mm_heapstat.c

+	mm_givesemaphore(heap);
+}
+
+void mm_heapstat_reset(char *addr)


same as above.

Fixed. Thanks.

Taejun-Kwon · 2019-06-19T02:16:53Z

os/mm/mm_heap/mm_heapstat.c

+		heap->mm_realtime_miss[ndx] = 0;
+	}
+	mm_givesemaphore(heap);
+}


When we separate binary, application call these 2 apis? It's not registered syscall.
I Think you'd better expand proc if you want to display info of heap stat in application..

These are included in user library.

Taejun-Kwon · 2019-06-19T02:20:41Z

os/mm/mm_heap/mm_initialize.c

+
+	for (i = 0; i < MM_REALTIME_SUPPORT_NUMOF_SIZES; ++i) {
+		if (heap->mm_realtime_num[i] < 1) {
+			heap->mm_realtime_num[i] = 0;


How about use unsigned int?

I don't think so because it's not a size but a signed number.

You mean it allowed negative value?

No, but I checked whether the input is negative at line 170.
However, if this var is unsigned, it might be greater than the maximum number.

I mentioned unsigned type because it is stable for kind of size of buffer, address, length, and so on.
We usually use size_t or ssize_t for it, That's why I commented it, choice is yours.

Taejun-Kwon · 2019-06-19T02:22:19Z

os/mm/mm_heap/mm_initialize.c

+
+	/* First, add one big chunk of free node for non-realtime. */
+	node            = (FAR struct mm_freenode_s *)(heapbase + SIZEOF_MM_ALLOCNODE);
+	node->size      = heapsize - 2 * SIZEOF_MM_ALLOCNODE - mem_pool_size;


You had better add bracket for multiplication.

I don't think it's necessary, but fixed.

yes minor one, just for readability, thanks.

JeongJunSik · 2019-06-19T02:56:31Z

apps/examples/heap_stat_realtime/heap_stat_realtime.c

+			mm_heapstat_reset(addr);
+			printf("\nReset the stat of heap for realtime support.");
+		} else {
+			printf("Usage:	%s [reset]\n", argv[1]);


Don't you need free(addr) here?

Good point, dear.
Fixed. Thanks.

YiHyunJin · 2019-06-20T01:13:53Z

apps/examples/heap_stat_realtime/heap_stat_realtime.c

+	size_t miss[MM_REALTIME_SUPPORT_NUMOF_SIZES] = { 0, };
+	int num[MM_REALTIME_SUPPORT_NUMOF_SIZES] = { 0, };
+
+	addr = (char *)malloc(sizeof(char));


I think so that add free(addr)

Fixed. Thanks.

seinfra · 2019-06-20T02:03:26Z

Target : [51e7c0d1d16daa63a5cd18e9e5917c32d9e62c11] - Code Rule Check (C++) OK.

seinfra · 2019-06-20T02:03:27Z

Target : [51e7c0d1d16daa63a5cd18e9e5917c32d9e62c11] - Code Rule Check OK.

YiHyunJin · 2019-06-20T03:13:05Z

os/include/tinyara/mm/mm.h

@@ -671,6 +695,9 @@ void mm_addfreechunk(FAR struct mm_heap_s *heap, FAR struct mm_freenode_s *node)
 /* Functions contained in mm_size2ndx.c.c ***********************************/

 int mm_size2ndx(size_t size);
+#ifdef CONFIG_MM_REALTIME_SUPPORT


What about the below?

#ifdef CONFIG_MM_REALTIME_SUPPORT int mm_size2ndx_realtime(FAR struct mm_heap_s *heap, size_t size); #else int mm_size2ndx(size_t size); #endif

Thanks for the comment, but it's an additional function in case of realtime support.

JeongJunSik · 2019-06-20T03:48:09Z

os/mm/mm_heap/mm_initialize.c

+#endif
+
+	size_t tsize = (MM_REALTIME_SUPPORT_NUMOF_SIZES + 2) * MM_REALTIME_SUPPORT_STEP_SIZE;
+	heap->mm_ndx_offset = (tsize >> MM_MIN_SHIFT) - mm_size2ndx(tsize) - 2;


Isn't mm_size2ndx_realtime() instead of mm_size2ndx()?

Thanks for the comment, but in order to compute the offset the original function, mm_size2ndx(), is required.

…thin a constant time Do memory allocation and releasing within a constant time if the requested size is less than or equal to (16 * MM_REALTIME_SUPPORT_NUMOF_SIZES), where MM_REALTIME_SUPPORT_NUMOF_SIZES is the number of sizes for realtime support, starting from 16 bytes with 16 bytes step. For example, if MM_REALTIME_SUPPORT_NUMOF_SIZES is 3, 16, 32, 48 bytes memory can be allocated within a constant time. This number should be positive integer. MM_REALTIME_SUPPORT_NUMS is the list of numbers each of which means the number of a certain-sized free nodes which can be used for memory allocation within a constant time. For example, by default, MM_REALTIME_SUPPORT_NUMS = "100 100 100" means a hundred free nodes for each size, i.e, 16, 32, 48 bytes, are reserved for realtime support. The total amount of memory sizes for realtime support is calculated by (16 * N1 + 32 * N2 + ... + 16 * n * Nn) bytes, where n is the number of sizes (i.e., MM_REALTIME_SUPPORT_NUMOF_SIZES) and Nn is the number of free nodes for the size of 16 * n bytes. These numbers should not be negative integers. If n is larger than the number of these numbers, non-defined numbers of free nodes become, by default, zero.

…stat of heap for realtime support 'heapstat' is an utility app which shows the number of memory requests not allocated within a constant time. When MM_REALTIME_SUPPORT_NUMOF_SIZES = 8 and MM_REALTIME_SUPPORT_NUMS = "200 200 0 150 0 0 0 150", 'heapstat' might print out the current status as shown below: Size 16 bytes : 10 times, given 200 reserved nodes. Size 32 bytes : 20 times, given 200 reserved nodes. Size 48 bytes : 30 times, given 0 reserved nodes. Size 64 bytes : 0 times, given 150 reserved nodes. Size 80 bytes : 5 times, given 0 reserved nodes. Size 96 bytes : 0 times, given 0 reserved nodes. Size 112 bytes : 0 times, given 0 reserved nodes. Size 128 bytes : 8 times, given 150 reserved nodes.

seinfra · 2019-06-22T11:03:18Z

Target : [c5107ec] - Code Rule Check OK.

seinfra · 2019-06-22T11:03:19Z

Target : [c5107ec] - Code Rule Check (C++) OK.

drashti304 · 2019-06-24T06:40:07Z

apps/examples/heap_stat_realtime/Kconfig_ENTRY

@@ -0,0 +1,3 @@
+config ENTRY_HEAP_STAT_REALTIME


Hi,

Please add below Kconfig header as we follow in TizenRT.

# # For a description of the syntax of this configuration file, # see kconfig-language at https://www.kernel.org/doc/Documentation/kbuild/kconfig-language.txt #

I didn't see the same header in other Kconfig_ENTRY files.
I will add this header in all Kconfig_ENTRY files if necessary.

tizen-build · 2020-04-10T03:09:02Z

Target : [c5107ec] - Code Rule Check (C++) OK.

tizen-build · 2020-04-10T05:13:25Z

Target : [c5107ec] - Code Rule Check OK.

tizen-build · 2023-03-21T02:16:45Z

Target : [c5107ec] - Code Rule Check OK.

davidfather requested review from jeongarmy, jeongchanKim, sunghan-chang and YiHyunJin June 15, 2019 09:50

davidfather force-pushed the heap_enhance branch from 006d616 to eac8f63 Compare June 15, 2019 10:06

davidfather force-pushed the heap_enhance branch from b21610a to 2fe0862 Compare June 15, 2019 10:38

davidfather force-pushed the heap_enhance branch from 2fe0862 to c573119 Compare June 15, 2019 10:42

davidfather mentioned this pull request Jun 15, 2019

mm_heap : add a simple implementation of memory pool #2736

Closed

NilabhShant reviewed Jun 17, 2019

View reviewed changes

davidfather force-pushed the heap_enhance branch from c573119 to 39bc5e7 Compare June 18, 2019 02:28

Taejun-Kwon reviewed Jun 19, 2019

View reviewed changes

JeongJunSik reviewed Jun 19, 2019

View reviewed changes

YiHyunJin reviewed Jun 20, 2019

View reviewed changes

davidfather force-pushed the heap_enhance branch from 39bc5e7 to 51e7c0d Compare June 20, 2019 02:00

YiHyunJin reviewed Jun 20, 2019

View reviewed changes

JeongJunSik reviewed Jun 20, 2019

View reviewed changes

davidfather added 2 commits June 22, 2019 03:59

davidfather force-pushed the heap_enhance branch from 51e7c0d to c5107ec Compare June 22, 2019 11:00

drashti304 reviewed Jun 24, 2019

View reviewed changes


		else
		install:


		else
		context:

mm/mm_heap : realtime support for malloc/free functions performing wi… #3398

Are you sure you want to change the base?

mm/mm_heap : realtime support for malloc/free functions performing wi… #3398

Conversation

davidfather commented Jun 15, 2019

seinfra commented Jun 15, 2019

seinfra commented Jun 15, 2019

seinfra commented Jun 15, 2019

seinfra commented Jun 15, 2019

seinfra commented Jun 15, 2019

seinfra commented Jun 15, 2019

seinfra commented Jun 15, 2019

seinfra commented Jun 15, 2019

seinfra commented Jun 15, 2019

seinfra commented Jun 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seinfra commented Jun 18, 2019

seinfra commented Jun 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seinfra commented Jun 20, 2019

seinfra commented Jun 20, 2019

YiHyunJin Jun 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seinfra commented Jun 22, 2019

seinfra commented Jun 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tizen-build commented Apr 10, 2020

tizen-build commented Apr 10, 2020

tizen-build commented Mar 21, 2023

YiHyunJin Jun 20, 2019 •

edited

Loading