-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
MDEV-38918 Make large pages an explicit per-caller opt-in #5609
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
base: 10.11
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -95,6 +95,10 @@ C_MODE_START | |
| #define MY_ROOT_USE_VMEM 0x20000U /* init_alloc_root: use my_virtual_mem_commit */ | ||
| /* Tree that should delete things automatically */ | ||
| #define MY_TREE_WITH_DELETE 0x40000U | ||
| #define MY_TRY_LARGE_PAGES 0x80000U /* my_large_malloc(): attempt to use | ||
| large pages; the caller must only | ||
| pass this when my_use_large_pages | ||
| is set */ | ||
|
|
||
| #define MY_CHECK_ERROR 1U /* Params to my_end; Check open-close */ | ||
| #define MY_GIVE_INFO 2U /* Give time info about process*/ | ||
|
|
@@ -178,12 +182,20 @@ extern char *my_strdup(PSI_memory_key key, const char *from,myf MyFlags); | |
| extern char *my_strndup(PSI_memory_key key, const char *from, size_t length, myf MyFlags); | ||
| extern my_bool my_use_large_pages; | ||
|
|
||
| /** @return the myf flags to request large pages from my_large_malloc(), | ||
| my_large_virtual_alloc(), or the my_virtual_mem_*() functions, if | ||
| --large-pages is enabled */ | ||
| static inline myf my_large_pages_flag(void) | ||
| { | ||
| return MYF(my_use_large_pages ? MY_TRY_LARGE_PAGES : 0); | ||
| } | ||
|
Comment on lines
+188
to
+191
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be written without conditional code: compile_time_assert(1 << 19 == MY_TRY_LARGE_PAGES);
return (MYF(my_use_large_pages)) << 19;Not every target ISA supports conditional assignment. Conditional branches are best avoided. |
||
|
|
||
| int my_init_large_pages(void); | ||
| uchar *my_large_malloc(size_t *size, myf my_flags); | ||
| #ifdef _WIN32 | ||
| /* On Windows, use my_virtual_mem_reserve() and my_virtual_mem_commit(). */ | ||
| #else | ||
| char *my_large_virtual_alloc(size_t *size); | ||
| char *my_large_virtual_alloc(size_t *size, myf my_flags); | ||
| #endif | ||
| void my_large_free(void *ptr, size_t size); | ||
| void my_large_page_truncate(size_t *size); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
|
|
||
| --source include/have_innodb.inc | ||
|
|
||
| call mtr.add_suppression("\\[Warning\\] (mysqld|mariadbd): Couldn't allocate [0-9]+ bytes \\((Large/HugeTLB memory|MEMLOCK) page size [0-9]+\\).*"); | ||
| call mtr.add_suppression("\\[Warning\\] (mysqld|mariadbd): Couldn't allocate [0-9]+ bytes \\((Large/HugeTLB memory|MEM_LARGE_PAGES|MEMLOCK) page size [0-9]+\\).*"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is |
||
| call mtr.add_suppression("\\[ERROR\\]*Lock Pages in memory access rights required.*"); | ||
| create table t1 ( | ||
| a int not null auto_increment, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -295,13 +295,19 @@ MAP_ANON but MAP_ANONYMOUS is marked "for compatibility" */ | |
| uchar *my_large_malloc(size_t *size, myf my_flags) | ||
| { | ||
| uchar *ptr= NULL; | ||
| /* Only actually attempt large pages if the caller passed | ||
| MY_TRY_LARGE_PAGES (the caller is expected to only do so when | ||
| my_use_large_pages is set); otherwise always do a plain allocation of | ||
| the exact requested size, so *size is never rounded up to the large | ||
| page granularity. */ | ||
| const my_bool use_large_pages= (my_flags & MY_TRY_LARGE_PAGES) != 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also here we can avoid a conditional branch, as well as the normalisation: const myf use_large_pages= my_flags & MY_TRY_LARGE_PAGES; |
||
|
|
||
| #ifdef _WIN32 | ||
| DWORD alloc_type= MEM_COMMIT | MEM_RESERVE; | ||
| size_t orig_size= *size; | ||
| DBUG_ENTER("my_large_malloc"); | ||
|
|
||
| if (my_use_large_pages) | ||
| if (use_large_pages) | ||
| { | ||
| alloc_type|= MEM_LARGE_PAGES; | ||
| /* Align block size to my_large_page_size */ | ||
|
|
@@ -312,7 +318,7 @@ uchar *my_large_malloc(size_t *size, myf my_flags) | |
| { | ||
| if (my_flags & MY_WME) | ||
| { | ||
| if (my_use_large_pages) | ||
| if (use_large_pages) | ||
| { | ||
| my_printf_error(EE_OUTOFMEMORY, | ||
| "Couldn't allocate %zu bytes (MEM_LARGE_PAGES page " | ||
|
|
@@ -325,7 +331,7 @@ uchar *my_large_malloc(size_t *size, myf my_flags) | |
| my_error(EE_OUTOFMEMORY, MYF(ME_BELL+ME_ERROR_LOG), *size); | ||
| } | ||
| } | ||
| if (my_use_large_pages) | ||
| if (use_large_pages) | ||
| { | ||
| *size= orig_size; | ||
| ptr= VirtualAlloc(NULL, *size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); | ||
|
|
@@ -345,7 +351,7 @@ uchar *my_large_malloc(size_t *size, myf my_flags) | |
| while (1) | ||
| { | ||
| mapflag= MAP_PRIVATE | OS_MAP_ANON; | ||
| if (my_use_large_pages) | ||
| if (use_large_pages) | ||
| { | ||
| large_page_size= my_next_large_page_size(*size, &page_i); | ||
| /* this might be 0, in which case we do a standard mmap */ | ||
|
|
@@ -432,13 +438,21 @@ uchar *my_large_malloc(size_t *size, myf my_flags) | |
| Special large pages allocator, with possibility to commit to allocating | ||
| more memory later. | ||
| Every implementation returns a zero filled buffer here. | ||
| Initial protection of returned buffer is readwrite, if MY_TRY_LARGE_PAGES | ||
| is set in my_flags or on AIX(ask Marko why), but no access otherwise. | ||
| The caller is expected to call my_virtual_mem_commit() before using memory. | ||
| */ | ||
| char *my_large_virtual_alloc(size_t *size) | ||
| char *my_large_virtual_alloc(size_t *size, myf my_flags) | ||
| { | ||
| char *ptr; | ||
| int prot; | ||
| DBUG_ENTER("my_large_virtual_alloc"); | ||
|
|
||
| if (my_use_large_pages) | ||
| #ifdef _AIX | ||
| prot= PROT_READ | PROT_WRITE; | ||
| #else | ||
| prot= (my_flags & MY_TRY_LARGE_PAGES) ? PROT_READ|PROT_WRITE : PROT_NONE; | ||
| #endif | ||
| if (my_flags & MY_TRY_LARGE_PAGES) | ||
| { | ||
| size_t large_page_size; | ||
| int page_i= 0; | ||
|
Comment on lines
+448
to
458
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest the following change to reduce the amount of run-time conditions in this function (including the final #ifdef _AIX
int flags= MAP_PRIVATE | OS_MAP_ANON;
int prot= PROT_READ | PROT_WRITE;
#else
/* Illumos important to have MAP_NORESERVE otherwise reserves all swap
on innodb_buffer_pool_size_max overallocation.
Linux is controlled on sysctl vm.overcommit_memory. */
int flags= MAP_PRIVATE | OS_MAP_ANON | MAP_NORESERVE;
int prot= PROT_NONE;
#endif
if (my_flags & MY_TRY_LARGE_PAGES)
{
size_t large_page_size;
int page_i= 0;
#ifndef _AIX
prot= PROT_READ | PROT_WRITE;
flags= MAP_PRIVATE | OS_MAP_ANON;
#endif |
||
|
|
@@ -469,7 +483,7 @@ char *my_large_virtual_alloc(size_t *size) | |
| OS_MAP_ANON; | ||
|
|
||
| size_t aligned_size= MY_ALIGN(*size, (size_t) large_page_size); | ||
| ptr= mmap(NULL, aligned_size, PROT_READ | PROT_WRITE, mapflag, -1, 0); | ||
| ptr= mmap(NULL, aligned_size, prot, mapflag, -1, 0); | ||
| if (ptr == MAP_FAILED) | ||
| { | ||
| ptr= NULL; | ||
|
|
@@ -490,24 +504,20 @@ char *my_large_virtual_alloc(size_t *size) | |
| DBUG_RETURN(ptr); | ||
| } | ||
| } | ||
|
|
||
| my_use_large_pages= FALSE; | ||
|
Comment on lines
-493
to
-494
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a large page allocation failed, we used to disable further attempts to allocate large pages. This change is mentioned in the commit message, so it seems to be intentional. |
||
| } | ||
|
|
||
| # ifdef _AIX | ||
| /* On IBM AIX, my_virtual_mem_commit() relies on mprotect(2) rather than | ||
| a subsequent mmap(2) with MAP_FIXED. */ | ||
| ptr= mmap(NULL, *size, PROT_READ | PROT_WRITE, | ||
| MAP_PRIVATE | OS_MAP_ANON, -1, 0); | ||
| # else | ||
| /* | ||
| Illumos important to have MAP_NORESERVE otherwise reserves all swap. On | ||
| innodb_buffer_pool_size_max overallocation. | ||
| Linux is controlled on sysctl vm.overcommit_memory. | ||
|
|
||
| MAP_NORESERVE only applies to the PROT_NONE, reserve-only case: like the | ||
| large page loop above, prot == PROT_READ|PROT_WRITE (MY_TRY_LARGE_PAGES | ||
| fallback, or AIX) needs memory that is guaranteed to be usable right away. | ||
| */ | ||
| ptr= mmap(NULL, *size, PROT_NONE, MAP_PRIVATE | OS_MAP_ANON | MAP_NORESERVE, | ||
| ptr= mmap(NULL, *size, prot, | ||
| MAP_PRIVATE | OS_MAP_ANON | (prot == PROT_NONE ? MAP_NORESERVE : 0), | ||
| -1, 0); | ||
|
Comment on lines
-508
to
520
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ptr= mmap(NULL, *size, prot, flags, -1, 0); |
||
| # endif | ||
| if (ptr == MAP_FAILED) | ||
| ptr= NULL; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my_use_large_pagesis missing a comment that its value is not supposed to be initialized bymy_init_large_pages()and not thereafter.As far as I understand, with the removal of the assignment from
my_large_virtual_alloc(), the variablemy_use_large_pageswould always be 1 outside Microsoft Windows. Therefore, it should only be declared there. I would suggest the following:The initialisation would be as follows: