linux-kernel - Re: [PATCH v3 07/14] drm/gem/shmem: Extract drm_gem_shmem_init() from drm_gem_shmem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <891BE2DC-6B2D-409D-970C-F57BA7D86598@collabora.com>
Date: Thu, 4 Sep 2025 10:29:59 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Lyude Paul <lyude@...hat.com>
Cc: dri-devel@...ts.freedesktop.org,
 rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>,
 Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>,
 Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <lossin@...nel.org>,
 Andreas Hindborg <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>,
 Danilo Krummrich <dakr@...nel.org>
Subject: Re: [PATCH v3 07/14] drm/gem/shmem: Extract drm_gem_shmem_init() from
 drm_gem_shmem_create()

Hi Lyude,

Did something happen here? All the indentation seems off.

It looks ok when applied though.

> On 29 Aug 2025, at 19:35, Lyude Paul <lyude@...hat.com> wrote:
> 
> With gem objects in rust, the most ideal way for us to be able to handle
> gem shmem object creation is to be able to handle the memory allocation of
> a gem object ourselves - and then have the DRM gem shmem helpers initialize
> the object we've allocated afterwards. So, let's spit out
> drm_gem_shmem_init() from drm_gem_shmem_create() to allow for doing this.
> 
> Signed-off-by: Lyude Paul <lyude@...hat.com>
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 75 +++++++++++++++++---------
> include/drm/drm_gem_shmem_helper.h     |  1 +
> 2 files changed, 51 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 5d1349c34afd3..b20a7b75c7228 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -48,28 +48,12 @@ static const struct drm_gem_object_funcs drm_gem_shmem_funcs = {
> .vm_ops = &drm_gem_shmem_vm_ops,
> };
> 
> -static struct drm_gem_shmem_object *
> -__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
> -       struct vfsmount *gemfs)
> +static int __drm_gem_shmem_init(struct drm_device *dev, struct drm_gem_shmem_object *shmem,
> + size_t size, bool private, struct vfsmount *gemfs)
> {
> - struct drm_gem_shmem_object *shmem;
> - struct drm_gem_object *obj;
> + struct drm_gem_object *obj = &shmem->base;
> int ret = 0;
> 
> - size = PAGE_ALIGN(size);
> -
> - if (dev->driver->gem_create_object) {
> - obj = dev->driver->gem_create_object(dev, size);
> - if (IS_ERR(obj))
> - return ERR_CAST(obj);
> - shmem = to_drm_gem_shmem_obj(obj);
> - } else {
> - shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
> - if (!shmem)
> - return ERR_PTR(-ENOMEM);
> - obj = &shmem->base;
> - }
> -
> if (!obj->funcs)
> obj->funcs = &drm_gem_shmem_funcs;
> 
> @@ -81,7 +65,7 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
> }
> if (ret) {
> drm_gem_private_object_fini(obj);
> - goto err_free;
> + return ret;
> }
> 
> ret = drm_gem_create_mmap_offset(obj);
> @@ -102,14 +86,55 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
>     __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
> }
> 
> - return shmem;
> -
> + return 0;
> err_release:
> drm_gem_object_release(obj);
> -err_free:
> - kfree(obj);
> + return ret;
> +}
> 
> - return ERR_PTR(ret);
> +/**
> + * drm_gem_shmem_init - Initialize an allocated object.
> + * @dev: DRM device
> + * @obj: The allocated shmem GEM object.
> + *
> + * Returns:
> + * 0 on success, or a negative error code on failure.
> + */
> +int drm_gem_shmem_init(struct drm_device *dev, struct drm_gem_shmem_object *shmem, size_t size)
> +{
> + return __drm_gem_shmem_init(dev, shmem, size, false, NULL);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_shmem_init);
> +
> +static struct drm_gem_shmem_object *
> +__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
> +       struct vfsmount *gemfs)
> +{
> + struct drm_gem_shmem_object *shmem;
> + struct drm_gem_object *obj;
> + int ret = 0;
> +
> + size = PAGE_ALIGN(size);
> +
> + if (dev->driver->gem_create_object) {
> + obj = dev->driver->gem_create_object(dev, size);
> + if (IS_ERR(obj))
> + return ERR_CAST(obj);
> + shmem = to_drm_gem_shmem_obj(obj);
> + } else {
> + shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
> + if (!shmem)
> + return ERR_PTR(-ENOMEM);
> + obj = &shmem->base;
> + }
> +
> + ret = __drm_gem_shmem_init(dev, shmem, size, private, gemfs);
> + if (ret) {
> + kfree(obj);
> + return ERR_PTR(ret);
> + }
> +
> + return shmem;
> }
> /**
>  * drm_gem_shmem_create - Allocate an object with the given size
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index 92f5db84b9c22..235dc33127b9a 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -107,6 +107,7 @@ struct drm_gem_shmem_object {
> #define to_drm_gem_shmem_obj(obj) \
> container_of(obj, struct drm_gem_shmem_object, base)
> 
> +int drm_gem_shmem_init(struct drm_device *dev, struct drm_gem_shmem_object *shmem, size_t size);
> struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size);
> struct drm_gem_shmem_object *drm_gem_shmem_create_with_mnt(struct drm_device *dev,
>   size_t size,
> -- 
> 2.50.0
> 
> 

Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>