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 D5C09E77182 for ; Tue, 10 Dec 2024 16:23:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 602026B018C; Tue, 10 Dec 2024 11:23:37 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5B2716B018D; Tue, 10 Dec 2024 11:23:37 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 452986B018F; Tue, 10 Dec 2024 11:23:37 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 26DC76B018C for ; Tue, 10 Dec 2024 11:23:37 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 9ED7DC098C for ; Tue, 10 Dec 2024 16:23:36 +0000 (UTC) X-FDA: 82879569024.08.83DB65F Received: from mail-qt1-f180.google.com (mail-qt1-f180.google.com [209.85.160.180]) by imf28.hostedemail.com (Postfix) with ESMTP id 65E3DC000B for ; Tue, 10 Dec 2024 16:23:09 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=S9tVeOc4; spf=pass (imf28.hostedemail.com: domain of surenb@google.com designates 209.85.160.180 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733847800; 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=g+EjMlFhXDunShDRcDP0ShIng5wdn8oOvu3uP5ozI5o=; b=l6TnD85MyaVBfZ/ZOu1T8vSq6lZag2NmD0Pf4UD04LECCS/Yj/TmDFjD0hjI8NJnMOLLzs RZ6exxeFREcrSWX7YonHfn3AJzoRTRufJo2uLctozShn9yi6y/baw86e+ytLuiRpMW9kJ1 1clGy5RY/j/5fHbmJ574yDOru8rfxR0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733847800; a=rsa-sha256; cv=none; b=kVO310e5HhQCrPG4MW7tKHAvg7YB63vY5gxDXvKWDxgqqcYZlbMIGxlZKzs+g0FWRD1Ze3 H12O2rJQ+AwdYh3+zZP0IbBgmK16Zj4vckV/UHafugKZCiXOAYZrq5mRAxrLRGv6YjpLJT Kw+dOKs98UhzpMLAahRqTg0LRy7vKe0= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=S9tVeOc4; spf=pass (imf28.hostedemail.com: domain of surenb@google.com designates 209.85.160.180 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-qt1-f180.google.com with SMTP id d75a77b69052e-4675936f333so174341cf.0 for ; Tue, 10 Dec 2024 08:23:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1733847814; x=1734452614; 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=g+EjMlFhXDunShDRcDP0ShIng5wdn8oOvu3uP5ozI5o=; b=S9tVeOc4JKdHdkHppT8ftNjE/Jje+bBnc7zAtIPGUiX2jN0k5sLC9tymxxt9Ju9cR/ MXFYdhFb7tgHH8jKfOfqGeOUZ7Ah2lBBfu2F5ktzicLCdsEK2OQ7m0nO/WcQ9bjKUpM7 KM018aZKCneyofUuUUu8dPvNbbGEgtaccOPG0T9L2BKrcJFie5SDSfeJBZSBvOmD5chz 5gCjhMAI+RsZLn2Hp9z3ild1nHx24TFoeQZq9InF0vHevt1ULjU/V9pzcsSWEGlbo+Hg CFh6SVxjzETMjI31AlzSathodlr89UPspAqDONu6yt2sXXc7SdtqQIoJySRBBwJb7Uxf FCDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733847814; x=1734452614; 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=g+EjMlFhXDunShDRcDP0ShIng5wdn8oOvu3uP5ozI5o=; b=wpN0AvnuQlnl4LeyQqpmhvv5A+3mFlF2A66AkpRbtjlTisIgF2MWusc+pGHPobQ6qf Yvg4U3b1aUOpjFKAluCM0bqPkyTEhQCwjKxOZ5ZZq7i63c1R2yOFxmV95p+fs1mI/4kp PlmEYxSAIc1B2pFZLjRUU6M3A+F3jd1r6AfH3hsNP6RBCdZzJ3GYHusKjxmSISqj+Q5h 40p7XsNceCXX21OO+lXOAH2r8i9GS1kVhMdPcDVUy26dVmnxEwrlaJQ2LkfWeOYwCyg0 CS7H3TNCQAi65kBSAESYpEm28Sjki4/mb1rHmEHP4bSdsbPRS/ELOOlo40FSpV2rHCGg BcVA== X-Forwarded-Encrypted: i=1; AJvYcCU4MrMA+1yt4w/ry8LINJqIMNs2m+E4tlr/PRx/w1BDCPxPTtviG7zBXYFtKQH/Mq9iXCpAJWlrQA==@kvack.org X-Gm-Message-State: AOJu0YxDi/g6DszIdd4cNBxsfYEaDA9XSyGqjixJoiS5KWAPX2dXZ/xg 9iU6JJGTL6HtWhYm/S9LwW6dQJZqz/KdFQP9GRU5dG2HLMNwJiVMYRhUnFCRtzNLtT49eIVLz4c 1VjJPm167kVAkssMgsPw8IN4OvS62duPFb75J X-Gm-Gg: ASbGncvxneJkchT7ESPBiJyeOQMu5b9wKqnIwZQFyAMXRgtDpaFvnKqfE2cve6IwmQx NyYzfTtXrukXRycaca9SY8xqjjaVQIqt3fIrXo+DHelwTLSJL8nf+snxHImo8tjTRgw== X-Google-Smtp-Source: AGHT+IEOXgRpOlgsb1NGKHvnvi+3D5vXYnKehzcgJVuWcqYg9JXGMb6VpYKVRgpdRgZJFhzDzTjw+1bk5bdlflK5QZU= X-Received: by 2002:a05:622a:4c18:b0:466:97d6:b245 with SMTP id d75a77b69052e-46776274fdamr4711171cf.22.1733847813416; Tue, 10 Dec 2024 08:23:33 -0800 (PST) MIME-Version: 1.0 References: <20241206225204.4008261-1-surenb@google.com> <20241206225204.4008261-5-surenb@google.com> <9f000fbf-b5c4-41f5-8a4a-9c78b37c2ec5@suse.cz> In-Reply-To: <9f000fbf-b5c4-41f5-8a4a-9c78b37c2ec5@suse.cz> From: Suren Baghdasaryan Date: Tue, 10 Dec 2024 08:23:22 -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-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 65E3DC000B X-Stat-Signature: 8maj3gwpzqdosxf1wr64xd34nnqpdtuq X-Rspam-User: X-HE-Tag: 1733847789-374186 X-HE-Meta: U2FsdGVkX1/9lotLd9/FQDFpHtEyYJeYiqmOTGeEN/mfY249cxwoz+Ff6gePkhI0ZCiOdFIXYLNWzOPBd1NeTfaF2l9CCFy2K+iEdeL1kmbT4ZOjd58A/lZHoYEeKci80zdNvL3wLUZ3rQlze/4M3g2jED5GFzpH+5/UNxOuBFbQb5vwn4a3U3SsJb9CLb7K8tNwjUbjnHQU9oBB/ggN/Qmle+c9dh5SnEi40ki+WPa70A9v+dioKEuAuJWMmlzb2JY3hISuVWET+werOR4RIoH/mrhG5VklCt0oqG51Ii00S+I12jfm2OhQEA5L+YPKDMfdXHGuv3WLFaR0e2rdqcep8j385UOS2IslPjeIZJfMBOAkwwli8EKBvOCWy4lZZfWMX7TcLCtFXj79Aet/wjHClSbZAOrQ++98afEOW63/aLU6UrbVpAS18SnoiegrJhtiEzgvUKuhMc1XyealjP050f1KI7km4WaS8a9EBlItvrcskEaCj2NqBn3/ChGMpqqKMPRP5+qv4hJpgOhC4u+ZXpSD55rWcZtUgmvgT0r6UhrT5DrJJ30FpRMe9uM/Zp4pNc2RxEgN+qkjXHDUPvpCRvOGoU/6v6se7QjnAY54/i+FOOFddo5bEobYpwQtszIfCv3GMDqxlbzfggTnEiD7Pdrw7QYcPPTm6up10YE7ruDy8g/owO4scGkQwOcygkuSxXUkEIM5LcvSL1UdhXbsyqXjsu9nCPjuzm+5kENxM8JFGPQvu4eV0+G6Kl/SQl6V3YRbptXSz6PF+gLUbAE9VEjLtJ0xV5H8skvi+OSirYJBeSb/GfjNcWXwSlTg1g8+9K2CEQWLmqSNEWs6kNBR5MCrsVaABk/Z/j1xIWQ4+aWKlXIXhGVk48c5HrLsHXjkhjcwinGtuDsZb3v4hi1j26uD4TENIMdvdt34fgjiKdzV8RzmV6MiBTaTMO8LfV1t6E4p3bG71XHSd/d 2cqJoc10 bzzgmylRiKxzQZLxCPTlEiR1usrK5pwG8dJ76XOoJbpWAfV0+q4Toc/fnLJVi6oBuk73EWp0i2Ue6i8dAOKExktKfuvXFyhRP8+hzEQUyTUNgYna2qAgAmFDfrSAosjQQkIF8BoxjB/zzGPAxXA1SLYLJ2b+DNpHVCMVDWmwiToh6MguzDexbbI37TK9QUobHiBvUSolx0115RVwP6J2MX3ZS88XDL6GjMkNW6E0GZE1OtJu6fnsk3nOCTpfGBDnsdigiExx+voLwWJg= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, 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 4:06=E2=80=AFAM Vlastimil Babka wr= ote: > > On 12/6/24 23:52, Suren Baghdasaryan wrote: > > To enable SLAB_TYPESAFE_BY_RCU for vma cache we need to ensure that > > object reuse before RCU grace period is over will be detected inside > > lock_vma_under_rcu(). > > lock_vma_under_rcu() enters RCU read section, finds the vma at the > > given address, locks the vma and checks if it got detached or remapped > > to cover a different address range. These last checks are there > > to ensure that the vma was not modified after we found it but before > > 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 before > > 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 not > > 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() returning > > a false locked result but that's not critical if it's rare because > > 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 after > > vm_start/vm_end/vm_mm were set and lock_vma_under_rcu() should check > > vma->detached before checking vm_start/vm_end/vm_mm. This is required > > because attaching vma happens without vma write-lock, as opposed to > > vma detaching, which requires vma write-lock. This patch adds memory > > 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 number > > of call_rcu() calls. > > > > Signed-off-by: Suren Baghdasaryan > > --- > > include/linux/mm.h | 36 +++++-- > > include/linux/mm_types.h | 10 +- > > include/linux/slab.h | 6 -- > > kernel/fork.c | 157 +++++++++++++++++++++++++------ > > mm/memory.c | 15 ++- > > mm/vma.c | 2 +- > > tools/testing/vma/vma_internal.h | 7 +- > > 7 files changed, 179 insertions(+), 54 deletions(-) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 2bf38c1e9cca..3568bcbc7c81 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -257,7 +257,7 @@ struct vm_area_struct *vm_area_alloc(struct mm_stru= ct *); > > struct vm_area_struct *vm_area_dup(struct vm_area_struct *); > > void vm_area_free(struct vm_area_struct *); > > /* Use only if VMA has no other users */ > > -void __vm_area_free(struct vm_area_struct *vma); > > +void vm_area_free_unreachable(struct vm_area_struct *vma); > > > > #ifndef CONFIG_MMU > > extern struct rb_root nommu_region_tree; > > @@ -706,8 +706,10 @@ static inline void vma_lock_init(struct vm_area_st= ruct *vma) > > * Try to read-lock a vma. The function is allowed to occasionally yie= ld false > > * locked result to avoid performance overhead, in which case we fall = back to > > * using mmap_lock. The function should never yield false unlocked res= ult. > > + * False locked result is possible if mm_lock_seq overflows or if vma = gets > > + * reused and attached to a different mm before we lock it. > > */ > > -static inline bool vma_start_read(struct vm_area_struct *vma) > > +static inline bool vma_start_read(struct mm_struct *mm, struct vm_area= _struct *vma) > > { > > /* > > * Check before locking. A race might cause false locked result. > > @@ -716,7 +718,7 @@ static inline bool vma_start_read(struct vm_area_st= ruct *vma) > > * we don't rely on for anything - the mm_lock_seq read against w= hich we > > * need ordering is below. > > */ > > - if (READ_ONCE(vma->vm_lock_seq) =3D=3D READ_ONCE(vma->vm_mm->mm_l= ock_seq.sequence)) > > + if (READ_ONCE(vma->vm_lock_seq) =3D=3D READ_ONCE(mm->mm_lock_seq.= sequence)) > > return false; > > > > if (unlikely(down_read_trylock(&vma->vm_lock.lock) =3D=3D 0)) > > @@ -733,7 +735,7 @@ static inline bool vma_start_read(struct vm_area_st= ruct *vma) > > * after it has been unlocked. > > * This pairs with RELEASE semantics in vma_end_write_all(). > > */ > > - if (unlikely(vma->vm_lock_seq =3D=3D raw_read_seqcount(&vma->vm_m= m->mm_lock_seq))) { > > + if (unlikely(vma->vm_lock_seq =3D=3D raw_read_seqcount(&mm->mm_lo= ck_seq))) { > > up_read(&vma->vm_lock.lock); > > return false; > > } > > This could have been perhaps another preparatory patch to make this one s= maller? > > > > > +static void vm_area_ctor(void *data) > > +{ > > + struct vm_area_struct *vma =3D (struct vm_area_struct *)data; > > + > > +#ifdef CONFIG_PER_VMA_LOCK > > + /* vma is not locked, can't use vma_mark_detached() */ > > + vma->detached =3D true; > > +#endif > > + INIT_LIST_HEAD(&vma->anon_vma_chain); > > + vma_lock_init(vma); > > +} > > + > > +#ifdef CONFIG_PER_VMA_LOCK > > + > > +static void vma_clear(struct vm_area_struct *vma, struct mm_struct *mm= ) > > +{ > > + vma->vm_mm =3D mm; > > + vma->vm_ops =3D &vma_dummy_vm_ops; > > + vma->vm_start =3D 0; > > + vma->vm_end =3D 0; > > + vma->anon_vma =3D NULL; > > + vma->vm_pgoff =3D 0; > > + vma->vm_file =3D NULL; > > + vma->vm_private_data =3D NULL; > > + vm_flags_init(vma, 0); > > + memset(&vma->vm_page_prot, 0, sizeof(vma->vm_page_prot)); > > + memset(&vma->shared, 0, sizeof(vma->shared)); > > + memset(&vma->vm_userfaultfd_ctx, 0, sizeof(vma->vm_userfaultfd_ct= x)); > > + vma_numab_state_init(vma); > > +#ifdef CONFIG_ANON_VMA_NAME > > + vma->anon_name =3D NULL; > > +#endif > > +#ifdef CONFIG_SWAP > > + memset(&vma->swap_readahead_info, 0, sizeof(vma->swap_readahead_i= nfo)); > > +#endif > > +#ifndef CONFIG_MMU > > + vma->vm_region =3D NULL; > > +#endif > > +#ifdef CONFIG_NUMA > > + vma->vm_policy =3D NULL; > > +#endif > > This isn't the ideal pattern I think, now that we have a ctor. Ideally th= e > ctor would do all this (except setting the vm_mm), and then we need to ma= ke > sure it's also done when freeing the vma, to make sure the freed object i= s > in the same state as a new object after the constructor. > > On freeing, things like numab_state and anon_name could be NULL'd (by the > respective destructors) only when they are non-NULL and thus freeing the > objects pointed to. vm_policy and vm_file could perhaps be handled same w= ay > after some refactoring (see remove_vma()), vma_dummy_vm_ops are possibly > already reset by vma_close(), etc. Ok, let me look some more into it and see if I can improve and simplify the initialization/freeing logic. Thanks! > > > +} > > + > > +static void vma_copy(const struct vm_area_struct *src, struct vm_area_= struct *dest) > > +{ > > + dest->vm_mm =3D src->vm_mm; > > + dest->vm_ops =3D src->vm_ops; > > + dest->vm_start =3D src->vm_start; > > + dest->vm_end =3D src->vm_end; > > + dest->anon_vma =3D src->anon_vma; > > + dest->vm_pgoff =3D src->vm_pgoff; > > + dest->vm_file =3D src->vm_file; > > + dest->vm_private_data =3D src->vm_private_data; > > + vm_flags_init(dest, src->vm_flags); > > + memcpy(&dest->vm_page_prot, &src->vm_page_prot, > > + sizeof(dest->vm_page_prot)); > > + memcpy(&dest->shared, &src->shared, sizeof(dest->shared)); > > + memcpy(&dest->vm_userfaultfd_ctx, &src->vm_userfaultfd_ctx, > > + sizeof(dest->vm_userfaultfd_ctx)); > > +#ifdef CONFIG_ANON_VMA_NAME > > + dest->anon_name =3D src->anon_name; > > +#endif > > +#ifdef CONFIG_SWAP > > + memcpy(&dest->swap_readahead_info, &src->swap_readahead_info, > > + sizeof(dest->swap_readahead_info)); > > +#endif > > +#ifndef CONFIG_MMU > > + dest->vm_region =3D src->vm_region; > > +#endif > > +#ifdef CONFIG_NUMA > > + dest->vm_policy =3D src->vm_policy; > > +#endif > > +} > > + > > +#else /* CONFIG_PER_VMA_LOCK */