Skip to content

Conversation

@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 commented Aug 3, 2023

Summary

since kernel just allocate memory from kmm_malloc

Impact

code refactor only since lib_free call kmm_free when is compiled in kernel spacee

Testing

ci

since kernel just allocate memory from kmm_malloc

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
@acassis acassis merged commit 7884c18 into apache:master Aug 6, 2023
@xiaoxiang781216 xiaoxiang781216 deleted the lib_free branch August 6, 2023 09:54
@mu578
Copy link

mu578 commented Aug 6, 2023

Speaking of which:

void *_malloc_r(struct _reent *r, size_t size)

there is a use of plain malloc/free. They might be other few spots where it happens, e.g not using indirection. I think in minicxx operator new/delete should also use on lib_*. Maybe would have been easier to make libmalloc build according to target.

@xiaoxiang781216
Copy link
Contributor Author

xiaoxiang781216 commented Aug 6, 2023

Speaking of which:

void *_malloc_r(struct _reent *r, size_t size)

@acassis esp use the wrong function.

there is a use of plain malloc/free. They might be other few spots where it happens, e.g not using indirection. I think in minicxx operator new/delete should also use on lib_*.

No, minicxx doesn't need call lib_* since cxx is only allowed to be used in userspace.

Maybe would have been easier to make libmalloc build according to target.

Only components which are compiled and used by both kernel and usespace(e.g. libc/libkc, libmm/libkmm) need call lib_xxx.

@pkarashchenko
Copy link
Contributor

There are many places like it. I have changes stashed somewhere and will push change for review soon

@royfengsss
Copy link
Contributor

Could you consider below issue:

  1. configure build with esp32-devkitc:psram_usrheap
  2. enable assertion in config
  3. build and run on a ESP32 DevkitC board
  4. Just run ls under nsh prompt, below assertion found:
NuttShell (NSH)
nsh> ls
/:
_assert: Current Version: NuttX  0.0.0 7884c18620 Aug  7 2023 10:14:10 xtensa
_assert: Assertion failed (mem == ((void*)0)) || kmm_heapmember(mem): at file: kmm_heap/kmm_free.c:55 task: nsh_main 0x400d9600
up_dump_register:    PC: 400e4f09    PS: 00060022
up_dump_register:    A0: 800e1e6a    A1: 3f8006e0    A2: 00000000    A3: 3ffb2650
up_dump_register:    A4: 20000000    A5: 3f8008e0    A6: 00000008    A7: ff000000
up_dump_register:    A8: 00000001    A9: 3ffe0460   A10: 3ffb2650   A11: 0000007e
up_dump_register:   A12: 00060c20   A13: 00000000   A14: 00000008   A15: 3ffe0460
up_dump_register:   SAR: 00000000 CAUSE: 3f800690 VADDR: 00000008
up_dump_register:  LBEG: 4000c46c  LEND: 4000c477  LCNT: 00000000
dump_stack: User Stack:
dump_stack:   base: 0x3f800378
dump_stack:   size: 00001992
dump_stack:     sp: 0x3f8006e0
stack_dump: 0x3f8006e0: 3f405e8d 3f8006e0 3f400fef 00000037 800d75d5 3f8007f0 3f400fef 00000037
stack_dump: 0x3f800700: 3ffe04e4 400d9600 00000001 3f8007b8 7474754e 00000058 00000000 00000000
stack_dump: 0x3f800720: 00000000 00000000 07450dc7 ddf57c50 800eddf5 3f800760 3f8007f8 3f8007b8
stack_dump: 0x3f800740: 00000002 302e3000 0000302e 3f404786 800e3cf1 3f800780 383707a0 31633438
stack_dump: 0x3f800760: 30323638 67754120 20372020 33323032 3a303120 313a3431 00000030 3f404784
stack_dump: 0x3f800780: 3f800870 3f800850 00000008 65747803 0061736e 3f800840 3f800b50 3f404784
stack_dump: 0x3f8007a0: 00000000 3ffe0460 3ffb2650 3f400fc4 3f400fef 00000037 6f720a2f 00000063
stack_dump: 0x3f8007c0: 00000037 00060020 00000000 3f800e20 3f800b40 000007c8 00000000 3f8006e0
stack_dump: 0x3f8007e0: 800df504 3f800810 3f800e60 00060220 3f400fc4 400dadb8 00000000 3f800e30
stack_dump: 0x3f800800: 800de3b3 3f800830 00000000 3ffe06e0 00000000 00000200 00000000 00000001
stack_dump: 0x3f800820: 800de091 3f800850 3f800870 3ffb3fa4 3ffb4558 3f800e60 00000025 3f404786
stack_dump: 0x3f800840: 800de0a4 3f800870 3ffe0460 3ffe05f0 00000000 3ffe0600 3ffb3fa4 00000001
stack_dump: 0x3f800860: 800de0b4 3f8008a0 00000003 3f800e34 00000c01 00000002 3ffb3fa4 3ffe06e0
stack_dump: 0x3f800880: 3ffe0568 3ffe0568 00000008 00000003 800e371a 3f8008c0 00000003 3f800e34
stack_dump: 0x3f8008a0: 00000022 3f800850 00000008 3ffe0460 800dd0d3 3f8008e0 3f800e30 400d96e8
stack_dump: 0x3f8008c0: 00000022 3f8008e0 00000008 ff000000 800db4a3 3f800900 00000000 3f40428e
stack_dump: 0x3f8008e0: 3f800e34 3f800e35 3f800b50 3f800194 800dacbc 3f800920 3f800b50 3f800e20
stack_dump: 0x3f800900: 3f800e20 400dadb8 00000000 3f800e30 800da660 3f800980 3f800b50 00000001
stack_dump: 0x3f800920: 00000000 00000000 00004124 00000000 00000000 00000000 00000000 00000000
stack_dump: 0x3f800940: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
stack_dump: 0x3f800960: 3f40428e 00000000 00004000 00000001 800da785 3f8009a0 00000001 00000000
stack_dump: 0x3f800980: 3f8009a0 400db364 3f800dd8 3f800b50 800d98bd 3f800a60 3f800b50 3f800dd8
stack_dump: 0x3f8009a0: 3f800dd8 00000000 00000000 00000000 00000000 00000000 00000000 00000000
stack_dump: 0x3f8009c0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
stack_dump: 0x3f8009e0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 3f800dd8
stack_dump: 0x3f800a00: 00000003 00000000 00000000 0000002c 800d9896 3f800ddb 3f800dd8 00000040
stack_dump: 0x3f800a20: 00000001 3ffe0568 3ffe0648 0000002c 00000000 400d993c 00000002 00000000
stack_dump: 0x3f800a40: 3f800b50 3f800d50 00000000 00000000 800d966f 3f800a90 3f800b50 3f800dd8
stack_dump: 0x3f800a60: 00000001 3f800a90 3f800b50 3f800dd8 00000001 3f800d50 0000000a 3f800dd8
stack_dump: 0x3f800a80: 800d962b 3f800ab0 00000001 3f800358 00000001 3f403f7b 00060020 3ffb0498
stack_dump: 0x3f800aa0: 800d5fe8 3f800ad0 00000001 3f800358 3f800b50 3ffaffb0 00000000 00000000
stack_dump: 0x3f800ac0: 800d3bbb 3f800b00 400d9600 00000001 00000064 00000000 00000000 00000000
stack_dump: 0x3f800ae0: 3f40e2d0 3ffb00d8 00000001 00000000 00000000 3f800b20 00000000 00000000
stack_dump: 0x3f800b00: 3f800358 00000100 3ffaec5d 0fffffff 00000000 3f800b40 00000000 00000000
stack_dump: 0x3f800b20: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
 dev/
 proc/

The assertion happened in kmm_free() Via further debug, it could be found that kmm_free() was called by dir_close().

The cause of this assertion is that the memory to be freeed by kmm_free() is allocated in asprintf(), which used lib_malloc(). Please note that asprintf() is a libc function and built in libc module.

In libc building, __KERNEL__ is not defined and lib_malloc() should be malloc() rather than kmm_malloc().

Under the circumstance of CONFIG_ESP32_SPIRAM_USER_HEAP=y (Just esp32-devkitc:psram_usrheap like), malloc() is not equal to kmm_malloc...

#8974 is trying to fix this issue. This PR is acturally revert that fix.

@royfengsss
Copy link
Contributor

Besides asprintf(), another libc function strdup() have the similiar case. #10017 is to fix one of the case.

@xiaoxiang781216
Copy link
Contributor Author

Ok, CONFIG_ESP32_SPIRAM_USER_HEAP is a verry new option, @acassis @gustavonihei could you take a look.

@gustavonihei
Copy link
Contributor

Ok, CONFIG_ESP32_SPIRAM_USER_HEAP is a verry new option, @acassis @gustavonihei could you take a look.

@xiaoxiang781216 the need for library functions inside the kernel has been discussed previously here: https://github.com/apache/nuttx/pull/8974/files#r1159313382

@xiaoxiang781216
Copy link
Contributor Author

OK, look like my patch is wrong. @pkarashchenko could you revert my change in your PR.

@pkarashchenko
Copy link
Contributor

OK, look like my patch is wrong. @pkarashchenko could you revert my change in your PR.

Here it is #10110, but I have some limited availability to test it well and would appreciate support in verification part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants