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 8A852D3C52B for ; Thu, 17 Oct 2024 18:55:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 069126B007B; Thu, 17 Oct 2024 14:55:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F2F146B0082; Thu, 17 Oct 2024 14:55:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DD01A6B0083; Thu, 17 Oct 2024 14:55:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id BBC816B007B for ; Thu, 17 Oct 2024 14:55:51 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 09FDBAB05D for ; Thu, 17 Oct 2024 18:55:29 +0000 (UTC) X-FDA: 82683998208.10.93E70D6 Received: from mail-pf1-f175.google.com (mail-pf1-f175.google.com [209.85.210.175]) by imf26.hostedemail.com (Postfix) with ESMTP id D043314001B for ; Thu, 17 Oct 2024 18:55:41 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=PJt1bg6q; spf=pass (imf26.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.210.175 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729191301; 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=jCbiAckxU+1LrXu7YNeHArlba++n1QiAkjUcgAqY7TM=; b=rjI6wT3uggnX7Ar1I8QdHPupOL8mOeSbSgepdeVPf3DW4bSKy9qpW4+yqRGwPJsWi9t01x K16vn9LzH/2Ln6J7URIXh1SAWTJSCVEDCORfx+4V1LHq4sG/asGZcokd15O/tklrEY5/Zr KEFcLqJkDsw+Rsf+FMN24TzCQFE+f6o= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=PJt1bg6q; spf=pass (imf26.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.210.175 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729191301; a=rsa-sha256; cv=none; b=W99GTLwWHKaNYoYs1ikkREVTvCethovxsoXdfVzEwG4g8K+62R9lv6ck8KmlspaDtHwzEX tOrFRada0cuAfNQ4zpTArs9/CkoxNWZnmYpz3qZYyS83PZTzoS8FwCbmRIaXA1tB9EJzdi Vr9opvTccE/5tUJnVFrN5Vz7/+b8FJs= Received: by mail-pf1-f175.google.com with SMTP id d2e1a72fcca58-71e4fa3ea7cso1088231b3a.0 for ; Thu, 17 Oct 2024 11:55:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729191348; x=1729796148; 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=jCbiAckxU+1LrXu7YNeHArlba++n1QiAkjUcgAqY7TM=; b=PJt1bg6q8SHC+RGnl/DWMdWO8FPVxJn6M6J4XzpSRrPJ/8J6p3t1LBuZZJleAM1Td0 Mr/9aXerF2wgCg5BWxbqtnbLN/hucjyd3+xy5D0B7ATRQv79zFHVb7PxVLzwMfoJnmmS crRW/LHlvKNbW7tjwNqhaF38qOP/+cLyBNTEusEmrCZFYotQqLJ0QCv9XZozlYiEHEbx qmoEgK5Lc5Un5ARUrO7ZHwkcGq0i70TO62cnfyzRCuHUpMGnEmOXIPESPqmZyr4oKQse Ar8qSNYpQdND9eIhJEv5JGKxYUNqRVhHs5tMquEeq1N9wz0IEhsdKJ7pz3tRU4y5+iI1 jeng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729191348; x=1729796148; 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=jCbiAckxU+1LrXu7YNeHArlba++n1QiAkjUcgAqY7TM=; b=PPvLNN+IXIbFpCZMX6UgKpidaLApgmvnKw1pezRoJvF6Ebu0t/HD4erOKhb0nWfjjG YXHS7lwvn3fPWCoRZGFr19Ul2zAWtvMWBp1XuzWnThJXAWDiixRnMaRr0k+eFkf0FFas 9E/4K4il8/Z3JWemW6vQ3gIvv8kGuDRhyYM3O7xgIcf/xs/X4xWjhjQNKflBZ9tO2Wnf a8QKuF5BUNtgPJfjtLarC8onBuQ0E1R3UBhcMBwOCmzvd+o/lOs0V7GWRTdf5/0rPY5X fdaG+3QjgvLmt9BLfT5FIHFD4XsfcbFi6fctUxtD89vDWMtemhZaf6guIfLug+OrZ7uN B0Kg== X-Forwarded-Encrypted: i=1; AJvYcCXxHlVRYQqaYB4DAHY0FVy4C5dbWerPzg1m+qZh4RJqEwn/v3J1eMqXfVo5SjAmr2lv+wWycJJK9g==@kvack.org X-Gm-Message-State: AOJu0Yxhlnj+lx5XCIHads83NPqFqD/Hjp80hrCcfVqr9wYmNAoolp21 7Bh0kHTlFziQbS53YhaMZx4Mb7FUIgCQ+5tgyvTat8JkS0mSMaaW2lQdNyu6Ds7bu2KHU3ZmwCk AtUBRVfk935rO3mLJcozvuGywWO8= X-Google-Smtp-Source: AGHT+IGCdpUfDxA6zAdi5OINLOxawquAP3ymgOYackxEhc10GVlIEJPFiB03uSJkD6QlvJghlEnn0pw9AdKdAXMwGgo= X-Received: by 2002:a05:6a00:1250:b0:71e:fb4:6c98 with SMTP id d2e1a72fcca58-71e4c1cfc05mr35446834b3a.23.1729191347929; Thu, 17 Oct 2024 11:55:47 -0700 (PDT) MIME-Version: 1.0 References: <20241010205644.3831427-1-andrii@kernel.org> <20241010205644.3831427-3-andrii@kernel.org> <55hskn2iz5ixsl6wvupnhx7hkzcvx2u4muswvzi4wuqplmu2uo@rj72ypyeksjy> In-Reply-To: From: Andrii Nakryiko Date: Thu, 17 Oct 2024 11:55:35 -0700 Message-ID: Subject: Re: [PATCH v3 tip/perf/core 2/4] mm: switch to 64-bit mm_lock_seq/vm_lock_seq on 64-bit architectures To: Suren Baghdasaryan Cc: Shakeel Butt , Andrii Nakryiko , linux-trace-kernel@vger.kernel.org, linux-mm@kvack.org, peterz@infradead.org, oleg@redhat.com, rostedt@goodmis.org, mhiramat@kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org, jolsa@kernel.org, paulmck@kernel.org, willy@infradead.org, akpm@linux-foundation.org, mjguzik@gmail.com, brauner@kernel.org, jannh@google.com, mhocko@kernel.org, vbabka@suse.cz, hannes@cmpxchg.org, Liam.Howlett@oracle.com, lorenzo.stoakes@oracle.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: jgxwsypnkasm9mzbx7hmjef1atccztay X-Rspamd-Queue-Id: D043314001B X-Rspamd-Server: rspam11 X-HE-Tag: 1729191341-797421 X-HE-Meta: U2FsdGVkX1/JBw3daDAXaXgvhDHJT0clmtCWCC5r9Ok08ZO67pgg56sI44eDBMFzTkDd9X5hpApsLLAxwqHLbxZKdm31jjqFtkx2qt72wxPIKb36h5FViNeHm1+spCRlRbk6dSECZT6dR62/GT134iX4hQIdaloVz8mNSEExyMMbIQFk1yRo3giawj/yYKdv7tabIC13/rUH1rmo1u2Pcq/vOHqjMcFY1xJZA9k2w65wqX2xItO21EHGKcq6GS5pq8UPAycVbqd5AluccywiLB/a+l9zWPf2t1244l4cfTCCSinIP20wjkXKpNj94rHfMLsMdy3lPR9jQUxL4wLAGGflIKA651mv9XxU6QXbmscRoK34vuwn8XT/RqEIOs30ZJUWGCH2JIqm3tkQWyxe6DvK4kt093An/OIEEyf9h0Ry2TYooBq0G5Svml8AKdfm1s5daML7rVFbQpyOIwhG99VoPrGrYBDUpY+WvGB431bGxXI3UcLM8fKs2EfUfJXaRC967WXfN/0fQ9irjHkFXmlyJJua6bOKJVglj5nmAn3B0R9l3D4Ui5/ayAhtR5ul7BbhDrDbK57rvyVNsjmmUqViltloSCCuTCUwo2f7f+snSjoT/VplePiYE6WAXzWy+RQnqqmLSxMsh2hfOO7/2l5WkmOTclKRrgqG98j5gG+Rd6xzIIrlvP5YYPbXyaOP3puvhG2JrRfXPj93HUgy63zrdKmbjQBleTkSlezJkZpnIWwK6wJuoCNRqY6FyVpPQf68GfE8vW5nvI1rbpw//WXC+YwZcFXO+117JKrbs3NIN7jRwNECUG1JQnvMjwhcqUl8TD02FewhcOBE0T++OLHoiwUpJparJg//H2fhAKLq1S/qx2wr/iBaq55T0Sm/1FdgODEJZgRAL60wk1jmoTazanzOhcbipm+qdsLK/wiN9pIqiNvDJ5U6tUMWuC45EPY/Gc4ewWl0oLKBov6 eTNJou0Z x4Bxb+SCA8gNlX37/r91FhTxZvPrYavlI8oJs1fCtsN1vHr7HFpxhu0SibDUM3fZBDYXFqjmqapa+vnES5YCHbS9H2xwXwulNuwsxRe3qaxCV8W/KJz3Q1wq6Cr6Rjg0pZZ45Ttbady+A6ISFdKlezd73zZL/ATu182jODaDkcBSaTRPj3YW8JH7itCG4j2bwkv8bwa6aUBjFdYhRHSvwzB8K3B/ek4N1uVX+WvH3n5Cc/arHzylpyAnnpWPSxOTfEeuRK7+FXqIP5AcPqasxsg6SkSJr+TsCIIm1/G/jzeMt1usWsjyEi5CpApv7+50RiXy2TcQJc9uIx1fkXrHrWGiLezfqmUD5V0O4KxgUfnUI/PCcFVKn60mSgVrytq8Dhs7gBsLc9/r08a1fW1xshxIJnYr42ARnG750mT6uzl5xp4B2Hk8XeV/ltq45n+Ud88MJ 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 Wed, Oct 16, 2024 at 7:02=E2=80=AFPM Suren Baghdasaryan wrote: > > On Sun, Oct 13, 2024 at 12:56=E2=80=AFAM Shakeel Butt wrote: > > > > On Thu, Oct 10, 2024 at 01:56:42PM GMT, Andrii Nakryiko wrote: > > > To increase mm->mm_lock_seq robustness, switch it from int to long, s= o > > > that it's a 64-bit counter on 64-bit systems and we can stop worrying > > > about it wrapping around in just ~4 billion iterations. Same goes for > > > VMA's matching vm_lock_seq, which is derived from mm_lock_seq. > > vm_lock_seq does not need to be long but for consistency I guess that How come, we literally assign vm_lock_seq from mm_lock_seq and do direct comparisons. They have to be exactly the same type, no? > makes sense. While at it, can you please change these seq counters to > be unsigned? There is `vma->vm_lock_seq =3D -1;` in kernel/fork.c, should it be switched to ULONG_MAX then? In general, unless this is critical for correctness, I'd very much like stuff like this to be done in the mm tree afterwards, but it seems trivial enough, so if you insist I'll do it. > Also, did you check with pahole if the vm_area_struct layout change > pushes some members into a difference cacheline or creates new gaps? > Just did. We had 3 byte hole after `bool detached;`, it now grew to 7 bytes (so +4) and then vm_lock_seq itself is now 8 bytes (so +4), which now does push rb and rb_subtree_last into *THE SAME* cache line (which sounds like an improvement to me). vm_lock_seq and vm_lock stay in the same cache line. vm_pgoff and vm_file are now in the same cache line, and given they are probably always accessed together, seems like a good accidental change as well. See below pahole outputs before and after. That singular detached bool looks like a complete waste, tbh. Maybe it would be better to roll it into vm_flags and save 8 bytes? (not that I want to do those mm changes in this patch set, of course...). vm_area_struct is otherwise nicely tightly packed. tl;dr, seems fine, and detached would be best to get rid of, if possible (but that's a completely separate thing) BEFORE =3D=3D=3D=3D=3D=3D struct vm_area_struct { union { struct { long unsigned int vm_start; /* 0 8 */ long unsigned int vm_end; /* 8 8 */ }; /* 0 16 */ struct callback_head vm_rcu; /* 0 16 */ } __attribute__((__aligned__(8))); /* 0 16 */ struct mm_struct * vm_mm; /* 16 8 */ pgprot_t vm_page_prot; /* 24 8 */ union { const vm_flags_t vm_flags; /* 32 8 */ vm_flags_t __vm_flags; /* 32 8 */ }; /* 32 8 */ bool detached; /* 40 1 */ /* XXX 3 bytes hole, try to pack */ int vm_lock_seq; /* 44 4 */ struct vma_lock * vm_lock; /* 48 8 */ struct { struct rb_node rb; /* 56 24 */ /* --- cacheline 1 boundary (64 bytes) was 16 bytes ago ---= */ long unsigned int rb_subtree_last; /* 80 8 */ } /* 56 32 */ struct list_head anon_vma_chain; /* 88 16 */ struct anon_vma * anon_vma; /* 104 8 */ const struct vm_operations_struct * vm_ops; /* 112 8 */ long unsigned int vm_pgoff; /* 120 8 */ /* --- cacheline 2 boundary (128 bytes) --- */ struct file * vm_file; /* 128 8 */ void * vm_private_data; /* 136 8 */ atomic_long_t swap_readahead_info; /* 144 8 */ struct mempolicy * vm_policy; /* 152 8 */ struct vma_numab_state * numab_state; /* 160 8 */ struct vm_userfaultfd_ctx vm_userfaultfd_ctx; /* 168 8 */ /* size: 176, cachelines: 3, members: 18 */ /* sum members: 173, holes: 1, sum holes: 3 */ /* forced alignments: 2 */ /* last cacheline: 48 bytes */ } __attribute__((__aligned__(8))); AFTER =3D=3D=3D=3D=3D struct vm_area_struct { union { struct { long unsigned int vm_start; /* 0 8 */ long unsigned int vm_end; /* 8 8 */ }; /* 0 16 */ struct callback_head vm_rcu; /* 0 16 */ } __attribute__((__aligned__(8))); /* 0 16 */ struct mm_struct * vm_mm; /* 16 8 */ pgprot_t vm_page_prot; /* 24 8 */ union { const vm_flags_t vm_flags; /* 32 8 */ vm_flags_t __vm_flags; /* 32 8 */ }; /* 32 8 */ bool detached; /* 40 1 */ /* XXX 7 bytes hole, try to pack */ long int vm_lock_seq; /* 48 8 */ struct vma_lock * vm_lock; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ struct { struct rb_node rb; /* 64 24 */ long unsigned int rb_subtree_last; /* 88 8 */ } /* 64 32 */ struct list_head anon_vma_chain; /* 96 16 */ struct anon_vma * anon_vma; /* 112 8 */ const struct vm_operations_struct * vm_ops; /* 120 8 */ /* --- cacheline 2 boundary (128 bytes) --- */ long unsigned int vm_pgoff; /* 128 8 */ struct file * vm_file; /* 136 8 */ void * vm_private_data; /* 144 8 */ atomic_long_t swap_readahead_info; /* 152 8 */ struct mempolicy * vm_policy; /* 160 8 */ struct vma_numab_state * numab_state; /* 168 8 */ struct vm_userfaultfd_ctx vm_userfaultfd_ctx; /* 176 8 */ /* size: 184, cachelines: 3, members: 18 */ /* sum members: 177, holes: 1, sum holes: 7 */ /* forced alignments: 2 */ /* last cacheline: 56 bytes */ } __attribute__((__aligned__(8))); > > > > > > I didn't use __u64 outright to keep 32-bit architectures unaffected, = but > > > if it seems important enough, I have nothing against using __u64. > > > > > > Suggested-by: Jann Horn > > > Signed-off-by: Andrii Nakryiko > > > > Reviewed-by: Shakeel Butt >