From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-244106.protonmail.ch (mail-244106.protonmail.ch [109.224.244.106]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 841AB33261F; Thu, 23 Apr 2026 17:15:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=109.224.244.106 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776964534; cv=none; b=X3f3bWluOnJCNOL6y868BtOBRWHblhP9uswIhdZu5u7PeREBklrAzubyTZCB2qvWiCT2jBsWftrO0wTwOisDTXsEyBym0HzUlSFkh4LOp0muYsn79u4sbnZoYa9q+d7cMU3m1DtnPt7bmvnbabFMM+BUyLxpSfz0D2xOe3gub1Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776964534; c=relaxed/simple; bh=hPmv7xfwZL+EcheokgSlaGX4OLAxZAr2E7so3oVQDWw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=m/mSZ5AYKaGxDzsZWJUXU+KCYTsL6L+rMnZ/O1prm5c8feFdlEADNcIsFw/eQYM0xhwVwMOvVkshdhtBCkjZWZyMceu++9cx/FMr1JPIT5e3vtfir5mKeN+xGHbwLa9Cwl091gcDSkvvAlfVuE0uMvCruYrB6doVqkxS1rW/SGA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=onurozkan.dev; spf=pass smtp.mailfrom=onurozkan.dev; dkim=pass (2048-bit key) header.d=onurozkan.dev header.i=@onurozkan.dev header.b=KSKR6b7m; arc=none smtp.client-ip=109.224.244.106 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=onurozkan.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=onurozkan.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=onurozkan.dev header.i=@onurozkan.dev header.b="KSKR6b7m" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=onurozkan.dev; s=protonmail; t=1776964523; x=1777223723; bh=Z565GjO7YrtU4h57BjOTttW9CeupQn/HHRIOQjJ7HFc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:From:To: Cc:Date:Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector; b=KSKR6b7mua/xIbWJq33WEVr1PyHiTJJCvkhk1K2oQygPye2G/eRBaghdQEorAElrb 06VvHzh2+EmV7jZ7+srNlgz/ImJKZEK7UL2QtX4B6uXll6KvGZeFZ0btbIj58879kJ TpVh27NYsJWD6cWoJCsrqHbQiBnJZEHtl3xT7y9dxgssAarjcuTHsW+rG+1SpCDMqH g+oWjvIOt6Ak/1QTzpQzBW6EWZPa/pWHvi72jmxk2do5AxGGeEDGaQdbu0I+PaD9u0 Is5MqrNy7x1duQTdTr2VuYwHCqbPaCeJ8QuWV8p9FTyBLN4qbQQyEGLfqG1hxNGjrm 6CU1vFULIQkRg== X-Pm-Submission-Id: 4g1jQb4w2Dz1DDs5 From: =?UTF-8?q?Onur=20=C3=96zkan?= To: Eliot Courtney Cc: Danilo Krummrich , Alice Ryhl , David Airlie , Simona Vetter , Miguel Ojeda , Boqun Feng , Gary Guo , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Trevor Gross , Alexandre Courbot , John Hubbard , Alistair Popple , Joel Fernandes , Timur Tabi , dri-devel@lists.freedesktop.org, rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, =?UTF-8?q?Onur=20=C3=96zkan?= Subject: Re: [PATCH] rust: drm: gem: clean up GEM state in init failure case Date: Thu, 23 Apr 2026 20:15:17 +0300 Message-ID: <20260423171518.24896-1-work@onurozkan.dev> X-Mailer: git-send-email 2.51.2 In-Reply-To: <20260423-fix-gem-1-v1-1-e12e35f7bba9@nvidia.com> References: <20260423-fix-gem-1-v1-1-e12e35f7bba9@nvidia.com> Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Thu, 23 Apr 2026 21:36:52 +0900=0D Eliot Courtney wrote:=0D =0D > Currently, if `drm_gem_object_init` fails, the object is freed without=0D > any cleanup. Perform the cleanup in that case.=0D > =0D > Signed-off-by: Eliot Courtney =0D > ---=0D > I looked at `drm_gem_shmem_init` for an example, and it seems like the=0D > correct cleanup here is to do `drm_gem_private_object_fini` if=0D > `drm_gem_object_init` fails. Other C drivers do different things, but=0D > looking at the implementation of `drm_gem_object_init`, this looks like=0D > the only thing we need to clean up.=0D > ---=0D > rust/kernel/drm/gem/mod.rs | 9 ++++++++-=0D > 1 file changed, 8 insertions(+), 1 deletion(-)=0D > =0D > diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs=0D > index 75acda7ba500..7b6a085ace27 100644=0D > --- a/rust/kernel/drm/gem/mod.rs=0D > +++ b/rust/kernel/drm/gem/mod.rs=0D > @@ -278,7 +278,14 @@ pub fn new(dev: &drm::Device, size: usize= , args: T::Args) -> Result unsafe { (*obj.as_raw()).funcs =3D &Self::OBJECT_FUNCS };=0D > =0D > // SAFETY: The arguments are all valid per the type invariants.= =0D > - to_result(unsafe { bindings::drm_gem_object_init(dev.as_raw(), o= bj.obj.get(), size) })?;=0D > + if let Err(err) =3D=0D > + to_result(unsafe { bindings::drm_gem_object_init(dev.as_raw(= ), obj.obj.get(), size) })=0D > + {=0D > + // SAFETY: `drm_gem_object_init()` initializes the private G= EM object state before=0D > + // failing, so `drm_gem_private_object_fini()` is the matchi= ng cleanup.=0D > + unsafe { bindings::drm_gem_private_object_fini(obj.obj.get()= ) };=0D > + return Err(err);=0D > + }=0D =0D I can confirm that this matches the usage on the C side. I think the API co= uld=0D be improved to avoid relying on developers remembering this pattern. I don'= t=0D expect that to happen in this patch, just saying it :).=0D =0D Reviewed-by: Onur =C3=96zkan =0D =0D > =0D > // SAFETY: We will never move out of `Self` as `ARef` is a= lways treated as pinned.=0D > let ptr =3D KBox::into_raw(unsafe { Pin::into_inner_unchecked(ob= j) });=0D > =0D > ---=0D > base-commit: a7a080bb4236ebe577b6776d940d1717912ff6dd=0D > change-id: 20260423-fix-gem-1-6c68b9fa0972=0D > =0D > Best regards,=0D > -- =0D > Eliot Courtney =0D > =0D