From 6adffcd96873f57a2b4fd84e51a0829ff8c79f1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 8 Jan 2025 12:59:28 +0200 Subject: [PATCH 1/2] MDEV-35785 innodb_log_file_mmap is not defined on 32-bit systems HAVE_INNODB_MMAP: Remove, and unconditionally enable this code. log_mmap(): On 32-bit systems, ensure that the size fits in 32 bits. log_t::resize_start(), log_t::resize_abort(): Only handle memory-mapping if HAVE_PMEM is defined. The generic memory-mapped interface is only for reading the log in recovery. Writable memory mappings are only for persistent memory, that is, Linux file systems with mount -o dax. Reviewed by: Debarun Banerjee --- extra/mariabackup/xtrabackup.cc | 9 +--- .../innodb_redo_log_overwrite.combinations | 4 ++ .../suite/sys_vars/r/sysvars_innodb.result | 13 +++++- .../suite/sys_vars/t/sysvars_innodb.test | 2 +- storage/innobase/handler/ha_innodb.cc | 4 -- storage/innobase/include/log0log.h | 11 ----- storage/innobase/include/log0recv.h | 7 +-- storage/innobase/include/univ.i | 3 -- storage/innobase/log/log0log.cc | 44 +++++++------------ storage/innobase/log/log0recv.cc | 4 -- storage/innobase/mtr/mtr0mtr.cc | 2 +- storage/innobase/srv/srv0start.cc | 4 -- 12 files changed, 36 insertions(+), 71 deletions(-) create mode 100644 mysql-test/suite/mariabackup/innodb_redo_log_overwrite.combinations diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc index 5367ea5148e6a..8385114f39b6f 100644 --- a/extra/mariabackup/xtrabackup.cc +++ b/extra/mariabackup/xtrabackup.cc @@ -1331,9 +1331,7 @@ enum options_xtrabackup OPT_INNODB_BUFFER_POOL_FILENAME, OPT_INNODB_LOCK_WAIT_TIMEOUT, OPT_INNODB_LOG_BUFFER_SIZE, -#ifdef HAVE_INNODB_MMAP OPT_INNODB_LOG_FILE_MMAP, -#endif #if defined __linux__ || defined _WIN32 OPT_INNODB_LOG_FILE_BUFFERING, #endif @@ -1895,13 +1893,11 @@ struct my_option xb_server_options[] = (G_PTR*) &log_sys.buf_size, (G_PTR*) &log_sys.buf_size, 0, GET_UINT, REQUIRED_ARG, 2U << 20, 2U << 20, log_sys.buf_size_max, 0, 4096, 0}, -#ifdef HAVE_INNODB_MMAP {"innodb_log_file_mmap", OPT_INNODB_LOG_FILE_SIZE, "Whether ib_logfile0 should be memory-mapped", (G_PTR*) &log_sys.log_mmap, (G_PTR*) &log_sys.log_mmap, 0, GET_BOOL, NO_ARG, log_sys.log_mmap_default, 0, 0, 0, 0, 0}, -#endif #if defined __linux__ || defined _WIN32 {"innodb_log_file_buffering", OPT_INNODB_LOG_FILE_BUFFERING, "Whether the file system cache for ib_logfile0 is enabled during --backup", @@ -3370,7 +3366,6 @@ static my_bool xtrabackup_copy_datafile(ds_ctxt *ds_data, return(FALSE); } -#ifdef HAVE_INNODB_MMAP static int xtrabackup_copy_mmap_snippet(ds_file_t *ds, const byte *start, const byte *end) { @@ -3470,7 +3465,6 @@ static bool xtrabackup_copy_mmap_logfile() msg(">> log scanned up to (" LSN_PF ")", recv_sys.lsn); return false; } -#endif /** Copy redo log until the current end of the log is reached @return whether the operation failed */ @@ -3482,10 +3476,9 @@ static bool xtrabackup_copy_logfile() ut_a(dst_log_file); ut_ad(recv_sys.is_initialised()); -#ifdef HAVE_INNODB_MMAP if (log_sys.is_mmap()) return xtrabackup_copy_mmap_logfile(); -#endif + const size_t sequence_offset{log_sys.is_encrypted() ? 8U + 5U : 5U}; const size_t block_size_1{log_sys.write_size - 1}; diff --git a/mysql-test/suite/mariabackup/innodb_redo_log_overwrite.combinations b/mysql-test/suite/mariabackup/innodb_redo_log_overwrite.combinations new file mode 100644 index 0000000000000..5221a9747be4c --- /dev/null +++ b/mysql-test/suite/mariabackup/innodb_redo_log_overwrite.combinations @@ -0,0 +1,4 @@ +[pread] +--innodb-log-file-mmap=OFF +[mmap] +--innodb-log-file-mmap=ON diff --git a/mysql-test/suite/sys_vars/r/sysvars_innodb.result b/mysql-test/suite/sys_vars/r/sysvars_innodb.result index d24dfd5c37472..44b24f4384ba3 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_innodb.result +++ b/mysql-test/suite/sys_vars/r/sysvars_innodb.result @@ -4,7 +4,6 @@ variable_name not in ( 'innodb_numa_interleave', # only available WITH_NUMA 'innodb_evict_tables_on_commit_debug', # one may want to override this 'innodb_use_native_aio', # default value depends on OS -'innodb_log_file_mmap', # only available on 64-bit 'innodb_log_file_buffering', # only available on Linux and Windows 'innodb_buffer_pool_load_pages_abort') # debug build only, and is only for testing order by variable_name; @@ -1028,6 +1027,18 @@ NUMERIC_BLOCK_SIZE NULL ENUM_VALUE_LIST OFF,ON READ_ONLY NO COMMAND_LINE_ARGUMENT OPTIONAL +VARIABLE_NAME INNODB_LOG_FILE_MMAP +SESSION_VALUE NULL +DEFAULT_VALUE ON +VARIABLE_SCOPE GLOBAL +VARIABLE_TYPE BOOLEAN +VARIABLE_COMMENT Whether ib_logfile0 should initially be memory-mapped +NUMERIC_MIN_VALUE NULL +NUMERIC_MAX_VALUE NULL +NUMERIC_BLOCK_SIZE NULL +ENUM_VALUE_LIST OFF,ON +READ_ONLY YES +COMMAND_LINE_ARGUMENT OPTIONAL VARIABLE_NAME INNODB_LOG_FILE_SIZE SESSION_VALUE NULL DEFAULT_VALUE 100663296 diff --git a/mysql-test/suite/sys_vars/t/sysvars_innodb.test b/mysql-test/suite/sys_vars/t/sysvars_innodb.test index 86f5ffddf1c5a..308936cee1019 100644 --- a/mysql-test/suite/sys_vars/t/sysvars_innodb.test +++ b/mysql-test/suite/sys_vars/t/sysvars_innodb.test @@ -5,13 +5,13 @@ --vertical_results --replace_regex /^\/\S+/PATH/ /\.\//PATH/ +--replace_result 'resides in persistent memory or ' '' select VARIABLE_NAME, SESSION_VALUE, DEFAULT_VALUE, VARIABLE_SCOPE, VARIABLE_TYPE, VARIABLE_COMMENT, NUMERIC_MIN_VALUE, NUMERIC_MAX_VALUE, NUMERIC_BLOCK_SIZE, ENUM_VALUE_LIST, READ_ONLY, COMMAND_LINE_ARGUMENT from information_schema.system_variables where variable_name like 'innodb%' and variable_name not in ( 'innodb_numa_interleave', # only available WITH_NUMA 'innodb_evict_tables_on_commit_debug', # one may want to override this 'innodb_use_native_aio', # default value depends on OS - 'innodb_log_file_mmap', # only available on 64-bit 'innodb_log_file_buffering', # only available on Linux and Windows 'innodb_buffer_pool_load_pages_abort') # debug build only, and is only for testing order by variable_name; diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 83049ca4fad5f..d5a37f2efbcff 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -19478,7 +19478,6 @@ static MYSQL_SYSVAR_UINT(log_buffer_size, log_sys.buf_size, "Redo log buffer size in bytes.", NULL, NULL, 16U << 20, 2U << 20, log_sys.buf_size_max, 4096); -#ifdef HAVE_INNODB_MMAP static constexpr const char *innodb_log_file_mmap_description= "Whether ib_logfile0" # ifdef HAVE_PMEM @@ -19489,7 +19488,6 @@ static MYSQL_SYSVAR_BOOL(log_file_mmap, log_sys.log_mmap, PLUGIN_VAR_OPCMDARG | PLUGIN_VAR_READONLY, innodb_log_file_mmap_description, nullptr, nullptr, log_sys.log_mmap_default); -#endif #if defined __linux__ || defined _WIN32 static MYSQL_SYSVAR_BOOL(log_file_buffering, log_sys.log_buffered, @@ -19985,9 +19983,7 @@ static struct st_mysql_sys_var* innobase_system_variables[]= { MYSQL_SYSVAR(deadlock_report), MYSQL_SYSVAR(page_size), MYSQL_SYSVAR(log_buffer_size), -#ifdef HAVE_INNODB_MMAP MYSQL_SYSVAR(log_file_mmap), -#endif #if defined __linux__ || defined _WIN32 MYSQL_SYSVAR(log_file_buffering), #endif diff --git a/storage/innobase/include/log0log.h b/storage/innobase/include/log0log.h index cfde0472e78c2..7dca1d0e040ab 100644 --- a/storage/innobase/include/log0log.h +++ b/storage/innobase/include/log0log.h @@ -282,7 +282,6 @@ struct log_t uint write_size; /** format of the redo log: e.g., FORMAT_10_8 */ uint32_t format; -#ifdef HAVE_INNODB_MMAP /** whether the memory-mapped interface is enabled for the log */ my_bool log_mmap; /** the default value of log_mmap */ @@ -294,7 +293,6 @@ struct log_t # else /* an unnecessary read-ahead of a large ib_logfile0 is a risk */ # endif false; -#endif #if defined __linux__ || defined _WIN32 /** whether file system caching is enabled for the log */ my_bool log_buffered; @@ -347,11 +345,7 @@ struct log_t void set_buf_free(size_t f) noexcept { ut_ad(f < buf_free_LOCK); buf_free.store(f, std::memory_order_relaxed); } -#ifdef HAVE_INNODB_MMAP bool is_mmap() const noexcept { return !flush_buf; } -#else - static constexpr bool is_mmap() { return false; } -#endif /** @return whether a handle to the log is open; is_mmap() && !is_opened() holds for PMEM */ @@ -412,14 +406,9 @@ struct log_t @return whether the memory allocation succeeded */ bool attach(log_file_t file, os_offset_t size); -#ifdef HAVE_INNODB_MMAP /** Disable memory-mapped access (update log_mmap) */ void clear_mmap(); void close_file(bool really_close= true); -#else - static void clear_mmap() {} - void close_file(); -#endif #if defined __linux__ || defined _WIN32 /** Try to enable or disable file system caching (update log_buffered) */ void set_buffered(bool buffered); diff --git a/storage/innobase/include/log0recv.h b/storage/innobase/include/log0recv.h index d9dd094505930..1a4459001a6a9 100644 --- a/storage/innobase/include/log0recv.h +++ b/storage/innobase/include/log0recv.h @@ -430,12 +430,7 @@ struct recv_sys_t @tparam storing whether to store the records @param if_exists storing=YES: whether to check if the tablespace exists */ template - static parse_mtr_result parse_mmap(bool if_exists) noexcept -#ifdef HAVE_INNODB_MMAP - ; -#else - { return parse_mtr(if_exists); } -#endif + static parse_mtr_result parse_mmap(bool if_exists) noexcept; /** Erase log records for a page. */ void erase(map::iterator p); diff --git a/storage/innobase/include/univ.i b/storage/innobase/include/univ.i index 9c3f8fe1f657b..c9b0bfca1e09a 100644 --- a/storage/innobase/include/univ.i +++ b/storage/innobase/include/univ.i @@ -169,9 +169,6 @@ using the call command. */ #define UNIV_INLINE static inline #define UNIV_WORD_SIZE SIZEOF_SIZE_T -#if SIZEOF_SIZE_T == 8 -# define HAVE_INNODB_MMAP -#endif /** The following alignment is used in memory allocations in memory heap management to ensure correct alignment for doubles etc. */ diff --git a/storage/innobase/log/log0log.cc b/storage/innobase/log/log0log.cc index 10928c66b2061..c7de872cb648a 100644 --- a/storage/innobase/log/log0log.cc +++ b/storage/innobase/log/log0log.cc @@ -187,7 +187,6 @@ void log_file_t::write(os_offset_t offset, span buf) noexcept abort(); } -#ifdef HAVE_INNODB_MMAP # ifdef HAVE_PMEM # include "cache.h" # endif @@ -203,6 +202,10 @@ static void *log_mmap(os_file_t file, # endif os_offset_t size) { +#if SIZEOF_SIZE_T < 8 + if (size != os_offset_t(size_t(size))) + return MAP_FAILED; +#endif if (my_system_page_size > 4096) return MAP_FAILED; # ifndef HAVE_PMEM @@ -300,20 +303,17 @@ static void *log_mmap(os_file_t file, # endif return ptr; } -#endif #if defined __linux__ || defined _WIN32 /** Display a message about opening the log */ ATTRIBUTE_COLD static void log_file_message() { sql_print_information("InnoDB: %s (block size=%u bytes)", -# ifdef HAVE_INNODB_MMAP log_sys.log_mmap ? (log_sys.log_buffered ? "Memory-mapped log" : "Memory-mapped unbuffered log") : -# endif log_sys.log_buffered ? "Buffered log writes" : "File system buffers for log disabled", @@ -332,7 +332,6 @@ bool log_t::attach(log_file_t file, os_offset_t size) ut_ad(!buf); ut_ad(!flush_buf); ut_ad(!writer); -#ifdef HAVE_INNODB_MMAP if (size) { # ifdef HAVE_PMEM @@ -363,7 +362,6 @@ bool log_t::attach(log_file_t file, os_offset_t size) } } log_mmap= false; -#endif buf= static_cast(ut_malloc_dontdump(buf_size, PSI_INSTRUMENT_ME)); if (!buf) { @@ -400,9 +398,7 @@ bool log_t::attach(log_file_t file, os_offset_t size) writer_update(); memset_aligned<512>(checkpoint_buf, 0, write_size); -#ifdef HAVE_INNODB_MMAP func_exit: -#endif log_file_message(); return true; } @@ -477,25 +473,19 @@ ATTRIBUTE_COLD static void log_close_failed(dberr_t err) ib::fatal() << "closing ib_logfile0 failed: " << err; } -#ifdef HAVE_INNODB_MMAP void log_t::close_file(bool really_close) -#else -void log_t::close_file() -#endif { -#ifdef HAVE_INNODB_MMAP if (is_mmap()) { ut_ad(!checkpoint_buf); ut_ad(!flush_buf); if (buf) { - my_munmap(buf, file_size); + my_munmap(buf, size_t(file_size)); buf= nullptr; } } else -#endif { ut_ad(!buf == !flush_buf); ut_ad(!buf == !checkpoint_buf); @@ -512,9 +502,7 @@ void log_t::close_file() writer= nullptr; -#ifdef HAVE_INNODB_MMAP if (really_close) -#endif if (is_opened()) if (const dberr_t err= log.close()) log_close_failed(err); @@ -621,14 +609,10 @@ log_t::resize_start_status log_t::resize_start(os_offset_t size) noexcept void *ptr= nullptr, *ptr2= nullptr; success= os_file_set_size(path.c_str(), resize_log.m_file, size); if (!success); -#ifdef HAVE_INNODB_MMAP +#ifdef HAVE_PMEM else if (is_mmap()) { - ptr= ::log_mmap(resize_log.m_file, -#ifdef HAVE_PMEM - is_pmem, -#endif - size); + ptr= ::log_mmap(resize_log.m_file, is_pmem, size); if (ptr == MAP_FAILED) goto alloc_fail; @@ -636,6 +620,7 @@ log_t::resize_start_status log_t::resize_start(os_offset_t size) noexcept #endif else { + ut_ad(!is_mmap()); ptr= ut_malloc_dontdump(buf_size, PSI_INSTRUMENT_ME); if (ptr) { @@ -705,21 +690,26 @@ void log_t::resize_abort() noexcept if (resize_in_progress() > 1) { - if (!is_mmap()) +#ifdef HAVE_PMEM + const bool is_mmap{this->is_mmap()}; +#else + constexpr bool is_mmap{false}; +#endif + if (!is_mmap) { ut_free_dodump(resize_buf, buf_size); ut_free_dodump(resize_flush_buf, buf_size); resize_flush_buf= nullptr; } -#ifdef HAVE_INNODB_MMAP else { ut_ad(!resize_log.is_opened()); ut_ad(!resize_flush_buf); +#ifdef HAVE_PMEM if (resize_buf) my_munmap(resize_buf, resize_target); +#endif /* HAVE_PMEM */ } -#endif if (resize_log.is_opened()) resize_log.close(); resize_buf= nullptr; @@ -1224,7 +1214,6 @@ ATTRIBUTE_COLD void log_write_and_flush_prepare() group_commit_lock::ACQUIRED); } -#ifdef HAVE_INNODB_MMAP void log_t::clear_mmap() { if (!is_mmap() || @@ -1258,7 +1247,6 @@ void log_t::clear_mmap() } log_resize_release(); } -#endif /** Durably write the log up to log_sys.get_lsn(). */ ATTRIBUTE_COLD void log_write_and_flush() diff --git a/storage/innobase/log/log0recv.cc b/storage/innobase/log/log0recv.cc index d0f68996001ef..a39a0a5605be1 100644 --- a/storage/innobase/log/log0recv.cc +++ b/storage/innobase/log/log0recv.cc @@ -2285,7 +2285,6 @@ struct recv_buf } }; -#ifdef HAVE_INNODB_MMAP /** Ring buffer wrapper for log_sys.buf[]; recv_sys.len == log_sys.file_size */ struct recv_ring : public recv_buf { @@ -2417,7 +2416,6 @@ struct recv_ring : public recv_buf return log_decrypt_buf(iv, tmp + s, b, static_cast(len)); } }; -#endif template void recv_sys_t::rewind(source &l, source &begin) noexcept @@ -3119,7 +3117,6 @@ template recv_sys_t::parse_mtr_result recv_sys_t::parse_mtr(bool) noexcept; -#ifdef HAVE_INNODB_MMAP template recv_sys_t::parse_mtr_result recv_sys_t::parse_mmap(bool if_exists) noexcept { @@ -3140,7 +3137,6 @@ recv_sys_t::parse_mtr_result recv_sys_t::parse_mmap(bool if_exists) noexcept template recv_sys_t::parse_mtr_result recv_sys_t::parse_mmap(bool) noexcept; -#endif /** Apply the hashed log records to the page, if the page lsn is less than the lsn of a log record. diff --git a/storage/innobase/mtr/mtr0mtr.cc b/storage/innobase/mtr/mtr0mtr.cc index d2ec3b1d8dcf3..c01661e0a42f3 100644 --- a/storage/innobase/mtr/mtr0mtr.cc +++ b/storage/innobase/mtr/mtr0mtr.cc @@ -1212,7 +1212,7 @@ inline void log_t::resize_write(lsn_t lsn, const byte *end, size_t len, end-= len; size_t s; -#ifdef HAVE_INNODB_MMAP +#ifdef HAVE_PMEM if (!resize_flush_buf) { ut_ad(is_mmap()); diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc index f0500b1a7443f..e32ed6e4710f5 100644 --- a/storage/innobase/srv/srv0start.cc +++ b/storage/innobase/srv/srv0start.cc @@ -1879,15 +1879,11 @@ dberr_t srv_start(bool create_new_db) if (srv_print_verbose_log) { sql_print_information("InnoDB: " "log sequence number " LSN_PF -#ifdef HAVE_INNODB_MMAP "%s" -#endif "; transaction id " TRX_ID_FMT, recv_sys.lsn, -#ifdef HAVE_INNODB_MMAP log_sys.is_mmap() ? " (memory-mapped)" : "", -#endif trx_sys.get_max_trx_id()); } From 9057410ec72186fe9f9d0e6a73b9754e29c00afe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 8 Jan 2025 13:20:51 +0200 Subject: [PATCH 2/2] MDEV-33447 fixup: pmem_persist() on RISC-V and LoongArch Let us enable pmem_persist() on RISC-V and LoongArch, because those are available in the Debian CI. In commit 3f9f5ca48e6be55613926964314f550c8ca8542d these were initially disabled by default. --- storage/innobase/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/innobase/CMakeLists.txt b/storage/innobase/CMakeLists.txt index 2c0384575e8a2..7dde90286f5fe 100644 --- a/storage/innobase/CMakeLists.txt +++ b/storage/innobase/CMakeLists.txt @@ -49,7 +49,7 @@ IF(UNIX) LINK_LIBRARIES(numa) ENDIF() IF(CMAKE_SIZEOF_VOID_P EQUAL 8) - IF(CMAKE_SYSTEM_PROCESSOR MATCHES "(aarch|AARCH|p(ower)?pc|x86_|amd)64") + IF(CMAKE_SYSTEM_PROCESSOR MATCHES "(aarch|AARCH|p(ower)?pc|x86_|amd|loongarch|riscv)64") OPTION(WITH_INNODB_PMEM "Support memory-mapped InnoDB redo log" ON) ELSE() # Disable by default on ISA that are not covered by our CI OPTION(WITH_INNODB_PMEM "Support memory-mapped InnoDB redo log" OFF)