Quoting Haneen Mohammed (2018-07-09 16:44:26)
> +struct page **get_pages(struct vkms_gem_object *vkms_obj)
> +{
> + struct drm_gem_object *gem_obj = &vkms_obj->gem;
> + struct page **pages = vkms_obj->pages;
> +
> + if (!pages) {
> + mutex_lock(&vkms_obj->pages_lock);
> + pages = drm_gem_get_pages(gem_obj);
> + if (IS_ERR(pages)) {
> + mutex_unlock(&vkms_obj->pages_lock);
> + return pages;
> + }
> +
> + vkms_obj->pages = pages;
> + mutex_unlock(&vkms_obj->pages_lock);
You have a race here with two callers setting pages. Trivially you fix
it by checking if (!pages) again inside the lock, but the lock is
superfluous in this case:
if (!vkms_obj->pages) {
srtuct pages **pages;
pages = drm_gem_get_pages(gem_obj);
if (IS_ERR(pages))
return pages;
if (cmpxchg(&vkms_obj->pages, NULL, pages))
put_pages(pages);
/* barrier() is implied */
}
return vkms_obj->pages;
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
