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 A7090D44148 for ; Tue, 19 Nov 2024 12:11:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1AF956B0099; Tue, 19 Nov 2024 07:11:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 15F9B6B009E; Tue, 19 Nov 2024 07:11:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 027386B009F; Tue, 19 Nov 2024 07:11:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id D62516B0099 for ; Tue, 19 Nov 2024 07:11:21 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 9473D1A031B for ; Tue, 19 Nov 2024 12:11:21 +0000 (UTC) X-FDA: 82802727378.27.F11E4F8 Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) by imf13.hostedemail.com (Postfix) with ESMTP id E3EA220006 for ; Tue, 19 Nov 2024 12:10:26 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=rz++ZBkZ; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf13.hostedemail.com: domain of aliceryhl@google.com designates 209.85.221.49 as permitted sender) smtp.mailfrom=aliceryhl@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732018145; a=rsa-sha256; cv=none; b=0oAuR9QM+rjaQOS3unycqvD7ohp9y1yD8jYY4Lponcb9YTrrOice5oh3YF3zrKgz2qUgCW 7ogjcj4SkdT9orAoHd4Ccc0DXQ1CYDirMhopFMGp8MWxxfrkTOO3zLi+zBfCljZtUamLRM plsbP9LCqHk8XTLYPq/hHoHO86WxR6A= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=rz++ZBkZ; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf13.hostedemail.com: domain of aliceryhl@google.com designates 209.85.221.49 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=1732018145; 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=R0AK29HVbXNe+v5FqEWeSVJhzv0SqiMS3JRu+eQ7aaQ=; b=06cNIXaiR+1G8KC2hh9z3+v+elAfsu6o8wWqknJP/poJNYqlPelgYjO/u1Wa4TezbRuQJf wG/1fxU2HGMuS84KbOWHepHH03HoDguApt1/FcrMldYs/CB5X+IFsVc6t5bLxD4CkGJ6FB FWu3GU2mWzPsC3lsVtuqUkiKdTmPcSY= Received: by mail-wr1-f49.google.com with SMTP id ffacd0b85a97d-38231e9d518so2505200f8f.0 for ; Tue, 19 Nov 2024 04:11:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1732018278; x=1732623078; 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=R0AK29HVbXNe+v5FqEWeSVJhzv0SqiMS3JRu+eQ7aaQ=; b=rz++ZBkZ2BIWPcS04Lupe1nPcWEuJelctNCGbuifU/OMSpUh8Cox74DzCYDMFB4vS4 Kw9qAKntP3R+3kLthXIo9gtlWQhG+VqpcokJw2drvRFhA649xzQCEgNOhXfwkQFubXHd 4OgaIndhpQvRTYUBp4DekZ29hm9YFjsUfQeDQOSs/SbpryTCZVi+vc+HwiK17LvwhOk9 ayWCH7f1F2ou/M+yLcTGI1fnYLirTlHI63FhshUE9vDQbep2j6lQ/TBm9tiqLRHuOe/F LqnKkwgBt8/2XLzs44/KHt2P/gvERyOqAaCoxNPzCj38iyBGPeA3Z7+Iz8xYzxIS+z5Y AxAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732018278; x=1732623078; 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=R0AK29HVbXNe+v5FqEWeSVJhzv0SqiMS3JRu+eQ7aaQ=; b=wfsjou4LI+X6A3FQ9pgM+1txIaJIMC+/RsgEd+LeNSE41tnRStaQ1QDd4CCS+kRtxh pQfBAQmjDndI0gJpcwNOkEyA8GqBnrfZpQcCPuO85n6AWz2gNi0C6umnMTSfqkiqAMij JDuNFJYOeK1CHjABZQ0uyb/NodP+KJO2eSrBuYKC15Y0zNyKSuAbALzUTwZUcwNHTXL2 fd/5iYLJaiVCQLysCeCPkcyATKigcoJFk79p+eHi7fIRgDqIVXfGtf6VHIMFLjAcE3U4 NH2+E7Lpd3RhbXwXu+1iKwChfIrFm0sjTjOBTl/RdDM4jgYRdgLxntPwt8E/PmhqQ2/e givA== X-Forwarded-Encrypted: i=1; AJvYcCV2YAbSYqavQYsd8lfVZiLCv6aTCMCP7OAbPL+ZgknLX9l3+5lhB/ojI88FesTUn88hVUbO/HroUA==@kvack.org X-Gm-Message-State: AOJu0YxHGRaMqo3csQCUeGU1m/qj/bdzNFOBV+Sob9ImjGxqTLqd03Jb LR1JY2XGqB5sVt+u9PEuwTUN1FP53aFEhllt3c5iIRqfv0CEBP+9s77MRGh7af2HfyoCpgWkskd TO7M1m0r0velFNnhFCg58p15ojgfBLDgMyG5v X-Google-Smtp-Source: AGHT+IEKtsIARus6mKK5ASfADNSrf5QYJBJdJ237SnQ9mMhtg4z8AnizImg/prdEA84rjf2Tjy9SdSfo+610sNb9LY4= X-Received: by 2002:a5d:584b:0:b0:371:8eb3:603a with SMTP id ffacd0b85a97d-38225a0e1dfmr13371820f8f.27.1732018278176; Tue, 19 Nov 2024 04:11:18 -0800 (PST) MIME-Version: 1.0 References: <20241119112408.779243-1-abdiel.janulgue@gmail.com> <20241119112408.779243-2-abdiel.janulgue@gmail.com> <3c546153-5677-41e6-9bbe-dbf64de751da@gmail.com> In-Reply-To: <3c546153-5677-41e6-9bbe-dbf64de751da@gmail.com> From: Alice Ryhl Date: Tue, 19 Nov 2024 13:11:06 +0100 Message-ID: Subject: Re: [PATCH v3 1/2] rust: page: use the page's reference count to decide when to free the allocation To: Abdiel Janulgue Cc: rust-for-linux@vger.kernel.org, Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Trevor Gross , Danilo Krummrich , Wedson Almeida Filho , Valentin Obst , open list , Andrew Morton , "open list:MEMORY MANAGEMENT" , airlied@redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: E3EA220006 X-Stat-Signature: 84ccrntn8jabhw1znfykncyswiypjsx8 X-Rspam-User: X-Rspamd-Server: rspam05 X-HE-Tag: 1732018226-284891 X-HE-Meta: U2FsdGVkX1+8Wq3q2NGHgZurGRu4Y4gRyJI7INdxejdxFMNAiijRkzRX2fYrcVTHU+Wtu1ln1heEBVWnHfal1nw4Yuutao6lw9cPX+OP80tnFIal7sMLlD6ZCWTCMNiXD3GmaOWgHf04DUKgRMxh+MPEpZBO7oBQW7oODVYoeijsi80QpcMHurQsIxTgm0hZzXM7srDo/1m0TRRLFknn4y/BTFVVDIlBZNOAJAy/NliXJccaGwzH+kuQRWPdQw1nylCakXyHgPcoPljsh5HU+RX/gbwmX8Pi4mWUMa4Oa3o54ZBSFDPeV0z7CF420qeTttalJrm7iNJSeUcXpmjkMm1xrov9tLGurKunHXU1W6xL3nmOGTVZocZsVGRUanL0xsHMfjKwe7RQzeCmsGUv1BUIPcmlQ9FBtlhqLs25/yzHNHGW2qxHvLIhREbpTdLygY4U+kfVcDboZtX1abfUQ+F0K5I0OG6nLOAVtsQ1PseO6Gh1dgCyGEaHq2IBeErPjNQFLAEAH9wOk+LWcr5QbTsPQc+6YcHL1FglPjYUMD2EMqHhlZ56yWLWWBZvmcFlAnTIgRp06816qFazw/5GLt+/tyMYtPTIVkJIuUYwzznSNWZ4j2Owp7/2FU+asQjmcCQpOrnOL12otVwbx1mbnx+nHpNesM8JCnn1sbfHFdSymOnb3St4qMxAsg+qw0jn5AZDymFVdy7kqZ2bz4kHCInXMJ9HXcZRFs5qDQsKgqVDOxaf7nvLfDg+kgT9040r8W/0hiytk9QKVOSsWVg59quIATG8VZBnnKA9N16lEHhNI76bYJjlY7+jON1YlL+PYOOKMXNnMK6JZEBFB6TCl1sysaByjj+TXo6CNHFmmjtuyNpMmiQLUIp6rJEQqpJ6l25Yzt4P34V8V8zbE33XETSrBTY1qqmgZOYE1lw1vXybmMXOWe7fh96PJ5ACwa4e8ZU0agwrSG0ro648u/N 8y9SL4um AVkO1d6hoVQHQQUCOPrY4m4qZXsqZ+VAYQpPU7FnaN9jLQYvf+EG2JVed3C9sByr+oPjrWK1fCddGJzitfThhAeFOu4BMzPmow3TZ1pnh+4ntBXlwZFI1SwH2QGRRNKJUYwjjLfZmMX58xUmYJVcXM8Ngo+SDn049uZ3QWTH95TMJxBg3KEIUJvkVVdEOwidjhoWlSNc/EP/blCps95c9tkL6eAx2D6nP7/g7D/ZGoUNzJhFG8/mH4ktpx9ptxXxr/XZkk2KVFB2PjBtfzqa620WVyIeFDupE8aeLBALdXDLOG4Ib9vnjAK2k5wlWRksdDZUM X-Bogosity: Ham, tests=bogofilter, spamicity=0.160708, 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 1:06=E2=80=AFPM Abdiel Janulgue wrote: > > > On 19/11/2024 13:45, Alice Ryhl wrote: > >> + pub fn alloc_page(flags: Flags) -> Result, AllocError>= { > >> // SAFETY: Depending on the value of `gfp_flags`, this call = may sleep. Other than that, it > >> // is always safe to call this method. > >> let page =3D unsafe { bindings::alloc_pages(flags.as_raw(), = 0) }; > >> - let page =3D NonNull::new(page).ok_or(AllocError)?; > >> - // INVARIANT: We just successfully allocated a page, so we no= w have ownership of the newly > >> - // allocated page. We transfer that ownership to the new `Pag= e` object. > >> - Ok(Self { page }) > >> + if page.is_null() { > >> + return Err(AllocError); > >> + } > >> + // CAST: Self` is a `repr(transparent)` wrapper around `bindi= ngs::page`. > >> + let ptr =3D page.cast::(); > >> + // INVARIANT: We just successfully allocated a page, ptr poin= ts to the new `Page` object. > >> + // SAFETY: According to invariant above ptr is valid. > >> + Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(ptr)) }) > > > > Why did you change the null check? You should be able to avoid > > changing anything but the last line. > > Changing only the line, it complains: > > 86 | Ok(unsafe { ARef::from_raw(page) }) > | -------------- ^^^^ expected `NonNull`, > found `NonNull` > > Unless this is what you mean? > > let page =3D unsafe { bindings::alloc_pages(flags.as_raw(), 0) }= ; > let page =3D page.cast::(); > let page =3D NonNull::new(page).ok_or(AllocError)?; > Ok(unsafe { ARef::from_raw(page) }) You can put the cast here: let page =3D unsafe { bindings::alloc_pages(flags.as_raw(), 0) }; let page =3D NonNull::new(page).ok_or(AllocError)?; Ok(unsafe { ARef::from_raw(page.cast()) }) > But what if alloc_pages returns null in the place? Would that be a valid > cast still? The cast is correct both before and after the null check. The above code returns Err(AllocError) when the pointer is null. Alice