Hi Christian,
Am Dienstag, den 27.02.2018, 12:49 +0100 schrieb Christian König:
> Unpin the GEM object only after freeing the sg table.
What is the race that is being fixed here? The SG table is private to
the importer and the importer should hopefully only call map_detach if
it is done with all operations using the SG table. Thus it shouldn't
matter that the SG table might point to moved pages during execution of
this function.
Regards,
Lucas
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
> drivers/gpu/drm/drm_prime.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c
> b/drivers/gpu/drm/drm_prime.c
> index e82a976f0fba..c38dacda6119 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -230,26 +230,26 @@ void drm_gem_map_detach(struct dma_buf
> *dma_buf,
> struct drm_prime_attachment *prime_attach = attach->priv;
> struct drm_gem_object *obj = dma_buf->priv;
> struct drm_device *dev = obj->dev;
> - struct sg_table *sgt;
>
> - if (dev->driver->gem_prime_unpin)
> - dev->driver->gem_prime_unpin(obj);
> + if (prime_attach) {
> + struct sg_table *sgt = prime_attach->sgt;
>
> - if (!prime_attach)
> - return;
> -
> - sgt = prime_attach->sgt;
> - if (sgt) {
> - if (prime_attach->dir != DMA_NONE)
> - dma_unmap_sg_attrs(attach->dev, sgt->sgl,
> sgt->nents,
> - prime_attach->dir,
> - DMA_ATTR_SKIP_CPU_SYNC);
> - sg_free_table(sgt);
> + if (sgt) {
> + if (prime_attach->dir != DMA_NONE)
> + dma_unmap_sg_attrs(attach->dev, sgt-
> >sgl,
> + sgt->nents,
> + prime_attach-
> >dir,
> + DMA_ATTR_SKIP_CPU
> _SYNC);
> + sg_free_table(sgt);
> + }
> +
> + kfree(sgt);
> + kfree(prime_attach);
> + attach->priv = NULL;
> }
>
> - kfree(sgt);
> - kfree(prime_attach);
> - attach->priv = NULL;
> + if (dev->driver->gem_prime_unpin)
> + dev->driver->gem_prime_unpin(obj);
> }
> EXPORT_SYMBOL(drm_gem_map_detach);
>
