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 C1A94CFC52A for ; Mon, 14 Oct 2024 09:33:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 53E246B0082; Mon, 14 Oct 2024 05:33:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4C6406B0083; Mon, 14 Oct 2024 05:33:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 38EB66B0088; Mon, 14 Oct 2024 05:33:37 -0400 (EDT) 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 168E86B0082 for ; Mon, 14 Oct 2024 05:33:37 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id E7E2B120CEA for ; Mon, 14 Oct 2024 09:33:29 +0000 (UTC) X-FDA: 82671695064.30.A796D14 Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by imf01.hostedemail.com (Postfix) with ESMTP id A019B40009 for ; Mon, 14 Oct 2024 09:33:29 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=LB0wBoyM; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of aliceryhl@google.com designates 209.85.221.44 as permitted sender) smtp.mailfrom=aliceryhl@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728898258; 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=5Do+osA+eLZbyPfwfjx6Pg5S9LDhJ9bCJgF0diE+K1o=; b=Ixb9Rq5oj73gxqE3mnIQTo6BUqvdkUQ01ZFo5pOwB3R8DItSlhvpBUEFgea/0benlkFIn5 jXW7yaYswikLQ//rIECBUUsjOFdy9A8VEdaV5E+4k6enwiTQZMBRv5VT7mOb7tDzkwiwZU P3AI8wtinnWK21BXodyvb2dvbHSsbqg= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728898258; a=rsa-sha256; cv=none; b=EbjOsBmBlwNU6yPR2VZa/B1uAybMru0tgvSbqf+0fFwxtR07fPw4ia5odB/pizLZCybgMo KFct/FmEr52FYJo3i/Tk2y2As5PvnKRjC3PGHIWluRLzAjidALMzJsA1KxYCtiWp5+DRFH h5y0D0gDQp9xxgJT7z5GrJrvRsL0Qac= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=LB0wBoyM; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of aliceryhl@google.com designates 209.85.221.44 as permitted sender) smtp.mailfrom=aliceryhl@google.com Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-37d43a9bc03so2451309f8f.2 for ; Mon, 14 Oct 2024 02:33:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1728898413; x=1729503213; 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=5Do+osA+eLZbyPfwfjx6Pg5S9LDhJ9bCJgF0diE+K1o=; b=LB0wBoyM0V1ALIVpo7phEszBm6fBjolflLbx1pjhgzkrFlXD/05Rjok2TP1VVw+R3t EevFGOf0DQ8F2p4w+F8CzDAiq2mSAu/nNu809wJJ703xqC74Y2zeT7wxB52W5zb0CXfx kRu8ZP4psOLu0bmTUKyvMBIv1dI7XoMg8qdXzGwHIfKx+8xQreireyY/I/8xX3QzJTeV /nie7M+1aPu3y/NSpVyzJg3kOh4t3Kh+VUMcfuAtC451JYRJ263eMZOo0MRTLUkpqIl4 w1WEEHdUS7NUhnFncMSqz5ug591rLwdNJIbCENxKYj4WqM66Gow7Z4vZs4gHX+EqOdR7 AsyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728898413; x=1729503213; 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=5Do+osA+eLZbyPfwfjx6Pg5S9LDhJ9bCJgF0diE+K1o=; b=iBlPd5fRejnSNnb+rjV1lDBvxXrWCft7aIBDV71lcSdwrA/qc2OmAU6lzo657s4HY7 PFAU0yse8ZEhqnc72fQibjIBkS1wdlmHOGACSjsP4BSyWgM2sHudhR2ZKFiowHKB1Pa1 BTJ16j0ew8DaXZZgzOWWuhJZnr1xBalkEttIGw9xyoRdV1PeI1zsvJFgXqHkRxOHv2YI aNPcj02BQ+sennu7cphlSG6aA7H/N8XrQ30VqASDQSY1joG2BtoZM8m8xFsk27HH4BN1 3lRa19AAQUaJuPmd0/Ods/kucXskXcMnjOFyjeAOtXbMT0Kx5CtlMu4lyVBdNGa8uqgJ mI+g== X-Forwarded-Encrypted: i=1; AJvYcCUVuJUYxqYYKrykiauG7xHyf1xQlcG4K1NiShTmO/Sdjroh1nlXmJlWN5HrQYVSAxzLNJMfnA7gzQ==@kvack.org X-Gm-Message-State: AOJu0Yx25CPZC//Ht88bZRBEd53ar2cBBrXq5DdUEWxZY38pMy+KSc/O IXdY3f7Pub/VmWT+lqdl21jjIGIYKOas3/dx4K+yaiGOVQ8vt6qy4gxTxawxfNk8fD8/5CUGP01 fSsIuYSb9TsnE3koPTk8Y8mTpK0gXg/WsiEzR X-Google-Smtp-Source: AGHT+IGntwzOQW79lkB9HQi43v6cSR3yaLMLYz/k3lEB9teEmrOcZJQEk65UT2GN5x1c5ODrN3MzKf/h5z+nIxfyczg= X-Received: by 2002:a05:6000:b03:b0:37d:4cee:55b with SMTP id ffacd0b85a97d-37d600c927bmr5096241f8f.59.1728898413070; Mon, 14 Oct 2024 02:33:33 -0700 (PDT) MIME-Version: 1.0 References: <20241010-vma-v6-0-d89039b6f573@google.com> <20241010-vma-v6-1-d89039b6f573@google.com> In-Reply-To: From: Alice Ryhl Date: Mon, 14 Oct 2024 11:33:20 +0200 Message-ID: Subject: Re: [PATCH v6 1/2] rust: mm: add abstractions for mm_struct and vm_area_struct To: Boqun Feng Cc: Miguel Ojeda , Matthew Wilcox , Lorenzo Stoakes , Vlastimil Babka , John Hubbard , "Liam R. Howlett" , Andrew Morton , Greg Kroah-Hartman , Arnd Bergmann , Alex Gaynor , Gary Guo , =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , linux-kernel@vger.kernel.org, linux-mm@kvack.org, rust-for-linux@vger.kernel.org, Andreas Hindborg , Wedson Almeida Filho Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: A019B40009 X-Stat-Signature: hqo6sdoo1niabax8m9jno6uctjwrq173 X-Rspam-User: X-HE-Tag: 1728898409-212567 X-HE-Meta: U2FsdGVkX19sH352AbD8uPB2Q8f7Takedd7DN8ziakuMJN5ztCR8rSjmIEcKeGvg8bq7OvbiGFcN3SPE939csy5uIxkySSirWlerQIQkZ6aTfZjIyzz0pADDAFwf2gEYo/BoSob7v4jdp94+kGax99cjUz8CG31rkjCRXU13b/emlA455caRSOxrd9YR4XPmVmhKQ/DcyWYmajIQPj2iJiC+6D5rAqzMPKfbA98gdpiYWmjAiHrXgfgm9jhZt2zVWnjF7xSe4PtxUT7oJ0ViWwS1pjueBbMmyPFeN96eqJVe4juJn5jHwbHmR/LomH4oBzB4i4XZqGFY4y33AFnuTVq7DCCMLDYA0V53wLcOeVLeDQFDrzzircpdRG4CT/RX2AVg7Lh+rlQH+Fe6A2/XNx2VRl7SfwQqAV3kQGNB5m9XXh2OXkv32BJv1HFNNMmJzA6d8C2O0ZtM/AaivLdH1uRU/QOiw/7B1xXOMiGPmiZZIANX5XWpRip49GcdUZCRrVQ7Nv2tn5nO6LJCI36zFDTKEupSDO2lS5Cu8rOVcgOWDCIx+1gRv9pyhwkx+dpUh3fV20YhmPUIKuKrIMFinyuhwjHi52WoPqeSzwVTvajXjLv6Nlt10zLswQgF5n/OHkirJmQqs2MKaAFQG4B3dz9Jq9uPewnFzZL/9HKAyzv4rJQ1n+AzPCJgFKLjSy+1cjADlofMx2EZ8Dc3ZumOvUFhus0uSdWiSP7OiY1BlkSZ8oAaGXBVStyEzSNCxxe34roBU1sLr+A3XxkJK9/Gcdu18v70Z0+tTTK6AeI+sUQd+e5CLGhUxxvMdLBbZd5y8OgtzvkQ0Ws15NPOgmzNs6XgxS3r5OxJ9nE8ppk5m8hDwbTNl2HNh3h7IClRwWaZyu9EtwE8pk7FYxFSSK4oIBu6NZXfbrC3hXwQK06cCTGy0PC/yTLJT+A+g2kwf6vRD93vWVDncJ3VaTqP7c7 TqVgLh5r NcibG29lMpwc3EsWf6h7B+rl1BFrXYcHFN9bFFRMG7hILlNrNCGC5eE7CxJ9OZ4l3GZ2WOAcsHmcgojtYdUxnDZkwcGGoZhzDT4iktdluf60U0PvLYZkjexs6z8H9hGo5RBM1bYuekbvwcTF1vgd7bfiQ11TkGC023LxX8JUfCUv3chWOctnX9nzYVOtjO0DjLmDhq5SorkO733Q8Vu72CCXttXYDRQ/lIuTtuG7nhlot8rWryDOrNk0Z+xIo6YJkpPfwSHCB0pBER3C6ePa9V6qC+zpG81s1zl3Bu/TG4alRmih6uJHlm/3JbulQCGL4jzW3aHFQFx4jldPa1rpbW+PakUa8arjXKN4JT1SUaJf1FQxR5cTobuxYZAoiQSAW1JQe 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 Sun, Oct 13, 2024 at 2:16=E2=80=AFAM Boqun Feng w= rote: > > On Thu, Oct 10, 2024 at 12:56:35PM +0000, Alice Ryhl wrote: > > These abstractions allow you to manipulate vmas. Rust Binder will uses > > these in a few different ways. > > > > In the mmap implementation, a VmAreaNew will be provided to the mmap > > call which allows it to modify the vma in ways that are only okay durin= g > > initial setup. This is the case where the most methods are available. > > > > However, Rust Binder needs to insert and remove pages from the vma as > > time passes. When incoming messages arrive, pages may need to be > > inserted if space is missing, and in this case that is done by using a > > stashed ARef and calling mmget_not_zero followed by mmap_write_lock > > followed by vma_lookup followed by vm_insert_page. In this case, since > > mmap_write_lock is used, the VmAreaMut type will be in use. > > > > Another use-case is the shrinker, where the mmap read lock is taken > > instead, and zap_page_range_single is used to remove pages from the vma= . > > In this case, only the read lock is taken, so the VmAreaRef type will b= e > > in use. > > > > Future extensions could involve a VmAreaRcuRef for accessing vma method= s > > that are okay to use when holding just the rcu read lock. However, thes= e > > methods are not needed from Rust yet. > > > > This uses shared references even for VmAreaMut. This is preferable to > > using pinned mutable references because those are pretty inconvenient > > due to the lack of reborrowing. However, this means that VmAreaMut > > cannot be Sync. I think it is an acceptable trade-off. > > > > Interesting ;-) I agree it's better than using Pin. > > > This patch is based on Wedson's implementation on the old rust branch, > > but has been changed significantly. All mistakes are Alice's. > > > > Co-developed-by: Wedson Almeida Filho > > Signed-off-by: Wedson Almeida Filho > > Signed-off-by: Alice Ryhl > > --- > [...] > > +/// A wrapper for the kernel's `struct mm_struct`. > > +/// > > +/// This type is like [`Mm`], but with non-zero `mm_users`. It can onl= y be used when `mm_users` can > > +/// be proven to be non-zero at compile-time, usually because the rele= vant code holds an `mmget` > > +/// refcount. It can be used to access the associated address space. > > +/// > > +/// The `ARef` smart pointer holds an `mmget` refcount. It= s destructor may sleep. > > +/// > > +/// # Invariants > > +/// > > +/// Values of this type are always refcounted using `mmget`. The value= of `mm_users` is non-zero. > > +/// #[repr(transparent)] > > +pub struct MmWithUser { > > + mm: Mm, > > +} > > + > > +// SAFETY: It is safe to call `mmput` on another thread than where `mm= get` was called. > > +unsafe impl Send for MmWithUser {} > > +// SAFETY: All methods on `MmWithUser` can be called in parallel from = several threads. > > +unsafe impl Sync for MmWithUser {} > > + > [...] > > + > > +/// A guard for the mmap read lock. > > +/// > > +/// # Invariants > > +/// > > +/// This `MmapReadLock` guard owns the mmap read lock. > > +pub struct MmapReadLock<'a> { > > + mm: &'a MmWithUser, > > Since `MmWithUser` is `Sync`, so `MmapReadLock<'a>` is `Send`? However, > it cannot be a `Send` because the lock must be released by the same > thread: although ->mmap_lock is a read-write *semaphore*, but > rw_semaphore by default has strict owner semantics (see > Documentation/locking/locktypes.rst). > > Also given this type is really a lock guard, maybe name it > something like MmapReadGuard or MmapReadLockGuard? > > Same `Send` issue and name suggestion for `MmapWriteLock`. Fixed in v7. https://lore.kernel.org/lkml/20241014-vma-v7-0-01e32f861195@google.com/ > > +impl VmAreaRef { > > + /// Access a virtual memory area given a raw pointer. > > + /// > > + /// # Safety > > + /// > > + /// Callers must ensure that `vma` is valid for the duration of 'a= , and that the mmap read lock > > + /// (or stronger) is held for at least the duration of 'a. > > + #[inline] > > + pub unsafe fn from_raw<'a>(vma: *const bindings::vm_area_struct) -= > &'a Self { > > Unrelated to this patch, but since we have so many `from_raw`s, I want > to suggest that we should look into a #[derive(FromRaw)] ;-) For > example: > > pub trait FromRaw { > type RawType; > unsafe fn from_raw<'a>(raw: *const Self::RawType) -> &'a Self; > } > and > > #[derive(FromRaw)] > #[repr(transparent)] // repr(transparent) is mandatory. > struct VmAreaRef { > vma: Opaque // Opaque is also mandatory= . > } Well, perhaps. But it's not a trait, so is a derive the right way? Either way, it's unrelated to this patch. Alice