[PATCH 07/26] drm/radeon: add proper locking to the SA v2

[Jerome Glisse]

On Mon, Apr 30, 2012 at 10:50 AM, Christian König
<deathsimple at vodafone.de> wrote:
> Make the suballocator self containing to locking.
>
> v2: split the bugfix into a seperate patch.
>
> Signed-off-by: Christian König <deathsimple at vodafone.de>

I would say NAK but i don't have better solution yet to the issue.
Idea is that cs mutex protect the SA, i am trying hard to avoid adding
new lock, in fact i would like to remove more lock and have some idea
for that. I looked at the whole patch set and the issue is with radeon
semaphore allocation in ttm move blit case, all other case have the cs
mutex taken already. So if no better solution than yeah introduce a
new lock.

Cheers,
Jerome

> ---
>  drivers/gpu/drm/radeon/radeon.h    |    1 +
>  drivers/gpu/drm/radeon/radeon_sa.c |   17 +++++++++++------
>  2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 92682b7..99b188a 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -381,6 +381,7 @@ struct radeon_bo_list {
>  * alignment).
>  */
>  struct radeon_sa_manager {
> +       spinlock_t              lock;
>        struct radeon_bo        *bo;
>        struct list_head        sa_bo;
>        unsigned                size;
> diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
> index 8fbfe69..4ce5c51 100644
> --- a/drivers/gpu/drm/radeon/radeon_sa.c
> +++ b/drivers/gpu/drm/radeon/radeon_sa.c
> @@ -37,6 +37,7 @@ int radeon_sa_bo_manager_init(struct radeon_device *rdev,
>  {
>        int r;
>
> +       spin_lock_init(&sa_manager->lock);
>        sa_manager->bo = NULL;
>        sa_manager->size = size;
>        sa_manager->domain = domain;
> @@ -136,18 +137,19 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
>        struct radeon_sa_bo *tmp;
>        struct list_head *head;
>        unsigned offset = 0, wasted = 0;
> +       unsigned long flags;
>
>        BUG_ON(align > RADEON_GPU_PAGE_SIZE);
>        BUG_ON(size > sa_manager->size);
> +       spin_lock_irqsave(&sa_manager->lock, flags);
>
>        /* no one ? */
> -       head = sa_manager->sa_bo.prev;
>        if (list_empty(&sa_manager->sa_bo)) {
> +               head = &sa_manager->sa_bo;
>                goto out;
>        }
>
>        /* look for a hole big enough */
> -       offset = 0;
>        list_for_each_entry(tmp, &sa_manager->sa_bo, list) {
>                /* room before this object ? */
>                if (offset < tmp->offset && (tmp->offset - offset) >= size) {
> @@ -157,9 +159,8 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
>                offset = tmp->offset + tmp->size;
>                wasted = offset % align;
>                if (wasted) {
> -                       wasted = align - wasted;
> +                       offset += align - wasted;
>                }
> -               offset += wasted;
>        }
>        /* room at the end ? */
>        head = sa_manager->sa_bo.prev;
> @@ -167,11 +168,11 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
>        offset = tmp->offset + tmp->size;
>        wasted = offset % align;
>        if (wasted) {
> -               wasted = align - wasted;
> +               offset += wasted = align - wasted;
>        }
> -       offset += wasted;
>        if ((sa_manager->size - offset) < size) {
>                /* failed to find somethings big enough */
> +               spin_unlock_irqrestore(&sa_manager->lock, flags);
>                return -ENOMEM;
>        }
>
> @@ -180,10 +181,14 @@ out:
>        sa_bo->offset = offset;
>        sa_bo->size = size;
>        list_add(&sa_bo->list, head);
> +       spin_unlock_irqrestore(&sa_manager->lock, flags);
>        return 0;
>  }
>
>  void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo)
>  {
> +       unsigned long flags;
> +       spin_lock_irqsave(&sa_bo->manager->lock, flags);
>        list_del_init(&sa_bo->list);
> +       spin_unlock_irqrestore(&sa_bo->manager->lock, flags);
>  }
> --
> 1.7.5.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel