From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id E90806EC55; Wed, 28 Jul 2021 15:31:17 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E90806EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1627475478; bh=c9pTFKYVPRmB1CJy7UZSa7nBzdZacgehpFoQ2L1B8Bo=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=IlLRXm8b38lfed/8U8xBhS/YqhE2qnmNbx71FTdhv6hAYtuBr2m32KXt1OWL5lmXr W3rEcWrySuqYUzZxxYFesGXVeeDLHftc8o1ccc05BWG6jzqr0EcyZYGZrMOJVHW+Bv Tt7O4Pv0m/1JCxo2bS90KzwNCUdOOJT9HWDAN3II= Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 88CDA6EC57 for ; Wed, 28 Jul 2021 15:30:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 88CDA6EC57 Received: by smtp16.mail.ru with esmtpa (envelope-from ) id 1m8ii2-0007dO-L3; Wed, 28 Jul 2021 15:30:51 +0300 Date: Wed, 28 Jul 2021 15:29:38 +0300 To: Igor Munkin Message-ID: References: <20210727152323.GS27855@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210727152323.GS27855@tarantool.org> X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD941C43E597735A9C3038391AAE5FBFA7682283CBB09BF492D182A05F538085040981DF28605F63F1729D4355B03AD3D478B692AA1A8336B1FC918125C18FBE54F X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E0D4B9A53B20B455EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637B23888C9749F3CAC8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D80164F68E4A3B00CB8F495CC1EF3DE368117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18C26CFBAC0749D213D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6A45692FFBBD75A6A089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A53A9F4E81AB27E02ECC87176EF9913451519F7F117D3924A7D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA754263BA4E959D734C410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3463DBE2ADA183F62F6E17D0982EA57C425C8B36B3C6353B3DE55C0FEE73D383D85B55DA3A0F5BD3CB1D7E09C32AA3244C82CD00FCF36ED62256F7EE2047BE857F6C24832127668422FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojiF1u9eOpfTT5JYaWxfAdVA== X-Mailru-Sender: 3B9A0136629DC91206CBC582EFEF4CB41D4B88576C9853F609A76E1EBA7E6D111DCA76B1FCB5F293F2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A89437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 2/2] Linux/ARM64: Make mremap() non-moving due to VA space woes. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On 27.07.21, Igor Munkin wrote: > Sergey, > > Thanks for the patch! LGTM, except a few nits below. > > On 06.07.21, Sergey Kaplun wrote: > > From: Mike Pall > > > > This reduces overall performance on ARM64, but we have no choice. > > Linux kernel default userspace VA is 48 bit, but we'd need 47 bit. > > mremap() ignores address hints due to a kernel API issue. The mapping > > may move to an undesired address which will cause an assert or crash. > > > > Reported by Raymond W. Ko. > > > > (cherry picked from commit 67dbec82f4f05a416a78a560a726553beaa7a223) > > > > 47-bit VA space is required by LuaJIT for keeping a GC object pointer in > > TValue. When need to reallocate to huge sized block `mrepmap()` on arm64 > > may move out VA space from the 47-bit range. `mremap()` accepts the > > I'd rather reword the previous sentence the following way: > | In case of huge blobs that are mapped directly, `mremap()` may move > | the chunk out of 47-bit range of VA space on ARM64. Fixed. > > > fifth argument (new address hint) only with MREMAP_FIXED flag. In that > > case it unmaps any other mapping to specified address. > > > > To avoid this behaviour this patch restricts `mremap()` to relocate > > the mapping to a new virtual address by reset MREMAP_MAYMOVE flag > > I'm confused a bit with "reset" word: MREMAP_MAYMOVE is simply changed > to 0 (i.e. CALL_MREMAP_NOMOVE). Moreover, it's better to stay in LuaJIT > terms instead of Linux ones. Fixed. The new commit message is the following: =================================================================== Linux/ARM64: Make mremap() non-moving due to VA space woes. This reduces overall performance on ARM64, but we have no choice. Linux kernel default userspace VA is 48 bit, but we'd need 47 bit. mremap() ignores address hints due to a kernel API issue. The mapping may move to an undesired address which will cause an assert or crash. Reported by Raymond W. Ko. (cherry picked from commit 67dbec82f4f05a416a78a560a726553beaa7a223) 47-bit VA space is required by LuaJIT for keeping a GC object pointer in TValue. In case of huge blobs that are mapped directly, `mremap()` may move the chunk out of 47-bit range of VA space on ARM64. `mremap()` accepts the fifth argument (new address hint) only with MREMAP_FIXED flag. In that case it unmaps any other mapping to specified address. To avoid this behaviour this patch restricts `mremap()` to relocate the mapping to a new virtual address by set CALL_MREMAP_NOMOVE flag instead of CALL_MREMAP_MAYMOVE for arm64 architecture. Sergey Kaplun: * added the description and the test for the problem Needed for tarantool/tarantool#6154 =================================================================== > > > for arm64 architecture. > > > > Sergey Kaplun: > > * added the description and the test for the problem > > > > Needed for tarantool/tarantool#6154 > > --- > > src/lj_alloc.c | 2 +- > > .../lj-671-arm64-assert-after-mremap.test.lua | 24 +++++++++++++++++++ > > 2 files changed, 25 insertions(+), 1 deletion(-) > > create mode 100644 test/tarantool-tests/lj-671-arm64-assert-after-mremap.test.lua > > > > > > > diff --git a/test/tarantool-tests/lj-671-arm64-assert-after-mremap.test.lua b/test/tarantool-tests/lj-671-arm64-assert-after-mremap.test.lua > > new file mode 100644 > > index 00000000..0be60a2d > > --- /dev/null > > +++ b/test/tarantool-tests/lj-671-arm64-assert-after-mremap.test.lua > > @@ -0,0 +1,24 @@ > > +local tap = require('tap') > > + > > +-- Test file to demonstrate assertion after `mremap()` on arm64. > > +-- See also, https://github.com/LuaJIT/LuaJIT/issues/671. > > + > > +local test = tap.test('lj-671-arm64-assert-after-mremap') > > +test:plan(1) > > + > > +-- `mremap()` is used on Linux for remap directly mapped big > > Typo: s/for/to/. > > > +-- (>=DEFAULT_MMAP_THRESHOLD) memory chunks. > > +-- The simplest way to test memory move is to allocate the huge > > +-- memory chunk for string buffer directly and reallocate it > > +-- after. > > +-- To allocate buffer exactly to threshold limit for direct chunk > > +-- mapping use `string.rep()` with length equals threshold. > > +-- Then concatenate result string (with length of > > +-- DEFAULT_MMAP_THRESHOLD) with the other one to reallocate > > +-- and remap string buffer. > > Just polished two sections above: > | -- To allocate a memory buffer with the size up to the threshold > | -- for direct mapping `string.rep()` is used with the length that > | -- equals to DEFAULT_MMAP_THRESHOLD. > | -- Then concatenate the directly mapped result string with the > | -- other one to trigger buffer reallocation and its remapping. Fixed. See the iterative patch below. Branch is force pushed. =================================================================== diff --git a/test/tarantool-tests/lj-671-arm64-assert-after-mremap.test.lua b/test/tarantool-tests/lj-671-arm64-assert-after-mremap.test.lua index 0be60a2d..0558cbe3 100644 --- a/test/tarantool-tests/lj-671-arm64-assert-after-mremap.test.lua +++ b/test/tarantool-tests/lj-671-arm64-assert-after-mremap.test.lua @@ -6,16 +6,16 @@ local tap = require('tap') local test = tap.test('lj-671-arm64-assert-after-mremap') test:plan(1) --- `mremap()` is used on Linux for remap directly mapped big +-- `mremap()` is used on Linux to remap directly mapped big -- (>=DEFAULT_MMAP_THRESHOLD) memory chunks. -- The simplest way to test memory move is to allocate the huge -- memory chunk for string buffer directly and reallocate it -- after. --- To allocate buffer exactly to threshold limit for direct chunk --- mapping use `string.rep()` with length equals threshold. --- Then concatenate result string (with length of --- DEFAULT_MMAP_THRESHOLD) with the other one to reallocate --- and remap string buffer. +-- To allocate a memory buffer with the size up to the threshold +-- for direct mapping `string.rep()` is used with the length that +-- equals to DEFAULT_MMAP_THRESHOLD. +-- Then concatenate the directly mapped result string with the +-- other one to trigger buffer reallocation and its remapping. local DEFAULT_MMAP_THRESHOLD = 128 * 1024 local s = string.rep('x', DEFAULT_MMAP_THRESHOLD)..'x' =================================================================== > > > + > > +local DEFAULT_MMAP_THRESHOLD = 128 * 1024 > > +local s = string.rep('x', DEFAULT_MMAP_THRESHOLD)..'x' > > +test:ok(s) > > + > > +os.exit(test:check() and 0 or 1) > > -- > > 2.31.0 > > > > -- > Best regards, > IM -- Best regards, Sergey Kaplun