From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 40D52E7717F for ; Tue, 10 Dec 2024 18:53:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C67936B00ED; Tue, 10 Dec 2024 13:53:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BF03F6B00F1; Tue, 10 Dec 2024 13:53:45 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A69968D000B; Tue, 10 Dec 2024 13:53:45 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 8356D6B00ED for ; Tue, 10 Dec 2024 13:53:45 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 338FCA014E for ; Tue, 10 Dec 2024 18:53:45 +0000 (UTC) X-FDA: 82879947990.04.FFC09AC Received: from mail-qt1-f170.google.com (mail-qt1-f170.google.com [209.85.160.170]) by imf06.hostedemail.com (Postfix) with ESMTP id C48F418000B for ; Tue, 10 Dec 2024 18:53:27 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="ND/etBIG"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf06.hostedemail.com: domain of surenb@google.com designates 209.85.160.170 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733856813; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=yTxg1Tv9B28JSqo18UFTvQ4Yt+t7p72F7E79Abgqc2Y=; b=3ERl+Ayz1h7hK75xlW/Iec9QEZa6Z6DcuiIYa84w/BD+Q4JNzTR1RW1MkCzzdGqKJmhjbp 1IbPOJpGV7JsV6iDvZFbeRPZkb3vf1abUXnQ1UfGd1/S47/xMR0jxg2Ybct6gA07t4fifF I6s+f1gz1wrq/wsCz9lJvGZzHUmIK3Q= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733856813; a=rsa-sha256; cv=none; b=Z5RwzzrOTI008YoGVHgg1kBRT3H/7CSUXdtZ/ynSzHo03UhtVAbrbdMFwyxm4qklRHls7L RAzowK/jgnDXX6OCC741e4F8cgbrE1QQLXziS4XkVlBlAJ4ksrIRgVSto2FHcKD9iKxCfP RCUVZMIt+dMnzjyNeDuN8LPrvqxuanU= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="ND/etBIG"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf06.hostedemail.com: domain of surenb@google.com designates 209.85.160.170 as permitted sender) smtp.mailfrom=surenb@google.com Received: by mail-qt1-f170.google.com with SMTP id d75a77b69052e-4674a47b7e4so30361cf.1 for ; Tue, 10 Dec 2024 10:53:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1733856822; x=1734461622; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=yTxg1Tv9B28JSqo18UFTvQ4Yt+t7p72F7E79Abgqc2Y=; b=ND/etBIGmi35O2xNS9YRPrHogydjc6tF3l+OTd+iGv5hz/dgxaC+N0XQNJXxUpUTVE mclBO/Fc2beYA60HGvQQIYqn0SIYCJcCHD93yRWTUe3hfMzKp/qoK4/WCXbLyyelsyCw mhwlm611Kji4kiFMJeGSjnU+WHc17oKOywc5Q2BrFWQsTUr/G1zXJysbTmau7Q3UBt4j Q/mWjB7+UwH6OP6sTlhAVIatzDn+L+jSf7TXvpkurIMrGWDDycSRikG2HzIEhuP7y1z/ aORXdLM5bxx7x0vqsNlRwDwIrr6vos5iBYzHbUIwOF91yNtIx9UdC5AKOCPiXwhNqwJu z0HQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733856822; x=1734461622; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yTxg1Tv9B28JSqo18UFTvQ4Yt+t7p72F7E79Abgqc2Y=; b=jOAfZTrcr0jF4/mW/RviDSmS5H4t90OTUMct14aUQLQ/JdxBCxVrYIUPWv1hyqobTB PgxLW2uLZ7vCbmRUTavNlYv+Sy9Hx/P8xshYz3v/UZSGPDFM8ZrNo1sce/k6Xut0PgGT SWX28j9OyObQ5BiGtHmWniXntYx6/EIY71bXX7xxpM9t+QhV2PH4h8rjfa3i/LeF7BU8 zRw6W8l5DdUodjVaq6wvYW9nrAOG50j/jZDA5rOYMSrxbDwTZyLkH3lJHlD3MiQZ7hCp Til6i4Fuf195o+dUuxhgLtkUKf2DIig2FoM0j2fxyBKNwtvuLIS19VZERt/W9lpK6Wog aIFg== X-Forwarded-Encrypted: i=1; AJvYcCVMN55lnh8yrOLLzdLyzT0hM0NuKpylVPmpbMHJDFzWiVgbYDiT0zxyBA+J0aGR5FSYQzwhPN6UfA==@kvack.org X-Gm-Message-State: AOJu0YxjjmM5/WOzM6WjiaybTdu0arKMS9/Umy3dF/gYJvBGyhTEB4DR LLZim/7Bl9wMee1U9TZIHDurG1jvIziV9Bjbp1uLjeJm29inIx40Knm78CudPd0GuTfwWnYcMQ5 51PSuyyon9Pg5PcdACdIpgDhabiV3A1zZf09H X-Gm-Gg: ASbGncujUhBtX8U3GRhrjlin93WOcnVbBgGr/AfzGdlKdtTzoFxQdb0aCRyeAjNIdRT Vc1Fzg6mlFnXfoaW3e9WtBrNRmAtN8JIQAIawEFuFPYmzxlD7Kah0+OCsveKga0StoA== X-Google-Smtp-Source: AGHT+IGGTh7Wd7c9V0Hn51P0NeOMcT2HJORM6+yXkwY1q0Jtracb0QUR4je6F0teZ8jcQ/XBsI0mY88C2MXRP4HxjV8= X-Received: by 2002:a05:622a:418c:b0:467:82de:d949 with SMTP id d75a77b69052e-46788c9c18dmr204051cf.12.1733856822128; Tue, 10 Dec 2024 10:53:42 -0800 (PST) MIME-Version: 1.0 References: <20241206225204.4008261-1-surenb@google.com> <20241206225204.4008261-5-surenb@google.com> <6b29b2a5-c244-4930-a5a0-1a24a04e7e35@suse.cz> <643beb6c-4226-46ca-b7e9-292467479aea@suse.cz> <5036d089-0774-4863-88c5-eaaea1265ac7@suse.cz> In-Reply-To: <5036d089-0774-4863-88c5-eaaea1265ac7@suse.cz> From: Suren Baghdasaryan Date: Tue, 10 Dec 2024 10:53:30 -0800 Message-ID: Subject: Re: [PATCH v5 4/6] mm: make vma cache SLAB_TYPESAFE_BY_RCU To: Vlastimil Babka Cc: akpm@linux-foundation.org, willy@infradead.org, liam.howlett@oracle.com, lorenzo.stoakes@oracle.com, mhocko@suse.com, hannes@cmpxchg.org, mjguzik@gmail.com, oliver.sang@intel.com, mgorman@techsingularity.net, david@redhat.com, peterx@redhat.com, oleg@redhat.com, dave@stgolabs.net, paulmck@kernel.org, brauner@kernel.org, dhowells@redhat.com, hdanton@sina.com, hughd@google.com, minchan@google.com, jannh@google.com, shakeel.butt@linux.dev, souravpanda@google.com, pasha.tatashin@soleen.com, corbet@lwn.net, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: ijxizejpcof3z8zyaza1pko11fy4qakm X-Rspamd-Queue-Id: C48F418000B X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1733856807-838554 X-HE-Meta: U2FsdGVkX19wI+GApUy4KJc1mQWO2ZZ3MVtzJc9I6bng8E3kuGLO/QijYew/Ef4VJe8lSOt5kxJzgeP6pbJNW+KzxFc8lWrtUw5ocVCayOV4ascB2DIMbyZTq3P7ETgcpl8L6cB9Hbp7AdidFs2VQH6ktTlL6iZKy6Tnk4iMotW6MCSSX+jXpvFq8rvIg5HJojQO8nEFqVZvw/rW37pGcs3hHktVCry2uVkeP15G8mWYMJ/vJ9BHtPRE9KsqGGnbMchEceI9CpxyfrAwTWFeciFHNePtu3l9+OFm8q9FOuV/YEc1LUBbAunx1wFqaDBLUR9N2pSO2iZJOz+S2A0Iqpugf5aFOr022ob7gKXSaEb+npOdTSXl2D0Sltb3yR2FdP1LrbDgaFIrMDjmrDFQU2SKd+W62HY8qDErkdR8egw5gJQGsisQJUjE1RxC9U3lDjudcv9yGqFcjqFCe4sswIiAhXHULRZKYXtNAFPUUGhyq96Oy/Fx/QuiP6rmkmAhyJhnShrVN2SiqT0xZS6wnzKpXYwSR1trEt3pNQprQVS/WZJqNYOGaRodqFhKGcDGcp864p+LzirHPyaz1AyHbbrfgtFVZGzibfOcs8Fl+hrGn8RFoR9vKZRpo+r1AwYpMCPOF4RPfV/EtQSdztXGFaEhmEiMkMZNfMhzSAqrVqoJb6apFm+J4+U+c+8DpqDx1IQQlFa/lhapebna1HxaS9kCzU4MAXu062PX+Fi8kWtSj/gDpTz9JcRyNIVnzr9Zj4qKhumYYHQsvI69NJJgm53OvpzJGvflTIIy4boKu5AW5AA1Y8NOIeJf1RwivF4Q/65yM6w+QMTDridh6mMY0QzithNnnEKOl0shkycawY0KiWCpuumuICC/QWMJqiFym6mm26DpedxUHgMpRxepK+tGlLb9THXBqBuCxRSq3nrd1FjJZIlN1zQZySTOpaiPk6/vqJRQOVT9y9J5+I3 8hMYMhcW KrI9LTjKck6328Vp/OiXvKvvHOUH0rpRj8gjudldWIVLjUzTe9/tueEdSscL2tZq8j7fdGmqRksXA3HIBrm8ZBAzeOkWfUJe+27svxB927KzoWSUVSxgTqchZ6MCD46Yfu6XDuvXBybxR+W2Gxb2d1UI4cbWZvXznSeKKIUNk5sFYoSGrEhlBUv7jmoVHQnSb6C0jHx7W/XqKOPicXP0S4ROau20S2NGuFRldD/eJdZ0lpWG11Dc36nZRJ0wGqxjFCLklMB/94GRxrDw= X-Bogosity: Ham, tests=bogofilter, spamicity=0.010397, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Dec 10, 2024 at 9:25=E2=80=AFAM Vlastimil Babka wr= ote: > > On 12/10/24 18:16, Suren Baghdasaryan wrote: > > On Tue, Dec 10, 2024 at 8:32=E2=80=AFAM Vlastimil Babka wrote: > >> > >> On 12/10/24 17:20, Suren Baghdasaryan wrote: > >> > On Tue, Dec 10, 2024 at 6:21=E2=80=AFAM Vlastimil Babka wrote: > >> >> > >> >> On 12/6/24 23:52, Suren Baghdasaryan wrote: > >> >> > To enable SLAB_TYPESAFE_BY_RCU for vma cache we need to ensure th= at > >> >> > object reuse before RCU grace period is over will be detected ins= ide > >> >> > lock_vma_under_rcu(). > >> >> > lock_vma_under_rcu() enters RCU read section, finds the vma at th= e > >> >> > given address, locks the vma and checks if it got detached or rem= apped > >> >> > to cover a different address range. These last checks are there > >> >> > to ensure that the vma was not modified after we found it but bef= ore > >> >> > locking it. > >> >> > vma reuse introduces several new possibilities: > >> >> > 1. vma can be reused after it was found but before it is locked; > >> >> > 2. vma can be reused and reinitialized (including changing its vm= _mm) > >> >> > while being locked in vma_start_read(); > >> >> > 3. vma can be reused and reinitialized after it was found but bef= ore > >> >> > it is locked, then attached at a new address or to a new mm while > >> >> > read-locked; > >> >> > For case #1 current checks will help detecting cases when: > >> >> > - vma was reused but not yet added into the tree (detached check) > >> >> > - vma was reused at a different address range (address check); > >> >> > We are missing the check for vm_mm to ensure the reused vma was n= ot > >> >> > attached to a different mm. This patch adds the missing check. > >> >> > For case #2, we pass mm to vma_start_read() to prevent access to > >> >> > unstable vma->vm_mm. This might lead to vma_start_read() returnin= g > >> >> > a false locked result but that's not critical if it's rare becaus= e > >> >> > it will only lead to a retry under mmap_lock. > >> >> > For case #3, we ensure the order in which vma->detached flag and > >> >> > vm_start/vm_end/vm_mm are set and checked. vma gets attached afte= r > >> >> > vm_start/vm_end/vm_mm were set and lock_vma_under_rcu() should ch= eck > >> >> > vma->detached before checking vm_start/vm_end/vm_mm. This is requ= ired > >> >> > because attaching vma happens without vma write-lock, as opposed = to > >> >> > vma detaching, which requires vma write-lock. This patch adds mem= ory > >> >> > barriers inside is_vma_detached() and vma_mark_attached() needed = to > >> >> > order reads and writes to vma->detached vs vm_start/vm_end/vm_mm. > >> >> > After these provisions, SLAB_TYPESAFE_BY_RCU is added to vm_area_= cachep. > >> >> > This will facilitate vm_area_struct reuse and will minimize the n= umber > >> >> > of call_rcu() calls. > >> >> > > >> >> > Signed-off-by: Suren Baghdasaryan > >> >> > >> >> I'm wondering about the vma freeing path. Consider vma_complete(): > >> >> > >> >> vma_mark_detached(vp->remove); > >> >> vma->detached =3D true; - plain write > >> >> vm_area_free(vp->remove); > >> >> vma->vm_lock_seq =3D UINT_MAX; - plain write > >> >> kmem_cache_free(vm_area_cachep) > >> >> ... > >> >> potential reallocation > >> >> > >> >> against: > >> >> > >> >> lock_vma_under_rcu() > >> >> - mas_walk finds a stale vma due to race > >> >> vma_start_read() > >> >> if (READ_ONCE(vma->vm_lock_seq) =3D=3D READ_ONCE(mm->mm_lock_seq.= sequence)) > >> >> - can be false, the vma was not being locked on the freeing side? > >> >> down_read_trylock(&vma->vm_lock.lock) - suceeds, wasn't locked > >> >> this is acquire, but was there any release? > >> > > >> > Yes, there was a release. I think what you missed is that > >> > vma_mark_detached() that is called from vma_complete() requires VMA = to > >> > be write-locked (see vma_assert_write_locked() in > >> > vma_mark_detached()). The rule is that a VMA can be attached without > >> > write-locking but only a write-locked VMA can be detached. So, after > >> > >> OK but write unlocking means the mm's seqcount is bumped and becomes > >> non-equal with vma's vma->vm_lock_seq, right? > >> > >> Yet in the example above we happily set it to UINT_MAX and thus effect= ively > >> false unlock it for vma_start_read()? > >> > >> And this is all done before the vma_complete() side would actually rea= ch > >> mmap_write_unlock(), AFAICS. > > > > Ah, you are right. With the possibility of reuse, even a freed VMA > > should be kept write-locked until it is unlocked by > > mmap_write_unlock(). I think the fix for this is simply to not reset > > vma->vm_lock_seq inside vm_area_free(). I'll also need to add a > > But even if we don't reset vm_lock_seq to UINT_MAX, then whover reallocat= ed > it can proceed and end up doing a vma_start_write() and rewrite it there > anyway, no? Ugh, yes. It looks like we will need a write memory barrier in vma_mark_detached() to make it immediately visible. Case 1: lock_vma_under_rcu() vma_complete() vma_mark_detached(vp->remove); vma->detached =3D true; smp_wmb(); vm_area_free(vp->remove); vma->vm_lock_seq =3D UINT_MAX; vma_start_read() is_vma_detached() <<- abort kmem_cache_free(vm_area_cachep); mmap_write_unlock() Case 2: lock_vma_under_rcu() vma_complete() vma_mark_detached(vp->remove); vma->detached =3D true; smp_wmb(); vm_area_free(vp->remove); vma->vm_lock_seq =3D UINT_MAX; vma_start_read() kmem_cache_free(vm_area_cachep); mmap_write_unlock() // changes mm->mm_lock_seq but does not matter // reader holds vma->vm_lock, so new writers have to wa= it ... vm_area_alloc(); // sets all vma attributes vma_mark_attached(); smp_wmb(); // if is_vma_detached() called // before this point, we will // abort like in Case 1 vma->detached =3D true; is_vma_detached() // check vm_mm, vm_start, vm_end // if all checks pass, this is a // new attached vma and it's // read-locked (can't be modifie= d) Did I miss any other race? > > > comment for vm_lock_seq explaining these requirements. > > Do you agree that such a change would resolve the issue? > > > >> > >> > vma_mark_detached() and before down_read_trylock(&vma->vm_lock.lock) > >> > in vma_start_read() the VMA write-lock should have been released by > >> > mmap_write_unlock() and therefore vma->detached=3Dfalse should be > >> > visible to the reader when it executed lock_vma_under_rcu(). > >> > > >> >> is_vma_detached() - false negative as the write above didn't prop= agate > >> >> here yet; a read barrier but where is the write barrier? > >> >> checks for vma->vm_mm, vm_start, vm_end - nobody reset them yet s= o false > >> >> positive, or they got reset on reallocation but writes didn't p= ropagate > >> >> > >> >> Am I missing something that would prevent lock_vma_under_rcu() fals= ely > >> >> succeeding here? > >> >> > >> >