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 3834EC3DA64 for ; Tue, 6 Aug 2024 14:21:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A9F546B0092; Tue, 6 Aug 2024 10:21:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A28D16B0095; Tue, 6 Aug 2024 10:21:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8C99D6B0096; Tue, 6 Aug 2024 10:21:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 6B2E06B0092 for ; Tue, 6 Aug 2024 10:21:58 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 161C4160266 for ; Tue, 6 Aug 2024 14:21:58 +0000 (UTC) X-FDA: 82422034716.02.1755E7C Received: from bee.tesarici.cz (bee.tesarici.cz [37.205.15.56]) by imf02.hostedemail.com (Postfix) with ESMTP id F0DC68002C for ; Tue, 6 Aug 2024 14:21:55 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=tesarici.cz header.s=mail header.b="S/DEkhlz"; spf=pass (imf02.hostedemail.com: domain of petr@tesarici.cz designates 37.205.15.56 as permitted sender) smtp.mailfrom=petr@tesarici.cz; dmarc=pass (policy=quarantine) header.from=tesarici.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722954032; 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=v08YXF9LA1d68ac5XDr1akLL9Fn0pTDGFBld6nEjRIw=; b=4alTSy25Y29sRc+dYVG7IIhNyZ4ZfWm6OD1PhtmVkpGdoUBBdBFE5BR760ndFDVfb4LPIH kuNXc2aC2M4Dod/Yj7uI/VHdaK+F3C9BEebsfsJ1KFXIXqxe6r+xGgfZvM9pg6I9rstfrW jE7Zdq65jg18eHhk2GwrYEWqopJDInk= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=tesarici.cz header.s=mail header.b="S/DEkhlz"; spf=pass (imf02.hostedemail.com: domain of petr@tesarici.cz designates 37.205.15.56 as permitted sender) smtp.mailfrom=petr@tesarici.cz; dmarc=pass (policy=quarantine) header.from=tesarici.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722954032; a=rsa-sha256; cv=none; b=PR850iVaa9eaHNqIHaCTgoNk4Snrd0KFf9/I2CimwPXzF57/0m0CEt9T3AYQC31AOsQ+Km geOGEZbHs8py92faOHtE7oViZauhmxjKDxkaRsGpDQgUnxtPnWrWgzmCTU8Af+6GzVNqXm Wnfe75DomxQn2tPwKBmTqWYn7UeGjJw= Received: from mordecai.tesarici.cz (unknown [193.86.92.181]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bee.tesarici.cz (Postfix) with ESMTPSA id 4B34A1D0BD7; Tue, 6 Aug 2024 16:21:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tesarici.cz; s=mail; t=1722954114; bh=v08YXF9LA1d68ac5XDr1akLL9Fn0pTDGFBld6nEjRIw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=S/DEkhlzxdrWiUm/KJQ7gEMMv7PcqeSJuv+DV/Fw32TBB1z2/3M4UvE2Ja9dJNqlx DM0NBGm4U3jeSEbJnI5qeR2wMarxh7UTpowrkHFpa4/1FBul7A34TOSuhXl1RLDq1B iPINN7GStqq7bntmeHoj96sWR1Fc+nku31DMITzBrwFGysfjSzQKfdh7DiVBd6laYn sIN+LILfjAR4vANBWskubXOCxHtousI356YdUiGgOqG4xccgCqYQ1vuShvOdhM5z7V YU+uuN7SAHvrPvUysNrVZnw0azBRvx8TJsSkwUxNYCx4bN15nG1+uqlV834rITJfAu kkRQMkRcHaUtQ== Date: Tue, 6 Aug 2024 16:21:49 +0200 From: Petr =?UTF-8?B?VGVzYcWZw61r?= To: Lorenzo Stoakes Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , "Liam R . Howlett" , Vlastimil Babka Subject: Re: [PATCH 10/10] mm: rework vm_ops->close() handling on VMA merge Message-ID: <20240806162149.5d1de9e1@mordecai.tesarici.cz> In-Reply-To: <599a5806-5209-4454-8d4c-60e458905f2c@lucifer.local> References: <0afd85543d46fd743c0c71b6f6520f9580174b4f.1722849860.git.lorenzo.stoakes@oracle.com> <20240806155555.1125554c@mordecai.tesarici.cz> <599a5806-5209-4454-8d4c-60e458905f2c@lucifer.local> X-Mailer: Claws Mail 4.3.0 (GTK 3.24.42; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: F0DC68002C X-Stat-Signature: kncf59w38uo3q33kxj7ss33fbrbe4kdy X-Rspam-User: X-HE-Tag: 1722954115-863307 X-HE-Meta: U2FsdGVkX18jj0LZxfvgsB4B97R+q330lIrTO++wPY0P/YC4KB66Mo8semqq4dPpiVb4HGobN0hLCkmqYk5Kf8+R9v8WZivrn9ESjPvcEYxJwgAay8fbkpluE+GzbcGUd8p4aVQC7QoohAki0tmuNGXiXOaaeqZu678CZxMsnTRR2CeSW8ZDBXqAxIX4ZqzJcoIkiLAO8+o2YRFe1heZC7NYv+/9d57y/a4yhy+PSG61gi4vYCYKyryy75XbOmRLMXwzF4yPDpAfuwz2jd/W/VjtlfIBgAi6LLOjKfufqk9ITxw3BMQoTHGj8oC/hwwpdkJiKW2qALwPA82m9RO3sbo51MnlUS9foTf+aAQVsWmBhNfHgqapKiEj++pDp5yikEnPi0NDgmQ79eHEvEem+TMF9YllAw+dmkQC3RshV4xV0lb+zRykHvJVAAF4/RomUjYZ6d3cannHcL7EOdjL4U3kC2+pXxlupeKKaXh2S2l5omma3Bfg/TSdxMRe7X0jVDSZ80PUna9DW4AckXEdBdAnbVD2MTZcfNx0watiXOvedMZlumDd4SyFu4OOGBzVu4ClBXtujJpE18Es9G2wHa20e8cse/oi+ilBlqUiR8ArKTGg+wfF3LIT+JJ2BkNoHe214Sa9KlETyoqwPOjT22Y9VBxRviT/831t7dQbxBoaBrF3OxJI19Jz2ep21ISHfji2c4hNHdy/3sfGtq2C0DtgDKjXTJJ3mZ9iINf3FieAEylTwvSBjupVRiR2aDErBQYogUAUmKLPoYFVIHXZiQI/CYHBaAu93VpwkoxaxJvUsa7kLp8E7j+esrZB+EaYgb7Pj8ogGbNIjzs8GluW0ShUZeJq6bpoKgl+r0VT1gHeOvXjp5UFe0BRdOlFskpJ9Hw3v2q38vYt6ZChDLJb6kxKBULXikyzu8Sb6d9MlMj5N6MVuTpJHx5VaduCv9ugks6g8YfNmz69KgGdm0k U82OCFka 9ufEUv08ASexsqHTQECIQDx1C8qMIpXcMEf6WhvdmefIiPvZJ+eN+i8VpD58baGjSWLEM6/99wYfkbrQydtn+FL1atetieyE4XIcmnHbuYDJLYwwZkK5FD6A35nPZBCZkKCx4WMp9AxFJDFiR14gxToY56ZhZqmlzcbKS1f/NcapqXXyb20lHFAVlbIHuMY0MOMEFfuNGsgTV9exlqyu06Wuyxo+ufDtANGJqjJQ73vvkBHvpfHD1HoF1kEQw/9OSdAbKVFL30ZuWPn5nAX2ikHQIZl34B0+7DTQnietDg6DeADDifiyIpfoc5fotLXhIyWsdYwuxhGwBanIxcrTD+jeVeF1lBfRv5obi 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, 6 Aug 2024 15:08:33 +0100 Lorenzo Stoakes wrote: > On Tue, Aug 06, 2024 at 03:55:55PM GMT, Petr Tesa=C5=99=C3=ADk wrote: > > On Mon, 5 Aug 2024 13:13:57 +0100 > > Lorenzo Stoakes wrote: >[...] > > > @@ -886,6 +894,8 @@ struct vm_area_struct *vma_merge_new_vma(struct v= ma_merge_struct *vmg) > > > unsigned long end =3D vmg->end; > > > pgoff_t pgoff =3D vmg->pgoff; > > > pgoff_t pglen =3D PHYS_PFN(end - start); > > > + bool merge_next =3D false; > > > + struct anon_vma *anon_vma =3D vmg->anon_vma; =20 > > > > Calling this "anon_vma" feels a bit too generic. IIUC you want to save > > the original vmg->anon_vma in case the VMA turns out to be ummergeable > > with the next VMA after vmg->anon_vma has already been modified. > > > > What about calling it "orig_anon_vma"? =20 >=20 > I disagree, that'd be unnecessary noise (and this is applicable to _all_ > the fields). I'm afraid I don't understand what you mean with _all_ fields. FWIW my comment concerns a local variable called "anon_vma", not a struct member called "anon_vma". >=20 > Again we come to some trade-off between readability and inherent > complexity. I am not a fan of making variable names unnecessarily > overwrought. Then call it "a". ;-) See additional comments below: >=20 > In this case it's just a short-hand, as the only instance where we'd retry > the operation anon_vma would be NULL (from mmap_region()), so we reset th= at > to NULL, however strictly we should reset to anon_vma. >=20 > I'll change that on the next respin just to be strict. >=20 > > > > Petr T > > =20 > > > > > > VM_WARN_ON(vmg->vma); > > > > > > @@ -916,8 +926,9 @@ struct vm_area_struct *vma_merge_new_vma(struct v= ma_merge_struct *vmg) > > > vmg->end =3D next->vm_end; > > > vmg->vma =3D next; > > > vmg->pgoff =3D next->vm_pgoff - pglen; > > > - > > > vmg->anon_vma =3D next->anon_vma; Here, vmg->anon_vma is modified. Original value is lost. > > > + > > > + merge_next =3D true; > > > } > > > > > > /* If we can merge with the previous VMA, adjust vmg accordingly. */ > > > @@ -925,6 +936,16 @@ struct vm_area_struct *vma_merge_new_vma(struct = vma_merge_struct *vmg) > > > vmg->start =3D prev->vm_start; > > > vmg->vma =3D prev; > > > vmg->pgoff =3D prev->vm_pgoff; > > > + > > > + /* > > > + * If this merge would result in removal of the next VMA but we > > > + * are not permitted to do so, reduce the operation to merging > > > + * prev and vma. > > > + */ > > > + if (merge_next && !can_merge_remove_vma(next)) { > > > + vmg->end =3D end; > > > + vmg->anon_vma =3D anon_vma; But here you need to restore the original value of vmg->anon_vma. Isn't this why you introduced the local variable "anon_vma"? I believe it would be easier to understand its purpose if it includes the "orig_" prefix. Just my two eurocents. Petr T