gh-144319: Fix huge page leak in datastack chunk allocator#147963
gh-144319: Fix huge page leak in datastack chunk allocator#147963pablogsal wants to merge 1 commit intopython:mainfrom
Conversation
pablogsal
requested review from
FFY00,
ZeroIntensity and
ericsnowcurrently
as code owners
pablogsal
changed the title
The datastack chunk allocator (allocate_chunk in pystate.c) does not account for huge page granularity when requesting memory through _PyObject_VirtualAlloc. It requests small chunks (starting at 16KB = DATA_STACK_CHUNK_SIZE) and records chunk->size as the requested size, not the actual mapped size. When the arena allocator backs these with huge pages, the kernel maps a full 2MB page per request, but chunk->size still says 16KB.
|
CC: @Fidget-Spinner |
| assert(size_in_bytes % sizeof(PyObject **) == 0); | ||
| _PyStackChunk *res = _PyObject_VirtualAlloc(size_in_bytes); | ||
| size_t mapped_size = ROUND_UP_HUGEPAGE(size_in_bytes); | ||
| _PyStackChunk *res = _PyObject_VirtualAlloc(mapped_size); |
There was a problem hiding this comment.
Out of curiosity, why using PyMem_RawMalloc wouldn't be easier?
The comment even states:
Line 33 in 4497cf3
There was a problem hiding this comment.
Yes but I want to preserve the original intent of the code for the minimum bugfix. I haven't think of the subtle consequences of just getting a malloc chunk yet....
There was a problem hiding this comment.
I see! Does
Line 1010 in 4497cf3
| assert(size_in_bytes % sizeof(PyObject **) == 0); | ||
| _PyStackChunk *res = _PyObject_VirtualAlloc(size_in_bytes); | ||
| size_t mapped_size = ROUND_UP_HUGEPAGE(size_in_bytes); | ||
| _PyStackChunk *res = _PyObject_VirtualAlloc(mapped_size); |
There was a problem hiding this comment.
I see! Does
Line 1010 in 4497cf3
| * requested size) to _PyObject_VirtualFree, otherwise munmap receives a | ||
| * sub-hugepage length and fails with EINVAL, leaking the huge page. */ | ||
| #if defined(PYMALLOC_USE_HUGEPAGES) && defined(__linux__) | ||
| # define HUGEPAGE_SIZE (2U << 20) /* 2 MB */ |
There was a problem hiding this comment.
Maybe it could be a helper instead, leveraging
Line 546 in 4497cf3
_PyObject_VirtualAllocRounded, or similar
Fidget-Spinner
left a comment
Fidget-Spinner
left a comment
There was a problem hiding this comment.
Other than the stylistic stuff @maurycy pointed out, I think this looks fine.
3 participants
The datastack chunk allocator (allocate_chunk in pystate.c) does not account for huge page granularity when requesting memory through _PyObject_VirtualAlloc. It requests small chunks (starting at 16KB = DATA_STACK_CHUNK_SIZE) and records chunk->size as the requested size, not the actual mapped size. When the arena allocator backs these with huge pages, the kernel maps a full 2MB page per request, but chunk->size still says 16KB.