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 2E531E77199 for ; Thu, 9 Jan 2025 09:50:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A48426B007B; Thu, 9 Jan 2025 04:50:32 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9F8336B0082; Thu, 9 Jan 2025 04:50:32 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8F0616B0083; Thu, 9 Jan 2025 04:50:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 6FBC86B007B for ; Thu, 9 Jan 2025 04:50:32 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id F057D1A1813 for ; Thu, 9 Jan 2025 09:50:31 +0000 (UTC) X-FDA: 82987443462.23.50C5268 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf09.hostedemail.com (Postfix) with ESMTP id 3EA39140008 for ; Thu, 9 Jan 2025 09:50:30 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=KCFYhm2H; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf09.hostedemail.com: domain of a.hindborg@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=a.hindborg@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736416230; a=rsa-sha256; cv=none; b=6M4ukk6EngWzKAa3/OG1uRbXB8KzRrG2Cir5yOTbowOV6Vqq0M6HTe9gVvMDCbGOvRI9OI DnjNxJPdFkCzUhUrzsWKMqWoY6mUvv+tx9uy2AVsG/BskjPL5B6duz+FJJ3Ed8m13HhHJU lZOqGDXVkJBPVcfVXnjDaJM1iNwmT0A= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=KCFYhm2H; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf09.hostedemail.com: domain of a.hindborg@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=a.hindborg@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736416230; 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=e20oGQ9M3vZ55I0XZGHLFme/8PWm/OYGVqePEl1ybPU=; b=1n6/DFDW2yy2zTJyV2my/7eO31d3qxu5sm3ynMPd+zP1YyY1kGSjfmtr7jbJ0VvUiHfH/N EU6zlrWsPurt3fEBhAGTAWSHR+JhnMTN86KBAyRW8MZXugr+RotBWa77A9WMhojcEXnMFt wfdzHvUrirkDbK7Ezp0grEaFyUFUCmg= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 2AD0E5C5727; Thu, 9 Jan 2025 09:49:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E8EF6C4CEE3; Thu, 9 Jan 2025 09:50:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736416228; bh=OTmSpk8FgQtdGzsBLCapjkB10EVINFX+b6VXc1DcaII=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=KCFYhm2HAlgySDT+VDj7M7/hEOT7/f8m6lRANiIi0yf6cvrbkvUllkmlDD/6NPhWd Srq4DwwmLepLl5xDfb3d7XEZkB+HvnPTD+pGCvs5n4ai6LZFQebDyf+Inv5cmmX8eO 08BI2G9umgJDu54D87WXproEllZuBmS/2yGaE2EyWhmUrKmudf7+9lnWqJ31cKM3Qi ilAunlKUd72eq4/T0rdDmz8VXruZOWiW4Hlhij0SlDGN04n3Po55Ax2itg5n8dhcIz OEoONJ63RL/ygTgorac3pq0tQEI1y1MFC9Ye7NTvSQdt1pnfxZ8TqWa9OC4G1+KSOl lWF01p11kqGHA== From: Andreas Hindborg To: "Lorenzo Stoakes" Cc: "Alice Ryhl" , "Miguel Ojeda" , "Matthew Wilcox" , "Vlastimil Babka" , "John Hubbard" , "Liam R. Howlett" , "Andrew Morton" , "Greg Kroah-Hartman" , "Arnd Bergmann" , "Christian Brauner" , "Jann Horn" , "Suren Baghdasaryan" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?Q?Bj=C3=B6rn?= Roy Baron , "Benno Lossin" , "Trevor Gross" , , , Subject: Re: [PATCH v11 2/8] mm: rust: add vm_area_struct methods that require read access In-Reply-To: <18bc911a-ede5-410b-9955-5382bcef975c@lucifer.local> (Lorenzo Stoakes's message of "Thu, 09 Jan 2025 08:19:53 +0000") References: <20241211-vma-v11-0-466640428fc3@google.com> <20241211-vma-v11-2-466640428fc3@google.com> <874j33ddxt.fsf@kernel.org> <51PsGz5tctBZlyC7TWAGRwZbM13r71BM1gtm1Y8F4j2w3FtKSXtkVrsvwILAqvSBPrFJzahyUVDum-JXO3yZUw==@protonmail.internalid> <87frlsbekc.fsf@kernel.org> <18bc911a-ede5-410b-9955-5382bcef975c@lucifer.local> User-Agent: mu4e 1.12.7; emacs 29.4 Date: Thu, 09 Jan 2025 10:50:13 +0100 Message-ID: <87msg09uzu.fsf@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 3EA39140008 X-Stat-Signature: fespb8ydnroszhy37nhq6kizh3pabbkk X-HE-Tag: 1736416230-430173 X-HE-Meta: U2FsdGVkX18avpNojkFFwhzQBXbHk58dw/C0jOQjIbnDFQoFwqBbg7H6iqZ5jm8yt1WWYkjVJwApS6/c7Peg04vrTsNkNye/Wya6FA+2+odmsUbh7G+2md22wJeaCdO61h6222OEQPRK8RmsxByI850d1DN+11HnBcHxYClco5NuBJ+ilFTewPajuxHIbP+7gY5XBZs2iXvx0WDPfNuDNJAa+ivVgzTZuKD6xY6ph8yuzbY39ELUvsqPAw+Sn21bTxdo8kt3yv2YqIoNO/0BA23sYuXJ9+aSwwcQGUbJettW5ZuGX6nKlrvwm2SaB4cFPrtoXOl7SDg6vGCobb1ssQkpqN21dhoLt9aW8o7EPB2/hlj9kAF3fnHgNPJylprG8yQxXkfRXf9w3snsPFi+Fgo5oGtaxnffKAR4Yv6hAclBsD8Yo3PnbfbxY96Wn+xcf5ST3bae73CdkaD/wzJiGcYXDZOC+54lTU3Peo0XdzFUrwiALSXusGnjXRhUI1NSf5MxB+7Xrr/vpcsJfTmjcbjC6aQ+NNY0+txNv2aKrepSumohEvo7KVRnJdBHji2NQTP8/tRSSyOkMbicvGNHJnBSDdeGb/SjaTcWnaU3vCLCoKrXm48/1JklAnhD1lhKfNyMfTbGdJyHECkEhYdaw7t5lSTFgxiFCw0fGdPsvokt+1Bl9ZbVEHzvMjrBvxYfo/ERTpYWDxuIwkbOdV3zntpdtlqUniGuwmUAZqjfvCxW6uWOM6Jhx7ulZpnRKpZXNVtzQ45JvUInvt19NOmNL9wjW/a29DXJFyx2USCXqukHO8kdOxlzcRGBJ6vQ4CmfAVFtNvfHlU8x7jyqGGLdFtIlSGbQnmVOU1QVbUptdCZylM67iinSWUjq8zGc6K4eJUTNlmqt3yW+de8nIO8NXWZQHUHerG4tG1biRGd3OcCy36EnTtLwOMYrbmGRq7ebFQhkthd6vY1EVvtFkVH VG3TQmI3 Lz8eXvnPX+uIj9KpO8F65a50z/PeGfKtYA+xwn8C+4wbr78Y9voKAOnYo+LZjYuyQMgU9XR3fMgrNyd5qazLi2sHZf8RZJwG+fJReC/HqMcOanAOmWAlfeU6H9YDL/rI31kuXy9vP+vsKLKUOvDYXCsbFi/W+wVkqqWArs3UiO09tDHt2I7swQhCVSLWBAZfaS9E9iGkbNVWoTFQJsIaUGbV2tdFuSEktbYNrMRg1z7RSF2/+SFFQcdXY6MNCN+98NwApkmI4lC/yA60IA7rR974g+rwrT+vz+XeYrgi8mD7APEQCTXiKvqSyr9s6Ah+RM2JQHPdUeAjBdUFG0ZC03n+Nz2BuZiJU8T6n/TyXRwIJcvSN5TVwQA9Q8uD7Wos15gG/5PgWo1bj43TxsyAOzCGeNIkptGF45g9g X-Bogosity: Ham, tests=bogofilter, spamicity=0.361988, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: "Lorenzo Stoakes" writes: > On Thu, Jan 09, 2025 at 09:02:11AM +0100, Andreas Hindborg wrote: >> "Alice Ryhl" writes: >> >> > On Mon, Dec 16, 2024 at 3:51=E2=80=AFPM Andreas Hindborg wrote: >> >> >> >> >> >> > + >> >> > + /// Zap pages in the given page range. >> >> > + /// >> >> > + /// This clears page table mappings for the range at the leaf = level, leaving all other page >> >> > + /// tables intact, >> >> >> >> I don't fully understand this docstring. Is it correct that the funct= ion >> >> will unmap the address range given by `start` and `size`, _and_ free = the >> >> pages used to hold the mappings at the leaf level of the page table? >> > >> > If the vma owns a refcount on those pages, then the refcounts are drop= ped. >> >> Maybe drop the "at the leaf level leaving all other page tables intact". >> It confuses me, since when would this not be the case? > > I don't understand your objection. The whole nature of a zap is to traver= se > leaf level page table mappings, clearing the entries, leaving the other > page table entries intact. As someone not deeply familiar with this function and it's use, I became uncertain of my understanding when I read this sentence. As I asked above: When would you not clear mappings at the leaf level and leave all other mappings alone? Imagine you have a collection structure backed by a tree and the `remove_item` function has the sentence "remove item at the leaf level but leave all other items in the collection alone". That would be over specifying. It is enough information in the user facing documentation that the item is removed. You don't need to state that a remove operation on an item does not remove other items. Does this example transfer to this function, or am I missing something? > That is, precisely what is written here. In fact I think this > characterisation is derived from discussions had with us in mm, and it is > one with which I am happy. > > Why is it problematic to accurately describe what this does? Again, it might be that I don't properly understand what the function actually does, but if it is just removing the entries described by the range - write that. Don't add irrelevant details or specify what the function does not do. It slows down the user when reading documentation. > > For a series at v11 where there is broad agreement with maintainers within > the subsystem which it wraps, perhaps the priority should be to try to ha= ve > the series merged unless there is significant technical objection from the > rust side? > >> >> How about this: >> >> This clears the virtual memory map for the range given by `start` and >> `size`, dropping refcounts to memory held by the mappings in this range.= That >> is, anonymous memory is completely freed, file-backed memory has its >> reference count on page cache folio's dropped, any dirty data will still >> be written back to disk as usual. > > Sorry I object to this, 'clears the virtual memory map' is really > vague. What is already there is better. Would you like the proposed paragraph if we replaced "virtual memory map" with "page table mappings", or do you object to the entirety of the new suggestion? > >> >> > >> >> > and freeing any memory referenced by the VMA in this range. That is, >> >> > + /// anonymous memory is completely freed, file-backed memory h= as its reference count on page >> >> > + /// cache folio's dropped, any dirty data will still be writte= n back to disk as usual. >> >> > + #[inline] >> >> > + pub fn zap_page_range_single(&self, address: usize, size: usiz= e) { >> >> >> Best regards, >> Andreas Hindborg >> >> > > Let's please get this series merged. I think Alice has demonstrated > remarkable patience already, and modulo significant technical pushback on > the rust side (on which I defer entirely to the expertise of rust people), > I want to see this go in. I am sensing that you don't feel my comments are relevant at the current stage of this series (v11). Alice asked for reviews of the series. These ar= e my comments. Feel free do whatever you want with them. Best regards, Andreas Hindborg