On 10/3/25 08:34, [email protected] wrote:
> + /* for restoration of objects after hibernation */
> + struct virtio_gpu_object_params params;
> + struct list_head list;
These are bit too generic names for struct members that are supposed to
be used for hibernation-restore only.
what about this variant:
struct virtio_gpu_object {
...
struct virtio_gpu_object_params params;
struct list_head restore_node;
};
> +static void virtio_gpu_object_del_restore_list(struct virtio_gpu_device
> *vgdev,
> + struct virtio_gpu_object *bo)
> +{
> + struct virtio_gpu_object *curr, *tmp;
> +
> + list_for_each_entry_safe(curr, tmp, &vgdev->obj_restore_list, list) {
> + if (bo == curr) {
> + spin_lock(&vgdev->obj_restore_lock);
> + list_del(&curr->list);
> + spin_unlock(&vgdev->obj_restore_lock);
> + break;
> + }
> + }
1. The whole list_for_each_entry_safe() needs to be protected with a
lock. But you don't need this iteration at all. Instead of using
cleanup_object(), unconditionally remove object from list in
virtio_gpu_free_object(), since we care only about shmem objects.
2. Use mutex instead of spinlock, we don't want problems with mem
reclaim situations
static void virtio_gpu_free_object(struct drm_gem_object *obj)
{
+ mutex_lock(&vgdev->obj_restore_lock);
+ list_del(&bo->restore_node);
+ mutex_unlock(&vgdev->obj_restore_lock);
if (bo->created) {
virtio_gpu_cmd_unref_resource(vgdev, bo);
virtio_gpu_notify(vgdev);
/* completion handler calls virtio_gpu_cleanup_object() */
return;
}
virtio_gpu_cleanup_object(bo);
}
--
Best regards,
Dmitry
