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 72F5AE6ADCB for ; Fri, 22 Nov 2024 22:43:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C15B16B0082; Fri, 22 Nov 2024 17:43:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BC5146B0083; Fri, 22 Nov 2024 17:43:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A8C366B0085; Fri, 22 Nov 2024 17:43:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 8A9BF6B0082 for ; Fri, 22 Nov 2024 17:43:51 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 352FC801F9 for ; Fri, 22 Nov 2024 22:43:51 +0000 (UTC) X-FDA: 82815209862.11.29FFFF7 Received: from mail-qt1-f170.google.com (mail-qt1-f170.google.com [209.85.160.170]) by imf29.hostedemail.com (Postfix) with ESMTP id 510B312000B for ; Fri, 22 Nov 2024 22:43:49 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=oOBOavvH; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf29.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=1732315429; 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=OHORz47FiSIR9q+W9ZVN0yQsYQ1UBFTTA/DlKq9Xp9g=; b=J7C8OQZzGJJ/lxjfW8nwYKz0RnZrapCcYTmaKrk6nZs5CtUoc3TItGuobIvusWyMJKSBFC z0hIDFqGxT1j0aQJZOlpS174QaSMvIT1+0NF3hTcGLGlaosAtqVwuFtJjPwU5alYutZuhQ SDfBtIN/bxUiU/5RCZM12z0Ol3YECiE= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=oOBOavvH; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf29.hostedemail.com: domain of surenb@google.com designates 209.85.160.170 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732315429; a=rsa-sha256; cv=none; b=l2QiTIBeZnGcPM/6EJoGMw4KuEJwpCWCSubHoQKrtX3itAsODILbYm0qEIihL3rNk6X9es lCDE7Zdw6h0dRcNedvCHxYDVnYpi/MC1djC4PULg/khxF1m4IACg9LoEjQMB0KocP45LV6 e5JFRTz2yLACPfnxKIMpI/EzHXMWkps= Received: by mail-qt1-f170.google.com with SMTP id d75a77b69052e-460a8d1a9b7so25161cf.1 for ; Fri, 22 Nov 2024 14:43:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1732315428; x=1732920228; 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=OHORz47FiSIR9q+W9ZVN0yQsYQ1UBFTTA/DlKq9Xp9g=; b=oOBOavvHx6l3ssdru1x8X7yCCLzC+DlW7wuVdCE7Uq04OACd0/Agm/vHPqDv+VhUh+ 9uBGEp3N9jCOuRwPSDBdxm3KMxqXsuNH7oMIBFg5szX+z9zkL9azDXXl6ZyukyKZ1l4l j0u53px5Kh31Of2lvpRmQogZKNeSSLgjNrz/oSRWJOLwAXhyIvTKW5AiqdsX4B20vwgQ 7nYvF39R/cIgaUl5JKAMu924IYJRXshFtlTxO455AzLbJBjU8jzFUUB5j2E4XEZfKS9r 1XOEEMFcIhwfqtaay9Q8Bj6md5PyKXEWKSTvZWwyu5QALOhm9c2IATTifvy93Sp5oPlG i8Tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732315428; x=1732920228; 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=OHORz47FiSIR9q+W9ZVN0yQsYQ1UBFTTA/DlKq9Xp9g=; b=rCqx6OSmMeHOZ/6ci+kTgiqIxSyTB+bBVZxI/h4KG7/BEScce4wVxtiZwbEoeZ6nhC YxIuz7xdzR7dYPE8V/DR62CTR6DrfwThyv/amnCLwy0Jbamq3Ck0Bv6piAyVCmXHk9xM vLQ/+4YXAYZpvukCino1bo3bMEb1eB0ev4jz7YTij8CFSGUkv36p4KDcUv4P+hzS6CoX MNqGv6d7yD5UF4j9ZCjdAHBAh17eE211SavLHBncIQPUnBdgIkUvGknY3gWtFOx5mSaC GZLWpqV7U8zjVgIDasTaW1Rrqn+8+v30xKsmT2TqvtOsSgziMseh/3rtMplLk+B2d6rw A0Iw== X-Forwarded-Encrypted: i=1; AJvYcCXO0BQPg6nxs2m+slq6nfkTE/HDphZTzya11w/w8yWdDvVvcSKjXTQVeDb0YtQ0EH6VdBVKECI0dA==@kvack.org X-Gm-Message-State: AOJu0YxUnrYp6Or0/acmUcQuQTVADiIOPHAR/3n/EmQl7ui9LZnqiegB Mn/mYf1h1OUdwxycmjsbRyZB4D97VmUROcsjslfVtmGNufjMlWxgL12xcuip5qrcmUWyzcwNk/7 vpACZCdrEo+dVCTFnfBr7vzt++eIb4G6jZcuk X-Gm-Gg: ASbGnctm/dT0O+PMUgZ/Kw7YEv2x2A2FUyGhFsqE6a1PP4RLtb/aQ0iWjRj8xeFWkmE HXk+C8x7NT0fcQiY/ZmhaxNAYxqRe70E= X-Google-Smtp-Source: AGHT+IEFX7LWkW1c39d4ISGOpA0oNTEkNboBSr8vIIy/woFz0x8Q9YCXC3T5nxjXIL6abvvQKd+D6KUVCI48zqhhqtY= X-Received: by 2002:a05:622a:8c6:b0:460:b4e3:49e with SMTP id d75a77b69052e-46673dacb95mr259451cf.9.1732315428171; Fri, 22 Nov 2024 14:43:48 -0800 (PST) MIME-Version: 1.0 References: <20241120000826.335387-1-surenb@google.com> <20241120000826.335387-5-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Fri, 22 Nov 2024 14:43:37 -0800 Message-ID: Subject: Re: [PATCH v4 4/5] mm: make vma cache SLAB_TYPESAFE_BY_RCU To: Matthew Wilcox Cc: akpm@linux-foundation.org, liam.howlett@oracle.com, lorenzo.stoakes@oracle.com, mhocko@suse.com, vbabka@suse.cz, 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: rspam04 X-Rspamd-Queue-Id: 510B312000B X-Stat-Signature: 6qzcixzg55jsfeozxz7donsaiku6tcfg X-Rspam-User: X-HE-Tag: 1732315429-612466 X-HE-Meta: U2FsdGVkX1/8bMlLqGdGZ8fYOH3X2qECuanEjxSydM/Hw3leJJjsN1IhOf4DkLcx+WvPAnw30ZC90TLkA42L+FhSmI5oND49PX5w2CTKHpC1abuPWZxw/b4BvRpeZ9xVhEtFbhbM/wGA0PURWznUn3+acral8LC+0qHhz9vOQWxx/NOvBLDf0K/g2QgM0J0lhg93pVQ/OF64Sr+3IyKo7KMK7rK60KWuM6SW61udZNuU4aR/+gTvK7TjYAA6yLUozKfsGthBb2wjLysWzG8vtQmYKpQARcItcsQ6KACJEi3Kq7gifGXQcpqODgywqEnDwmEhfYTxW2RU6YL5++h058aBLljevAbQRpZ1LKGjQdrZZt3DYEjqBiz6UWkmU91ILgnkpCClVMaf1dRdJrT7Q99ZUHNsdAGmHP9QyXqMsh8G3rcrNmR8d2BnsPfO6Xbo4zmiUD7SerXtOm+7CZRybL7l7K2mDsCm29AErKt0Zh0KnJzjrjSVGKeXnDb09AbDG2wcq5riXkRpS/L8r1VAf5tW3A91WMVP353yCX4k1dfl7PbQRLuPhgfWhkhQ/pwBS8OzrBclW7f+ld4VurzHcGiFbVl+VbJQmCoelR1Y3RZC96GrhbVJoQEG7JrwKX2jZP7IqvB2pMJjWWBzZQynI1v9mIjCPkuNOGAOKD0pQeAVjYYRn9nmjBb4/YgLMHVxusiqlqBSyVHjObVsHnY8hDY8fU1iaXkCbDSFNpZ4HnD/ceFcjBY0dq6J0cBRl0cXAYSbNpUk9CQKTYEZoSm5L08hLvgdE6+Lo3Nnk0khs/wHM1FL7q0VtCXFlBuhiS7uME+wrrkfbZrQMf7Cjgbi5nHVGZrEZUdiXkE4FJnzQw8YiDfut9xMwGT8ppv5fagCT8CDApJyzunlrtSt61E27r43aOiPhVFsJemLT1hkke1K/E1izawZppDg2+0KOMmmAp0831LrDLH9rU5FHmM KZLnXacN 6y+D0ojSWmG1ewHwflziZzDTm+cTm9/cd+BDhOBJML2dXCmhtnXOgTJxH7/mzzB7n7F3QTb/3H9j2GpHjvd4wwSL9LFweoZDCvDN+7qN/Nl9PqTC10oQPSAsQwYVFB5cCzUwSPyxhQbtCwOa1oSIYjaFRxBibBWZZf5BdrMRegYfOSOCJMIyvcfhHgT0eb1DCUNUWPRnT6CvQcbdTRIaXM/pgQLbysKgzx8JUpOApT5vAvEoD4kFAeespxfwynaDbclH25NBuHv5uKMZ0bR3bfcI8VbYuTRjmT2O+dkAVReLcSFlrdizO/xd7rN+3MBgNgJ2Q5hJiaVahqsw= 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, Nov 19, 2024 at 10:37=E2=80=AFPM Suren Baghdasaryan wrote: > > On Tue, Nov 19, 2024 at 8:36=E2=80=AFPM Matthew Wilcox wrote: > > > > On Tue, Nov 19, 2024 at 04:08:25PM -0800, Suren Baghdasaryan wrote: > > > +static inline void vma_clear(struct vm_area_struct *vma) > > > +{ > > > + /* Preserve vma->vm_lock */ > > > + memset(vma, 0, VMA_BEFORE_LOCK); > > > + memset(VMA_LOCK_END(vma), 0, VMA_AFTER_LOCK); > > > +} > > > > This isn't how you're supposed to handle constructors. You've fixed > > the immediate problem rather than writing the code in the intended styl= e. > > Yeah, I don't like this myself but the only alternative I can think of > is to set the struct members individually. > > > > > > +static void vm_area_ctor(void *data) > > > +{ > > > + vma_lock_init(data); > > > +} > > > > After the ctor has run, the object should be in the same state as > > it is after it's freed. If you want to memset the entire thing > > then you can do it in the ctor. But there should be no need to > > do it in vma_init(). > > IIUC, your suggestion is to memset() the vma and initialize vm_lock > inside the ctor. Then when it's time to free the vma, we reset all > members except vm_lock before freeing the vma. As you mention later, > members like anon_vma_chain, which are already clear, also won't need > to be reset at this point. Am I understanding your proposal correctly? > > BTW, if so, then vma_copy() will have to also copy vma members individual= ly. > > > > > And there's lots of things you can move from vma_init() to the ctor. > > For example, at free time, anon_vma_chain should be an empty list. > > So if you init it in the ctor, you can avoid doing it in vma_init(). > > True. > > > I'd suggest that vma_numab_state_free() should be the place which > > sets vma->numab_state to NULL and we can delete vma_numab_state_init() > > entirely. > > Sounds good to me. I took a stab at it and the result does not look pretty... Couple notes: - vma_init() is used in other places to initialize VMAs allocated on the stack, so I left it alone for such cases. VMAs like that are not allocated from vm_area_cachep, can't be reused anyway, therefore we can override their vm_lock. - Since vma_init() has to stay, we can't retire vma_numab_state_init() because it's used in vma_init(). - I think resetting members before freeing might not be such a good idea because after resetting the object might not be reused at all. Now, the main point: I moved initializations of several members into ctor but even with that the code looks roughly like this: static void vm_area_ctor(void *data) { struct vm_area_struct *vma =3D (struct vm_area_struct *)data; vma->detached =3D true; INIT_LIST_HEAD(&vma->anon_vma_chain); vma_lock_init(vma); } struct vm_area_struct *vm_area_alloc(struct mm_struct *mm) { struct vm_area_struct *vma; vma =3D kmem_cache_alloc(vm_area_cachep, GFP_KERNEL); if (!vma) return NULL; vma->vm_mm =3D mm; vma->vm_ops =3D &vma_dummy_vm_ops; vma->vm_start =3D 0; vma->vm_end =3D 0; memset(&vma->vm_page_prot, 0, sizeof(vma->vm_page_prot)); vm_flags_init(vma, 0); vma_numab_state_init(vma); memset(&vma->shared, 0, sizeof(vma->shared)); vma->anon_vma =3D NULL; vma->vm_pgoff =3D 0; vma->vm_file =3D NULL; vma->vm_private_data =3D NULL; memset(&vma->vm_userfaultfd_ctx, 0, sizeof(vma->vm_userfaultfd_ctx)); #ifdef CONFIG_ANON_VMA_NAME vma->anon_name =3D NULL; #endif #ifdef CONFIG_SWAP atomic_long_set(&vma->swap_readahead_info, 0); #endif #ifndef CONFIG_MMU vma->vm_region =3D NULL; #endif #ifdef CONFIG_NUMA vma->vm_policy =3D NULL; #endif #ifdef CONFIG_NUMA_BALANCING vma->numab_state =3D NULL; #endif return vma; } I can of course add helper functions and get rid of the #ifdef's but still.= .. Matthew, want to double check if this looks like the solution you were proposing or am I completely off the target? > > Please confirm if I correctly got your idea and I'll update this patch. > Thanks for the feedback! > > >