Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc] Create debug version of malloc, fix near->far malloc conversion bug #2129

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

ghaerr
Copy link
Owner

@ghaerr ghaerr commented Dec 12, 2024

Fixes bug in malloc that returned non-zero DS:0 on failure, rather than 0:0 (NULL) in large model. Discussed in rafael2k/8086-toolchain#19.

Adds debugging display to malloc and fmemalloc. When 'debug=1` is set in /bootopts, the sizes in bytes of malloc and fmemalloc will be displayed along with the process id, for easier tracking of near and far memory allocations used by the under-development 8086-toolchain. When malloc or fmemalloc fails, that will also be displayed to let the developer know the application is out of memory.

For instance, when running LD86, the following is displayed.

    ./ld86 -0 -i test.o libc86.a -o test
(15)SBRK 148
(15)SBRK 32
(15)SBRK 32
(15)BRK 10120
(15)SBRK 32
(15)SBRK 68
(16)SBRK 150
MALLOC 3074
(16)SBRK 3076
MALLOC 1026
(16)SBRK 1028
MALLOC 1026
(16)SBRK 1028
MALLOC 2050
(16)SBRK 2052
MALLOC 1026
(16)SBRK 1028
(16)FMEMALLOC 57344

The MALLOC value is the requested allocation (+2 for internal tracking), and the SBRK value is the data segment heap expansion value (+ another 2 for heap tracking).

This allows for much better visibility on which tools ask for near vs far memory and how much is requested. Currently, no attempt is made to determine remaining free space or summing the allocations and frees to determine working set storage requirements.

@rafael2k: I haven't tested this on your last couple of days enhancements, as I'm in the middle of some C86 enhancements. Thus, my versions don't have the bug fixes you've run into regarding memrealloc and others. However, this PR does fix the problem of malloc returning DS:0 rather than NULL (0:), so the NULLPTR defines should not be necessary anymore, although it won't hurt to leave them in.

When you have time, please test this with your latest build (requires OWC libc rebuild and updated kernel, of course). I'm interested to hear what you think with regards to the memory allocations being used with the latest toolchain fixes.

@ghaerr ghaerr merged commit 0ff03dd into master Dec 12, 2024
2 checks passed
@ghaerr ghaerr deleted the malloc branch December 12, 2024 05:59
@rafael2k
Copy link
Contributor

I'll rebuild ELKS here and retest. Thanks!
We the commit previous to this, I can build the examples fine, after tweaking more nasm memory allocations - already committed.

@rafael2k
Copy link
Contributor

Btw, now and I remove that NULLPTR macro and just compare __far pointers to NULL?

@ghaerr
Copy link
Owner Author

ghaerr commented Dec 12, 2024

now and I remove that NULLPTR macro and just compare __far pointers to NULL?

Yes. The previous problem of malloc returning a non-zero value when out of memory is fixed.

BTW, after running my (older) version of the toolchain with this PR and seeing the large number of small fmemalloc's, assumedly required by your setting of MAX_NEAR_ALLOC to 64 in order to run, I think I understand what is really happening: the application is running out of near heap memory, and has to use fmemalloc in order to get enough memory to run. This in turn is using tons of kernel heap manager segments to manage the large number of main memory fmemalloc segments. In some cases, the kernel heap itself runs out of memory, just from the 16 bytes each it requires for each fmemalloc segment.

In other words, my idea of using 'memalloc' as a wrapper currently to divert allocations between only malloc and fmemalloc is not sufficient. We're going to have to move to a memory manager that likely allocates a full 64k near heap used for small (< 1024 byte?) allocations, and then fmemalloc for the larger allocations. We may even have to manage multiple 64k heaps in addition to using fmemalloc. The output of this PR will tell us the amounts and types of memory required.

Adding a wholly new memory allocator is potentially very complicated and could produce bugs, but I'm starting to think about algorithms we might use that reduce that risk. I hope to hide the entire implementation behind memalloc/memrealloc/memfree, and not have to worry about a special MAX_NEAR_ALLOC value for each tool - it will all just work.

In the meantime we can keep using what you have working, as it still works well enough for the larger chess.c program for the time being.

@rafael2k
Copy link
Contributor

I think this new arena for malloc allocations will really help, as we are running out of heap very fast. ld86 for example have an option behind a define (we are not using it) that allocates a big buffer and then handle internally small chunks allocation from this previously allocated by buffer.

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.

2 participants