On 22.09.25 10:40, Thomas Zimmermann wrote:
> Hi
>
> Am 22.09.25 um 10:34 schrieb Christian König:
>> On 19.09.25 17:54, Melissa Wen wrote:
>>> This reverts commit 5307dce878d4126e1b375587318955bd019c3741.
>>>
>>> We've already reverted all other commits related to dma_bug handling and
>>> there is still something wrong with this approach that does not allow
>>> unloading a driver. By reverting this commit, we'd just go back ot the
>>> old behavior.
>> I don't think we want to do this.
>>
>> Keeping the backing store alive for DMA-bufs while they are used for scanout
>> is actually a really important bug fix.
>
> That bug has rarely seen seen in practice. At least I'm not aware of any such
> report. And it's also just half of the fix IIRC. Not being able to unload the
> module is a regression OTOH. I'd rather go back to the old status quo than
> now having to deal with two problems.
Yeah wait a second, not being able to unload the module is potentially the
right thing to do.
So were is that module reference actually coming from? And why do we have it
now and didn't had it previously?
Regards,
Christian.
>
> Best regards
> Thomas
>
>>
>> Regards,
>> Christian.
>>
>>> Signed-off-by: Melissa Wen <m...@igalia.com>
>>> ---
>>> drivers/gpu/drm/drm_gem.c | 44 ++------------------
>>> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 ++++---
>>> drivers/gpu/drm/drm_internal.h | 2 -
>>> 3 files changed, 11 insertions(+), 51 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>>> index 09f80a84d61a..12efc04fb896 100644
>>> --- a/drivers/gpu/drm/drm_gem.c
>>> +++ b/drivers/gpu/drm/drm_gem.c
>>> @@ -213,35 +213,6 @@ void drm_gem_private_object_fini(struct drm_gem_object
>>> *obj)
>>> }
>>> EXPORT_SYMBOL(drm_gem_private_object_fini);
>>> -static void drm_gem_object_handle_get(struct drm_gem_object *obj)
>>> -{
>>> - struct drm_device *dev = obj->dev;
>>> -
>>> - drm_WARN_ON(dev, !mutex_is_locked(&dev->object_name_lock));
>>> -
>>> - if (obj->handle_count++ == 0)
>>> - drm_gem_object_get(obj);
>>> -}
>>> -
>>> -/**
>>> - * drm_gem_object_handle_get_unlocked - acquire reference on user-space
>>> handles
>>> - * @obj: GEM object
>>> - *
>>> - * Acquires a reference on the GEM buffer object's handle. Required
>>> - * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked()
>>> - * to release the reference.
>>> - */
>>> -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj)
>>> -{
>>> - struct drm_device *dev = obj->dev;
>>> -
>>> - guard(mutex)(&dev->object_name_lock);
>>> -
>>> - drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in
>>> create-tail helper */
>>> - drm_gem_object_handle_get(obj);
>>> -}
>>> -EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked);
>>> -
>>> /**
>>> * drm_gem_object_handle_free - release resources bound to userspace
>>> handles
>>> * @obj: GEM object to clean up.
>>> @@ -272,14 +243,8 @@ static void
>>> drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj)
>>> }
>>> }
>>> -/**
>>> - * drm_gem_object_handle_put_unlocked - releases reference on user-space
>>> handles
>>> - * @obj: GEM object
>>> - *
>>> - * Releases a reference on the GEM buffer object's handle. Possibly
>>> releases
>>> - * the GEM buffer object and associated dma-buf objects.
>>> - */
>>> -void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
>>> +static void
>>> +drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
>>> {
>>> struct drm_device *dev = obj->dev;
>>> bool final = false;
>>> @@ -304,7 +269,6 @@ void drm_gem_object_handle_put_unlocked(struct
>>> drm_gem_object *obj)
>>> if (final)
>>> drm_gem_object_put(obj);
>>> }
>>> -EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked);
>>> /*
>>> * Called at device or object close to release the file's
>>> @@ -434,8 +398,8 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
>>> int ret;
>>> WARN_ON(!mutex_is_locked(&dev->object_name_lock));
>>> -
>>> - drm_gem_object_handle_get(obj);
>>> + if (obj->handle_count++ == 0)
>>> + drm_gem_object_get(obj);
>>> /*
>>> * Get the user-visible handle using idr. Preload and perform
>>> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>>> b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>>> index e364fa36ee36..4bc89d33df59 100644
>>> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>>> @@ -101,7 +101,7 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb)
>>> unsigned int i;
>>> for (i = 0; i < fb->format->num_planes; i++)
>>> - drm_gem_object_handle_put_unlocked(fb->obj[i]);
>>> + drm_gem_object_put(fb->obj[i]);
>>> drm_framebuffer_cleanup(fb);
>>> kfree(fb);
>>> @@ -179,10 +179,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>>> if (!objs[i]) {
>>> drm_dbg_kms(dev, "Failed to lookup GEM object\n");
>>> ret = -ENOENT;
>>> - goto err_gem_object_handle_put_unlocked;
>>> + goto err_gem_object_put;
>>> }
>>> - drm_gem_object_handle_get_unlocked(objs[i]);
>>> - drm_gem_object_put(objs[i]);
>>> min_size = (height - 1) * mode_cmd->pitches[i]
>>> + drm_format_info_min_pitch(info, i, width)
>>> @@ -192,22 +190,22 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>>> drm_dbg_kms(dev,
>>> "GEM object size (%zu) smaller than minimum size (%u)
>>> for plane %d\n",
>>> objs[i]->size, min_size, i);
>>> - drm_gem_object_handle_put_unlocked(objs[i]);
>>> + drm_gem_object_put(objs[i]);
>>> ret = -EINVAL;
>>> - goto err_gem_object_handle_put_unlocked;
>>> + goto err_gem_object_put;
>>> }
>>> }
>>> ret = drm_gem_fb_init(dev, fb, info, mode_cmd, objs, i, funcs);
>>> if (ret)
>>> - goto err_gem_object_handle_put_unlocked;
>>> + goto err_gem_object_put;
>>> return 0;
>>> -err_gem_object_handle_put_unlocked:
>>> +err_gem_object_put:
>>> while (i > 0) {
>>> --i;
>>> - drm_gem_object_handle_put_unlocked(objs[i]);
>>> + drm_gem_object_put(objs[i]);
>>> }
>>> return ret;
>>> }
>>> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
>>> index ec1bf58e5714..5265eac81077 100644
>>> --- a/drivers/gpu/drm/drm_internal.h
>>> +++ b/drivers/gpu/drm/drm_internal.h
>>> @@ -163,8 +163,6 @@ void drm_sysfs_lease_event(struct drm_device *dev);
>>> /* drm_gem.c */
>>> int drm_gem_init(struct drm_device *dev);
>>> -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj);
>>> -void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj);
>>> int drm_gem_handle_create_tail(struct drm_file *file_priv,
>>> struct drm_gem_object *obj,
>>> u32 *handlep);
>
