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 C16DFCA0FEB for ; Fri, 30 Aug 2024 12:56:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 566616B013A; Fri, 30 Aug 2024 08:56:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 515066B013B; Fri, 30 Aug 2024 08:56:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4032E6B013C; Fri, 30 Aug 2024 08:56:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 242726B013A for ; Fri, 30 Aug 2024 08:56:54 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id D4EECA1875 for ; Fri, 30 Aug 2024 12:56:53 +0000 (UTC) X-FDA: 82508911506.24.7A6E994 Received: from mail-40131.protonmail.ch (mail-40131.protonmail.ch [185.70.40.131]) by imf07.hostedemail.com (Postfix) with ESMTP id F3F2440016 for ; Fri, 30 Aug 2024 12:56:51 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=hhjWIyCj; dmarc=pass (policy=quarantine) header.from=proton.me; spf=pass (imf07.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.131 as permitted sender) smtp.mailfrom=benno.lossin@proton.me ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725022539; a=rsa-sha256; cv=none; b=QWYl5CayiVN7Igiiyj9Qxi/w6QxcVWSyDehY5HmWAq1d2/HvnYlWSTjc5ue+TTuWm612oi Ld79SM6q6OnH4p1aRh/W0HiUwkqEdvqOjrKn72oDfWrMjxcJq8njtnE5QMk8p0VyF/MQpg soloJV7DfyjsQfL70n1TOzr4MtC3KMw= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=hhjWIyCj; dmarc=pass (policy=quarantine) header.from=proton.me; spf=pass (imf07.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.131 as permitted sender) smtp.mailfrom=benno.lossin@proton.me ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725022539; 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=4+gjy/Hs8b3AqyO8Vm08sJpbUTrKaPC+6nBp0PfO5Oc=; b=um3il/zuDnNxitG6S1/g+ArcZceejSk2glP33633NXtvrVq1AwzF3vO0m0B76HZzvsDea/ zCYEVnv1qCtyvAELTopB3CjBfR5bT9oU6xsXUUGkFXzluy2h59oSKtc2mI2vjphf18OlQe rxJQt+FjSTBh960nqdW4E4SOjtRjVWk= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1725022608; x=1725281808; bh=4+gjy/Hs8b3AqyO8Vm08sJpbUTrKaPC+6nBp0PfO5Oc=; 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=hhjWIyCjxO5t0cmJNVR9isGB2yN8YNFK6cNgoEE1XjHDq43gDW5qvNh7XWkbNEgZ1 04TOx0Uaq399+YlA1nYPBAf3AGUpxsIPQZ2dTS9oNtUTV8hZ5NAbKIVncKBJh7ZxUv dN5jV92FE71SV3f18PZzKtpNZgSkW3/xW4MiAkR8i6pPKSTlE9iOEU6bhZDMs1AOyn r+BcdiNAqI4XIN8Q5BnNFkAxivveeyYttJuoR4dJ2IZ0rf+IrcNE2uj/kSCHKkgRup Xk14xQJhCrOKTdX8jf/IGkJQ7kodOHr67P3Tb1unr2IDFCBWFWRxzdWxrmlZUn0FpP lS3KcweFgdHqg== Date: Fri, 30 Aug 2024 12:56:44 +0000 To: Danilo Krummrich From: Benno Lossin Cc: ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, a.hindborg@samsung.com, aliceryhl@google.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 v6 22/26] rust: alloc: implement `Cmalloc` in module allocator_test Message-ID: In-Reply-To: References: <20240816001216.26575-1-dakr@kernel.org> <20240816001216.26575-23-dakr@kernel.org> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: e94a7edc10b5d6f82b2d1567cb6823eaaf31655d MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: F3F2440016 X-Stat-Signature: g7f8jrrp9i81wteygrgasnxqpuqz9jy7 X-Rspam-User: X-HE-Tag: 1725022611-324384 X-HE-Meta: U2FsdGVkX1/NokttdOgmUQ3wC4hKAwt7S1EtzvRMw5PJyQHz6FVZv35w2P37JYrD2+hVCiNoOtf72YCLGbbiSDc64II8HUFS49Tb8VyLqZetkErvoGYYEQcIr/5Ge6sZ+hFKEp+nKGoavGths+CeKJzCldpd+dWCSWHW6P0KhDKO6K+wC9G57Ld+gykF7URU/YDex70+i4tTBWlAwtd11mA0ggbj24Y8MccYc2EWl4MxrjHOC0xnylbwoxTqOe9SzAGspxcpnTvdmPZKnecn3SDy/5TAdj2KcafL5nXa5bI3g2ODlUp9wzC30zC2HVQReCyuKuNSGr2LAc52VzlFg8Fo2XuGE9nr2tg0bg6uKe9XIAAUAlSXvf6mX2TC8BW+cJDV16UlD27GGpy+pa47u+Q8gYDUGxHB8eoKykRp8z9Aof4jqORLkMVK1hUI+PvKeTxQVvglst4gWXgLSrsMEFNtDd/WLq0cN9jLaQPWkQV03Hy0Bon68+7Ikca37BHXpY7AEv8ugXACjcysPxKJ2+VMOoRhw4ashlGd4h3Yg7WYl3jraw5owb3d+4uptwrtD7KQhkIgV2zGYLrUVviPObhilZ/Az3/Dd7aZ7dTAvO/qowvrd7YIHIHA/4OuWWKMM5vddKhpTFdma5Uwp82iN25sc3I4Takm/gLHtx7flbZk0xZUltKC2g4OUv5tloWxyC0yCLtku2PTlySTTb1gv140856wNCCk4W2GyMpoEIk9dBr+O5WVpbW3VFWrvLFv8Xe0vPTQiV1UoloiytFN1UFwnYGij6M2VEXqlMvLeE6gP9M7n8Re0F/jMbUMmjOeiOL5AR+BSDlJgY1SHb/xH5H0Iu8zZQPhM1ktbvafFY94+yFGtSFWpgQZGCNrAs6tedKlJG0I6tyXAgboDNOIUbXCfo3w0+vFfo0lCflJzuH1FrS0K6ifHGfFoFmpAK9BJdGKMAR+fiUucuZrx/Y JpeF0K8Y Lf1yUBNl4Frf9CZipIBTHFr7Fpc4iBt8eyiUmrGaLuUFZ2Lr6NgccGzuV5UZhghtBOamhMQISBiHVGhiMTvODICo6yc/lnmQQGRhXhQ56sWPyb8TlVVJeCpcu3MJd1Blf7bRPzYFyjKZ81KB4iB/4aZgRP9ZQyVjhWgaj48nP43pTltYvw+yOdZ4nRqNfWMP+8un6HmovAoy3p+4PvJqtyGbhxLDVvKl7XPmrRBU3P3W9jTmrDbb4SxjBvvoxr+71DugjOaNh6BX2VGqSexxmNJy9/oZCkHoWC8/cooY0x/+4VodMxwEBeBdh2/bIiqW5or1TgvKTxWgeZAm+sZ+bVbMaSA1+vz1El9LTPsPMZXZYnXG2hv6ymtQU93EFsFKbgP9kG+ufMT+zbNoAmkWrQRc9QQ== 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 30.08.24 00:25, Danilo Krummrich wrote: > On Thu, Aug 29, 2024 at 07:14:18PM +0000, Benno Lossin wrote: >> On 16.08.24 02:11, Danilo Krummrich wrote: >>> +impl Cmalloc { >>> + /// Adjust the size and alignment such that we can additionally st= ore `CmallocData` right >>> + /// before the actual data described by `layout`. >>> + /// >>> + /// Example: >>> + /// >>> + /// For `CmallocData` assume an alignment of 8 and a size of 16. >>> + /// For `layout` assume and alignment of 16 and a size of 64. >> >> This looks like you want it rendered as bulletpoints (but it won't). >=20 > Actually, that wasn't my intention, but I'm fine changing that. I see, in that case not putting a newline there is also fine with me. But I think bulletpoints are probably easier to read. >>> + fn alloc_store_data(layout: Layout) -> Result, AllocEr= ror> { >>> + let requested_size =3D layout.size(); >>> + >>> + let layout =3D Self::layout_adjust(layout)?; >>> + let min_align =3D layout.align() / 2; >>> + >>> + // SAFETY: Returns either NULL or a pointer to a memory alloca= tion that satisfies or >>> + // exceeds the given size and alignment requirements. >>> + let raw_ptr =3D unsafe { libc_aligned_alloc(layout.align(), la= yout.size()) } as *mut u8; >>> + >>> + let priv_ptr =3D NonNull::new(raw_ptr).ok_or(AllocError)?; >>> + >>> + // SAFETY: Advance the pointer by `min_align`. The adjustments= from `Self::layout_adjust` >>> + // ensure that after this operation the original size and alig= nment requirements are still >>> + // satisfied or exceeded. >> >> This SAFETY comment should address why it's OK to call `add`. You >> justify something different, namely why the allocation still satisfies >> the requirements of `layout`. That is something that this function >> should probably guarantee. >=20 > So, I guess you're arguing that instead I should say that, we're still wi= thin > the bounds of the same allocated object and don't exceed `isize`? Yes. >>> + unsafe fn free_read_data(ptr: NonNull) { >>> + // SAFETY: `ptr` has been created by `Self::alloc_store_data`. >>> + let data =3D unsafe { Self::data(ptr) }; >>> + >>> + // SAFETY: `ptr` has been created by `Self::alloc_store_data`. >>> + let priv_ptr =3D unsafe { ptr.as_ptr().sub(data.offset) }; >>> + >>> + // SAFETY: `priv_ptr` has previously been allocatored with thi= s `Allocator`. >>> + unsafe { libc_free(priv_ptr.cast()) }; >>> + } >>> +} >>> + >>> +unsafe impl Allocator for Cmalloc { >>> + fn alloc(layout: Layout, flags: Flags) -> Result, Al= locError> { >>> + if layout.size() =3D=3D 0 { >>> + return Ok(NonNull::slice_from_raw_parts(NonNull::dangling(= ), 0)); >>> + } >>> + >>> + let ptr =3D Self::alloc_store_data(layout)?; >>> + >>> + if flags.contains(__GFP_ZERO) { >>> + // SAFETY: `Self::alloc_store_data` guarantees that `ptr` = points to memory of at least >>> + // `layout.size()` bytes. >>> + unsafe { ptr.as_ptr().write_bytes(0, layout.size()) }; >>> + } >> >> This makes me wonder, what other flags should we handle for this >> allocator? >=20 > I don't think there are any other flags that we can handle. The only othe= r one > that'd make sense is __GFP_NOFAIL, but we can't guarantee that. >=20 > If any specific gfp flags are needed, I think it's simply not a candidate= for a > userspace test. >=20 > If we really want to do something here, we could whitelist the flags we i= gnore, > since they do not matter (such as __GFP_NOWARN) and panic() for everythin= g else. >=20 > But I don't think that's really needed. Makes sense, just wanted to check that this has been accounted for. --- Cheers, Benno