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 D560FC52D7C for ; Thu, 15 Aug 2024 14:19:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 695FB6B010D; Thu, 15 Aug 2024 10:19:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 645FE6B010E; Thu, 15 Aug 2024 10:19:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 534F06B010F; Thu, 15 Aug 2024 10:19:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 30A2D6B010D for ; Thu, 15 Aug 2024 10:19:50 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 79247A8F9D for ; Thu, 15 Aug 2024 14:19:49 +0000 (UTC) X-FDA: 82454688498.24.5F81F8A Received: from mail-40133.protonmail.ch (mail-40133.protonmail.ch [185.70.40.133]) by imf19.hostedemail.com (Postfix) with ESMTP id 695061A001D for ; Thu, 15 Aug 2024 14:19:47 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=proton.me header.s=gxr7zbezencsjmbcyyqdpmg2aa.protonmail header.b=aRv6qiIY; spf=pass (imf19.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.133 as permitted sender) smtp.mailfrom=benno.lossin@proton.me; dmarc=pass (policy=quarantine) header.from=proton.me ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723731515; 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=UqgtoJLouKx1iQTjqXRDGDK4zBAgDS774E+gg2Yq3mQ=; b=TLXOS4WjJcBPTDumQavONwgy6qrCU8blv9evGGzQlwqXdAQch0XcwxWP5kYgEymfdTO0w3 JVs/POV7N9ZSGIAThManbuoGHWXPpmGmrLJF3ETEWtzoVwAyFU0QA7JYDr2DRGR7EZy//X 2dDeQAQRXiJiEz7Z0ZwTuvHf1BxQM+E= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723731515; a=rsa-sha256; cv=none; b=KU/R9sRNhxKZps39MMPgOirY6umd/hK7TodXG9ziLFKv8ZX+W4hvQEnbCOqzGMT42gsgEm xQBbuNCgc3TMgLaOOXfr1BBAOMmoLK9mzngGOp+rMUStSwktnboASbRZqAzXNl0C6xNPWV kqRUsJ/Y5UbFVsUHlYB5OhbTg6ZLkX0= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=proton.me header.s=gxr7zbezencsjmbcyyqdpmg2aa.protonmail header.b=aRv6qiIY; spf=pass (imf19.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.133 as permitted sender) smtp.mailfrom=benno.lossin@proton.me; dmarc=pass (policy=quarantine) header.from=proton.me DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=gxr7zbezencsjmbcyyqdpmg2aa.protonmail; t=1723731585; x=1723990785; bh=UqgtoJLouKx1iQTjqXRDGDK4zBAgDS774E+gg2Yq3mQ=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=aRv6qiIYOKxr6XQFzLh4lyWHeVOo4C4FkfR4mNcwxlhMyOmZ03dM01iXRSvMuJQ0i EVEbDCaeps7o38gDrXZXa1BaBKFK9a7L/zCbCrFF5cm7d6YahkSFTl2K7ZPGF0jjrA aa1UcTRdMc9MnPg/wKbngMeCYDZ2xEOQdaB8tfhaaCX62am2OPE6gxNyZQaqKOyXmQ LWWf/x1uVSsXzMcyHX8te1qV6ecb4XycAl/1C3p4bzBW8FEHvUlGs1nFe/3gIPjK5T gTH9bpon1WYrdtF3Ve5wop6a6naCuybDJTGYIAAFSdlUesHuYdaLIyISddKHeQuoTa gwszo8yUJzyYA== Date: Thu, 15 Aug 2024 14:19:41 +0000 To: Danilo Krummrich From: Benno Lossin Cc: Alice Ryhl , Boqun Feng , ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, a.hindborg@samsung.com, akpm@linux-foundation.org, daniel.almeida@collabora.com, faith.ekstrand@collabora.com, boris.brezillon@collabora.com, lina@asahilina.net, mcanal@igalia.com, zhiw@nvidia.com, cjia@nvidia.com, jhubbard@nvidia.com, airlied@redhat.com, ajanulgu@redhat.com, lyude@redhat.com, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v5 00/26] Generic `Allocator` support for Rust Message-ID: <62e9af7b-df5a-495b-8fb5-27cf852293b4@proton.me> In-Reply-To: References: <20240812182355.11641-1-dakr@kernel.org> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: 82454498d143298b0af31e2a5c089cc31de5f112 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 695061A001D X-Stat-Signature: cmitugt3zfwucsow1o3ooocqsid5iq6x X-HE-Tag: 1723731587-285286 X-HE-Meta: U2FsdGVkX1/Q5U+1IobFwV9Jp1VQJd3XcmzjsDrhmbhwlHSM16XsCy6Kesl7Wg/hEQf9i76VCyRgPI7WyMvMCGbolV4R+nYjU6Wt930BN7lZJYcNwDMrKScwjJTWVnGQpJKtTMm6I3U/QykINB4GSqkMpfTUXB58S2g9ZndNReiGyz7alASg/a0rCKtjtPguU8SkIw4DXiY8jfG9JgqNHa34lC2uXGohVZJ/ssbVd+C7zoNS0sV8kVtFz+5y/RQ1FMmao1XBrskAr7expBGn7YhRvNfFBP/3pX3uEfaNhxDD4Yw3OFKfaV8/Z1003BVve855lXlV3zw+Mj4Wf/xXuUVUlCRO71G8S2ODeaBq7KTS57rGyX5ZIawkOHfj8qlCT1Box3DTVpmR0hdme6fVynBb4IoZACks9pfLFzETx3MozPmGSqlTQ7BOiMih0Z+WJZcFBnx+4XIDfNbo5YZl4cCdMiYml84okgCUjJFv4OYRKkjpLhI4i8TE2wPUAQ+VOpmIP3gvKsp2I9XnumhcnL0rX9PrV5Rvq9FlHiiYXJBkU7W8fVowXEyliivnl1a/9gqGbLv0OVORKcMWd62FIc3jRSMKuI2LD5oSwWn4aA+hTYijJXJFgahnCW0LBE7nLNvVJ0H7vLB+oI/p7fp1oCMP8LXh4nhpd0Bx2cdy5qFrmkKoaky5QiRVSOxfYvCJclai7Y9wPWCEW2Psy2IAF+lNNElufQ3bq0yeSOH5PcYitUc7KBaFHo0nHatUowcWSUohvQkHgtV+Azxyu5FDG1g6wcpFO5bfdA0FiAJ3OF1jnnzuDXaOirI0+2mUMxD0UcM/M8QNm9tme+3VrE91x0nsVBGU3IYqhNZuL2A8Pftw0S+SFyeleXuJNMaDHnJhoKqknp887KTHIYjMc5awj/dmSgdtuvD+Mb4RfaX7D59GmChJ9jCY7v6cG5b0N9FzteINdcADjHqQCZkGuCY uoc1VVoV eNKElbUSkCH/B38OGyrz69ae2BCmE7H4lOYtpcoCsDMgHbyk288YkDsNdkIuU2i7OcrgyX9ERceJu9d+ugBz57f1mwyntUCmQX/mpi0naFNMfNzfE4uOSFoR6XDlvLnmKmphi9TWi86ZGZt6Ct3Rj2vajPDeCelfvs5zRO2tyJ2UXOdGkfcZ3c+GJsLCBCG1/SpCp4MVOvC64GWouMc1qJFdM5Mfgj4Iw+dzMlZSER4jyJw7xhq8/FByCF6k1YNCztZTu52pO303NU6CDV1Z+JMlrM3daMNq2qKbSoBXhDE1zedK2xj2xc9xgSvsaMBHbCxjVWZV/xniBjBdi31UgbzMWC5BYuhMET7KUJ0aNGDg1xFQ+1RKdgxfjx5UYb6uvVrfPKT6WEDZ93aJjd33Kldz4Nm7FwLd/1bftKKOc8ciksHwMBdjmSWnAwKgYfNMaMzIp650AyC6Izss= X-Bogosity: Ham, tests=bogofilter, spamicity=0.006548, 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 15.08.24 16:09, Danilo Krummrich wrote: > On Thu, Aug 15, 2024 at 01:39:05PM +0000, Benno Lossin wrote: >> On 15.08.24 15:33, Danilo Krummrich wrote: >>> On Thu, Aug 15, 2024 at 02:34:50PM +0200, Alice Ryhl wrote: >>>> On Thu, Aug 15, 2024 at 2:33=E2=80=AFPM Danilo Krummrich wrote: >>>>> >>>>> On Thu, Aug 15, 2024 at 11:20:32AM +0200, Alice Ryhl wrote: >>>>>> On Thu, Aug 15, 2024 at 4:52=E2=80=AFAM Danilo Krummrich wrote: >>>>>>> >>>>>>> On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: >>>>>>>> Hi Danilo, >>>>>>>> >>>>>>>> I'm trying to put your series on rust-dev, but I hit a few conflic= ts due >>>>>>>> to the conflict with `Box::drop_contents`, which has been in rust-= dev >>>>>>>> for a while. And the conflict is not that trivial for me to resolv= e. >>>>>>>> So just a head-up, that's a requirement for me to put it on rust-d= ev for >>>>>>>> more tests from my end ;-) >>>>>>> >>>>>>> I rebased everything and you can fetch them from [1]. >>>>>>> >>>>>>> I resolved the following conflicts: >>>>>>> >>>>>>> - for `Box`, implement >>>>>>> - `drop_contents` >>>>>>> - `manually_drop_contents` [2] >>>>>> >>>>>> Not sure I like this name. It sounds like something that runs the >>>>>> destructor, but it does the exact opposite. >>>>> >>>>> I thought it kinda makes sense, since it's analogous to `ManuallyDrop= ::new`. >>>>> >>>>> What about `Box::forget_contents` instead? >>>> >>>> One option is `into_manually_drop`. This uses the convention of using >>>> the `into_*` prefix for conversions that take ownership of the >>>> original value. >>> >>> The signature of the current `Box::manually_drop_contents` is the same = as for >>> `Box::drop_contents`, namely >>> `fn manually_drop_contents(this: Self) -> Box, A>`. >>> >>> `into_manually_drop` seems misleading for for returning a >>> `Box, A>`. >>> >>> I still think `forget_contents` hits it quite well. Just as `drop_conte= nts` >>> drops the value, `forget_contents` makes the `Box` forget the value. >> >> I think `forget_contents` sounds good. Can you please add some more docs >> to that function though? Like an example and change "Manually drops the >> contents, but keeps the allocation" to "Forgets the contents (does not >> run the destructor), but keeps the allocation.". >=20 > I can't really think of a good real world example other than moving out o= f a > `Box`, can you? Otherwise, maybe it just shouldn't be public? Oh I thought you had a user for that function. In that case making it private makes a lot of sense. Also, `drop_contents` can be implemented with `forget_contents`, but that might be a good follow up as a good-first-issue. >> Another thing that I spotted while looking at the patch, `move_out` >> doesn't need the `transmute_copy`, you should be able to just call >> `read` on the pointer. >=20 > While technically it's the same I thought `transmute_copy` is considered = better, > since it has less stict safety requirements? I would avoid `transmute_copy` as much as possible, transmuting signifies to me that you somehow need to change the type and `transmute_copy` means that the compiler isn't even able to figure out that the two types have the same size (they are allowed to have different size [only `Src` larger than `Dst` though], but I most often have used this with generics where it was guaranteed to be the same type, but the compiler failed to notice it.). > For `transmute_copy` we only need to say, that dst has the same type as s= rc, > whereas for `read` the pointer must be valid for reads, properly aligned = and > point to an initialized value. You can input a reference, so you get pointer validity for free, though you still need to put that in the SAFETY comment. --- Cheers, Benno