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 722F7EDE9B4 for ; Tue, 10 Sep 2024 20:12:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 056198D00BB; Tue, 10 Sep 2024 16:12:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F20B08D0056; Tue, 10 Sep 2024 16:12:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DC15E8D00BB; Tue, 10 Sep 2024 16:12:36 -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 BAF608D0056 for ; Tue, 10 Sep 2024 16:12:36 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 5FCD21C48C2 for ; Tue, 10 Sep 2024 20:12:36 +0000 (UTC) X-FDA: 82549926312.24.E73B240 Received: from mail-40131.protonmail.ch (mail-40131.protonmail.ch [185.70.40.131]) by imf27.hostedemail.com (Postfix) with ESMTP id 52F0040004 for ; Tue, 10 Sep 2024 20:12:34 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=VS81JBGK; dmarc=pass (policy=quarantine) header.from=proton.me; spf=pass (imf27.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=1725999150; a=rsa-sha256; cv=none; b=LFlv+J9xg61rdF7Ayz1rCY56Ngz9JeF/p5s/N7sDjB68wAhRd+LS27toiJqicSsD3zND5L b6RnvLVZWhMRCnl+07EY3E6f5Hm08pKIQrEP4P3Zz1FBewUlaDif+JeIxiXQluzj5JwRTe 5N/LvBYmYdpPuVluJbNW9kePVrQJWxw= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=VS81JBGK; dmarc=pass (policy=quarantine) header.from=proton.me; spf=pass (imf27.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=1725999150; 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=U7TvNmIC8oYr76NVImDfIu1YBXLZLymi+/XLCAXDtOY=; b=MzveX3FCi8Tari3gQwBYue01UKPY4HQYuz0TrbhwevmKCq/6YREyU0geH07ShGCumXtrDJ MFu0YS4+r1srF216uyaO1C8+SmzN5PEfOye7+Po8CacqLttP3/UMX8m9UIbVvPDcxEvuug Nrfdn0NPv1cF02n0aXQpMFmu08lO8D8= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1725999151; x=1726258351; bh=U7TvNmIC8oYr76NVImDfIu1YBXLZLymi+/XLCAXDtOY=; 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=VS81JBGKA2mN7j8bQElk8fOfzC9fDcZxlStBjbVZYnxsl7TWbk4PN04noUNPcqMX0 FhctSlK3OLOTxPJTb7skC646auw+rjrcD3kPh2+QhTa4O+dkFkxgg6Sy4gnnuSVvMk vG6InoYUBHGGjp6F9pJuV5RV2+xV9G4PfskxW1EQweXYAhncuIIgxjNFfZbKrzryxD oky75NVa1vNDmoevRK7YElABRXvj0bbO/FKHPkl0mBA2POy9GtHU7egizmWkJPPegw yFvI5yiUqgsTi08SaAOAHeMgXYnaG0BWvztDohuk6xtGymJFcTeFi4uoQW4WxNgW1G squtgGPu7zcEg== Date: Tue, 10 Sep 2024 20:12:24 +0000 To: Danilo Krummrich , 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 From: Benno Lossin Cc: 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 15/26] rust: alloc: implement `collect` for `IntoIter` Message-ID: <747b8c1c-cb7b-422e-b6a0-ea863cc37f0a@proton.me> In-Reply-To: <20240816001216.26575-16-dakr@kernel.org> References: <20240816001216.26575-1-dakr@kernel.org> <20240816001216.26575-16-dakr@kernel.org> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: db6c5e1305b658d7c7974deb81cc2f1735bd099d MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: uzgqsbuw431uyw96p9i1b6bh5fs73j54 X-Rspamd-Queue-Id: 52F0040004 X-Rspamd-Server: rspam02 X-HE-Tag: 1725999154-706087 X-HE-Meta: U2FsdGVkX18CYJ1fy3NOIBX6j13JDV5eruM9RqANaz3KvwwR06t6/P+88pQmDIRehHa131/EkOP24U+4vHowK+YDcDNP9KKik1fAeqTXd3IL0KULa0l8Ng6QKTlxpRG4dhgSVx7xLyd1ZVQF5sVn1XAX6ZMw101QAk06s/pdnqyPKHpDVicQUu+97wnsru7tMAuRh7a9OHdWTM33JKNaNe+KEDKNVN3b3PSGu7NY2XJGRRi/xv/g0NmCxWxBduxviwdM82n5kqErx2JaeKu92o7DbdLSwZ7/oZQEnIDXKwz2FbbyHgrVeEsdinVV4r46YWNUvXBwqfvp5C1DajBEt+PigVOGZjfp9oaiUg//HhT6e5OCTUUviZDRz9C8bFeAUTtoQCfkzyqbLW0c5qbTdTj4AAqyQ9BFUSq4oJypFBNPf1MQds5jctQ+YXCUyZxe/waHzDiCzXWWXhxPEKHxGWSfzGL6PZR0vEcw6ykW+kI1nkqwjCT/liKkuSZIKFS+ZrdwxcSiLernBfJVNU5JA53cBvIK0UyFa/XqOJIcprp/tNZHocos1nZ7/oAxDV+zCdps/yo0/uEkBNC2ZM7OM1T5YcJZCvjQVP+dt6oFu4dZy+69gBTwUrl4Gsxobqjw9hAI0rzw1jZSZTe1BPNlFeLjeqFDyEp3JZfaWkpLMRQIYG4pdvuCV8rFK1zJd9OWS4t5IZuNcRJdu5M6pRqxDx2mUscBVt81NOFJDeyU8k6bgWMceC58QaurrhdzSoVYhkHWV8K0cJewWpRcqCP5T7bY0rICKcZ5Fjht7ZIlO8XJYCEMBcWl1VL5dRlY/wYZLoeUAfO260jvml0U4YC+4w/WvvXSF3ioD/oAPRIpigHkL0axzlanhpfFggz3AdRdWjWgEf4YhMgElaxdUu3jLx4MQAvQNfCCJv5SyjKUGLsFCvYD8JbhRlem6M5MKkljtJ1VUCkHWPE8iNP92k6 YaLsO+K1 qcVxqiHXGfgCA91OLjamKxVujzb3btUvzNuFmw/yLhwoQgp8rN4yfOx7XwR0OV/ALm1n3qszY9GVuhDExf+EsG9T9+TD1nqzuoHQb6nOqRaAaawrg+mspjfKx4zbsvQVDCxfMAfTekr/FoZaM9yOZADR5bJPMBLtQNUL9Z+GdeGIVWo6Iuw2+4RIdWPutdPBpYZIh8+/ut/Chmi9aJ24FakG6QdD81xJzzyryS6RyyylBu3f+C7wLJ3JxgLBXiUhpSZMDFJ+HvtAPJgww74q3b/K8Btdd9IkuBDkvOfz+E8LX9WSkdWYvk1hUx7w+E+uQUkH7ltq8Q59PI08Rzbq3WcMfPXcm7wSDIfiiAXvIA5cPVljjHU14xyZoayP3XqJOapNDN5RzW3HBlHMIpFtBI46q8GaSBLKVVz3fjRLqSFbwrxMjA799Ls278p87wLPO572ReEA/i0fq6ydRL3+ccEWH59LaRYpcw2hEY//aQssXqinmpnQiYzpiywJMqaGkmM5sUZVEsB3XRQY= 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 16.08.24 02:10, Danilo Krummrich wrote: > Currently, we can't implement `FromIterator`. There are a couple of > issues with this trait in the kernel, namely: >=20 > - Rust's specialization feature is unstable. This prevents us to > optimze for the special case where `I::IntoIter` equals `Vec`'s > `IntoIter` type. > - We also can't use `I::IntoIter`'s type ID either to work around this, > since `FromIterator` doesn't require this type to be `'static`. > - `FromIterator::from_iter` does return `Self` instead of > `Result`, hence we can't properly handle allocation > failures. > - Neither `Iterator::collect` nor `FromIterator::from_iter` can handle > additional allocation flags. >=20 > Instead, provide `IntoIter::collect`, such that we can at least convert > `IntoIter` into a `Vec` again. >=20 > Reviewed-by: Alice Ryhl > Signed-off-by: Danilo Krummrich > --- > rust/kernel/alloc/kvec.rs | 78 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 78 insertions(+) >=20 > diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs > index 3b79f977b65e..ad96f4c3af9e 100644 > --- a/rust/kernel/alloc/kvec.rs > +++ b/rust/kernel/alloc/kvec.rs > @@ -681,6 +681,84 @@ impl IntoIter > fn as_raw_mut_slice(&mut self) -> *mut [T] { > ptr::slice_from_raw_parts_mut(self.ptr, self.len) > } > + > + fn into_raw_parts(self) -> (*mut T, NonNull, usize, usize) { > + let me =3D ManuallyDrop::new(self); > + let ptr =3D me.ptr; > + let buf =3D me.buf; > + let len =3D me.len; > + let cap =3D me.cap; > + (ptr, buf, len, cap) > + } > + > + /// Same as `Iterator::collect` but specialized for `Vec`'s `IntoIte= r`. > + /// > + /// Currently, we can't implement `FromIterator`. There are a couple= of issues with this trait > + /// in the kernel, namely: > + /// > + /// - Rust's specialization feature is unstable. This prevents us to= optimze for the special > + /// case where `I::IntoIter` equals `Vec`'s `IntoIter` type. > + /// - We also can't use `I::IntoIter`'s type ID either to work aroun= d this, since `FromIterator` > + /// doesn't require this type to be `'static`. > + /// - `FromIterator::from_iter` does return `Self` instead of `Resul= t`, hence > + /// we can't properly handle allocation failures. > + /// - Neither `Iterator::collect` nor `FromIterator::from_iter` can = handle additional allocation > + /// flags. > + /// > + /// Instead, provide `IntoIter::collect`, such that we can at least = convert a `IntoIter` into a > + /// `Vec` again. I think it's great that you include this in the code, but I don't think that it should be visible in the documentation, can you move it under the `Examples` section and turn it into normal comments? > + /// > + /// Note that `IntoIter::collect` doesn't require `Flags`, since it = re-uses the existing backing > + /// buffer. However, this backing buffer may be shrunk to the actual= count of elements. > + /// > + /// # Examples > + /// > + /// ``` > + /// let v =3D kernel::kvec![1, 2, 3]?; > + /// let mut it =3D v.into_iter(); > + /// > + /// assert_eq!(it.next(), Some(1)); > + /// > + /// let v =3D it.collect(GFP_KERNEL); > + /// assert_eq!(v, [2, 3]); > + /// > + /// # Ok::<(), Error>(()) > + /// ``` > + pub fn collect(self, flags: Flags) -> Vec { > + let (mut ptr, buf, len, mut cap) =3D self.into_raw_parts(); > + let has_advanced =3D ptr !=3D buf.as_ptr(); > + > + if has_advanced { > + // SAFETY: Copy the contents we have advanced to at the begi= nning of the buffer. This first sentence should not be part of the SAFETY comment. > + // `ptr` is guaranteed to be between `buf` and `buf.add(cap)= ` and `ptr.add(len)` is > + // guaranteed to be smaller than `buf.add(cap)`. This doesn't justify all the requirements documented in [1]. [1]: https://doc.rust-lang.org/core/ptr/fn.copy.html#safety > + unsafe { ptr::copy(ptr, buf.as_ptr(), len) }; > + ptr =3D buf.as_ptr(); > + } > + > + // This can never fail, `len` is guaranteed to be smaller than `= cap`. > + let layout =3D core::alloc::Layout::array::(len).unwrap(); > + > + // SAFETY: `buf` points to the start of the backing buffer and `= len` is guaranteed to be > + // smaller than `cap`. Depending on `alloc` this operation may s= hrink the buffer or leaves > + // it as it is. > + ptr =3D match unsafe { A::realloc(Some(buf.cast()), layout, flag= s) } { > + // If we fail to shrink, which likely can't even happen, con= tinue with the existing > + // buffer. > + Err(_) =3D> ptr, > + Ok(ptr) =3D> { > + cap =3D len; > + ptr.as_ptr().cast() > + } > + }; > + > + // SAFETY: If the iterator has been advanced, the advanced eleme= nts have been copied to > + // the beginning of the buffer and `len` has been adjusted accor= dingly. `ptr` is guaranteed > + // to point to the start of the backing buffer. `cap` is either = the original capacity or, > + // after shrinking the buffer, equal to `len`. `alloc` is guaran= teed to be unchanged since > + // `into_iter` has been called on the original `Vec`. Turn this into bullet points please. --- Cheers, Benno > + unsafe { Vec::from_raw_parts(ptr, len, cap) } > + } > } >=20 > impl Iterator for IntoIter > -- > 2.46.0 >=20